Skip to content

Handle empty response (204)#78

Merged
floatdrop merged 1 commit intosindresorhus:masterfrom
connyay:handle-empty-json-response
Jun 30, 2015
Merged

Handle empty response (204)#78
floatdrop merged 1 commit intosindresorhus:masterfrom
connyay:handle-empty-json-response

Conversation

@connyay
Copy link
Copy Markdown

@connyay connyay commented Jun 26, 2015

No description provided.

@floatdrop
Copy link
Copy Markdown
Contributor

@connyay
Copy link
Copy Markdown
Author

connyay commented Jun 27, 2015

The situation I hit this with was on a 204 response. I started this PR only
checking if the status code was 204, but figured an empty 2xx would be
possible as well.

Thoughts?
On Fri, Jun 26, 2015 at 4:27 PM Vsevolod Strukchinsky <
[email protected]> wrote:

Technically - blank response (at least with 200 status code) is invalid
response.

http://stackoverflow.com/questions/11970962/valid-json-in-response


Reply to this email directly or view it on GitHub
#78 (comment).

@floatdrop
Copy link
Copy Markdown
Contributor

@connyay I think it is expected behaviour never less empty string is invalid JSON. request is behaving something like this.

@sindresorhus what do you think?

@sindresorhus
Copy link
Copy Markdown
Owner

I'm not sure tbh. Empty response on 200 is invalid, so we shouldn't do anything there. 204 is valid with blank response, so we should maybe handle that somehow (suggestions?). Maybe we should only try to parse JSON when content-type is "application/json"? I assume servers don't send that on 204?.

@floatdrop
Copy link
Copy Markdown
Contributor

@sindresorhus a lot of endpoints do not send application/json content-type in json responses. This was main point of strict json option - to parse body in any case.

@connyay could you do a 204 check instead?

Related: forwardemail/superagent#255

@connyay
Copy link
Copy Markdown
Author

connyay commented Jun 30, 2015

@floatdrop switched to check for 204

@connyay connyay changed the title Handle empty json response Handle empty response (204) Jun 30, 2015
@floatdrop
Copy link
Copy Markdown
Contributor

@connyay cool! Can you squash it into one commit?

@connyay connyay force-pushed the handle-empty-json-response branch from d30d74a to 3864556 Compare June 30, 2015 19:03
@connyay connyay force-pushed the handle-empty-json-response branch from 3864556 to 312c80d Compare June 30, 2015 19:03
@connyay
Copy link
Copy Markdown
Author

connyay commented Jun 30, 2015

@floatdrop should be good.

🍻

floatdrop added a commit that referenced this pull request Jun 30, 2015
@floatdrop floatdrop merged commit 2a155a8 into sindresorhus:master Jun 30, 2015
@floatdrop floatdrop added this to the 4.0.0 milestone Jun 30, 2015
@floatdrop
Copy link
Copy Markdown
Contributor

🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants