Don't show error or warning for ok response but no body. - #6196
Draft
michelinewu wants to merge 1 commit into
Draft
michelinewu wants to merge 1 commit into
michelinewu wants to merge 1 commit into
Conversation
BundleMonFiles updated (1)
Unchanged files (3)
Total files change +85B 0% Final result: ✅ View report in BundleMon website ➡️ |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Treat an OK Response With No Body as a Success in
jfetchIssues
Deleting a scheduled YouTube stream reported
The request to the platform failedeven though the delete had succeeded. YouTube'sliveBroadcasts.deleteanswers204 No Content, and Google stampsContent-Type: application/json; charset=UTF-8on it anyway.jfetchdecided how to read the body from that header alone.isJsonwas true, so it calledresponse.json()on an empty body and gotSyntaxError: 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:
platformRequestrejects,requestYoutubecatches anything thrown and runs it throughcreatePlatformError/throwPlatformError, and the result was aPLATFORM_REQUEST_FAILEDwithstatus: undefinedandreason: undefinedbecause 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.
handlePlatformResponseinapp/services/platforms/utils.tscarries 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 atry/catch. That guard just isn't on the pathplatformRequestuses, which isjfetch.This PR adds this guard for all similar cases.
Fixes
jfetchnow checks for the absence of a body before it decides how to parse one. Whenresponse.bodyis 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.tsPerformance 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 aresponse.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.bodyis null only for responses that cannot carry one —204,205,304, and replies toHEAD— so an OK response with a body that happens to be empty, such as a200withContent-Length: 0, still reachesresponse.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
undefinedwould 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 asundefined, and it no longer triggers thejfetch: Got non-JSON responsewarning — which was noise for204s in any case.