Do not parse as JSON on 204 responses
nholik opened this issue · 3 comments
Description
On 204 response codes should not attempt to call resp.json()
as a body would be unexpected by most conventions.
Proposal
Following this as one recommendation
I would suggest changing
Line 142 in fabfad2
- return res.ok ? { data: await res.json(), response } : { error: await res.json(), response };
+ return res.ok ? { data: res.status === 204 ? "" : await res.json(), response } : { error: await res.json(), response };
or a more sophisticated approach may be on 204 and body length is 0, or a similar strategy. I would be happy to submit a PR with the suggested change if this line of reasoning sounds okay.
Checklist
- I’m willing to open a PR for this (see CONTRIBUTING.md)
I think this is reasonable! Would definitely accept a PR for this, with one minor change: I’d change the ""
return to {}
. Mostly so if (data)
would still work the same as expected runtime wise. But also because an object type in general is a more common response than a string and it may be more predictable to reason about (even if it’s an empty object).
Not just 204 responses. There might be reasons to not parse as JSON for any status.