Skip to content

Don't show error or warning for ok response but no body. - #6196

Draft
michelinewu wants to merge 1 commit into
masterfrom
mw_ok_res
Draft

michelinewu wants to merge 1 commit into
masterfrom
mw_ok_res

Conversation

@michelinewu

Copy link
Copy Markdown
Contributor

Treat an OK Response With No Body as a Success in jfetch

Issues

Deleting a scheduled YouTube stream reported The request to the platform failed even though the delete had succeeded. YouTube's liveBroadcasts.delete answers 204 No Content, and Google stamps Content-Type: application/json; charset=UTF-8 on it anyway. jfetch decided how to read the body from that header alone. isJson was true, so it called response.json() on an empty body and got SyntaxError: Unexpected end of JSON input.

The content-type check was doing the wrong job. It answers "what format would the body be in?", not "is there a body?", and the code used it as though it answered both. Those coincide for every response except the empty ones.

From there the parse error was laundered into a platform error: platformRequest rejects, requestYoutube catches anything thrown and runs it through createPlatformError / throwPlatformError, and the result was a PLATFORM_REQUEST_FAILED with status: undefined and reason: undefined because there was no HTTP failure to describe in the first place. The log showed a failed request but the broadcast was gone from YouTube. Nothing user facing surface, but a developer could misdiagnose based off of this incorrect log.

The codebase already knew about this case. handlePlatformResponse in app/services/platforms/utils.ts carries the comment "Youtube API can return an empty content for a 'DELETE' request even if the content-type is application/json" and guards it with a try/catch. That guard just isn't on the path platformRequest uses, which is jfetch.

This PR adds this guard for all similar cases.

Fixes

jfetch now checks for the absence of a body before it decides how to parse one. When response.body is null the body is read as text and parsed only if there is something there, so an empty one resolves as the success it was instead of throwing. Everything else is untouched: a response with a body still routes on content-type exactly as before.

Files changed: app/util/requests.ts

Performance Implications

None meaningful. The extra response.text() only runs for responses that have no body to read, where it resolves immediately with an empty string, and it replaces a response.json() call that was doing the same read before throwing. Responses that carry a body take the identical path they did before.

Notes

The guard is deliberately narrow. response.body is null only for responses that cannot carry one — 204, 205, 304, and replies to HEAD — so an OK response with a body that happens to be empty, such as a 200 with Content-Length: 0, still reaches response.json() and would still throw. That shape does occur in the wild (some APIs answer a successful POST or DELETE that way), so this may need widening to a plain emptiness check later; it isn't widened here because nothing we call currently does it, and keeping the gate on "can this response have a body at all" avoids changing behaviour for anything that does.

Malformed JSON intentionally still throws because a truncated or corrupted payload handed back as a silent undefined would be a harder failure to diagnose than the one being fixed.

Two small changes fall out of routing bodyless responses through the new branch. A bodyless response with a non-JSON content-type previously resolved as '' and now resolves as undefined, and it no longer triggers the jfetch: Got non-JSON response warning — which was noise for 204s in any case.

@bundlemon

bundlemon Bot commented Sep 18, 2026

Copy link
Copy Markdown

BundleMon

Files updated (1)
Status Path Size Limits
✅ renderer.(hash).js
10.65MB (+85B 0%) -
Unchanged files (3)
Status Path Size Limits
✅ vendors~renderer.(hash).js
4.67MB -
✅ updater.js
115.29KB -
✅ guest-api.js
40.23KB -

Total files change +85B 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

This branch has not been deployed

No deployments
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.

1 participant