PUT requests to Suspend account using different content-type on 1.5

(Scott Nixon) #1

Our site is using Discourse 1.5.1 and I believe our hosting provider upgraded us from 1.4 last week. I have no control over this.

It appears that all API responses from Discourse have the content-type of “application/json”. But I have found that under 1.5 when you issue PUT requests to Suspend an account it now responses with the content-type “text/plain”.

I looked at a number of other PUT requests and they all were using “application/json” so I suspect this was an unintended change.

I’m using the pydiscourse library and until the upgrade didn’t throw any errors for PUT requests. Now every PUT request generates an exception because the library checks the response content-type.

Please let me know if I can provide any additional detail.

(Jeff Atwood) #2

Is this still an issue under 1.7?

(Scott Nixon) #3

Our site is still on 1.6.3 and is still showing plain text. FYI, I’m not able to upgrade our site. I’ve contacted our host about the upgrade.

(Jeff Atwood) #4

Any thoughts here @oblakeerickson?

(Blake Erickson) #7

I have confirmed we are not returning json. I don’t see any recent changes on that controller action that would have broken this, but most of the responses are just set to render nothing: true, so I’ll add this to my list since it should probably be returning a success json response.

(Sam Saffron) #8

@eviltrout / @oblakeerickson this is actually a bigger API problem:

  1. Are we strictly a JSON API, meaning do we expect every single endpoint to always return render json: success_json vs render nothing: true ?

  2. If not, should we always be strict about returning json if the request is made to a .json resource ? (which means we need a fancy helper for render success)

As to the OP, I think that is legit that a PUT to /suspend.json should return a JSON payload. But… just cause you send JSON encoded body does not imply that the server must return JSON.

There is an argument for returning 200 with blank body, you are sending an infinitesimally smaller amount of data and a tiny bit less work on the JS side.

I am leaning towards just going with (1) and making every single PUT and POST JSON by default, and all XHRs JSON by default.

(Blake Erickson) #9

I personally just care that we return the correct status codes (which we do) so I don’t think this is really a bug.

I do think we should work toward being a strict JSON API though and if we can clean/improve things as we go lets do that.

(Scott Nixon) #10

I’ve done a fair amount of work over the years with Discourse’s API and it’s always felt really inconsistent. I’m not trying to be critical, but your attitude seems pretty dismissive of something that is clearly inconsistent. I detailed in the first post that all requests that I reviewed returned one way, and after the upgrade to 1.5.1 the API started returning a different content-type in this one particular case. You can chalk this up as not a big deal, but APIs being consistent and reliable is important. If you want to continue to have software that people want use and integrate with you should care about the details.

(Sam Saffron) #11

Why can’t we have both.

I think the point @oblakeerickson is trying to make is that this is not strictly a bug. I agree with that, but one persons bug is another persons feature, and this semantics really does not matter in this case.

Yes, we want to make the API more consistent, I think my post offers a good path forward, just waiting on feedback from @eviltrout

(Blake Erickson) #12

We really do appreciate your feedback because we don’t always realize or feel the pain of any inconsistencies or regressions that might occur with the API.

The API is still a priority for us and this year we took a huge step in that direction and actually documented the API, as @sam said we do plan to make the API more consistent, and I also would like to add some API tests so that we can better detect any API regressions.