drwpow/openapi-fetch

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

return res.ok ? { data: await res.json(), response } : { error: await res.json(), response };
to something like

-        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 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).

@drwpow that makes perfect sense to do {}. I've submitted a PR to that effect #28

rojvv commented

Not just 204 responses. There might be reasons to not parse as JSON for any status.