Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GodotFetch glue code for null response bodies #98431

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

lodicolo
Copy link
Contributor

@lodicolo lodicolo commented Oct 22, 2024

Fixes #76825

The spec says that body can be null for fetch responses (in the event of requests that should have no body, like HEAD requests) and Firefox adheres to it which results in request failure for HEAD requests on Firefox for web exports.

This fix addresses that by treating a null body as an "empty" body (without using a polyfill) and avoids changing the request lifecycle as much as possible.

Minimal reproduction project: mrp-gh76825.zip

Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small changes (more nitpicks than real changes), but very good overall. Thanks!

@adamscott
Copy link
Member

You can remove me from the commit! Keep ownership of it, really.

The spec says that Response.body can be null (in the event of requests that should have no body, like HEAD requests) and Firefox adheres to it which results in request failure for HEAD requests on Firefox for web exports.

This commit addresses that by treating a null body as an "empty" body (without using a polyfill) and avoids changing the request lifecycle as much as possible.

PR review changes:
- Use == instead of strict ===
- Do not use ?? null
- Comment formatting
@lodicolo lodicolo force-pushed the 4.3-stable_GH_76825 branch from a36871f to c7f421e Compare October 23, 2024 12:20
@lodicolo
Copy link
Contributor Author

I updated the commit and also fixed the eslint error, and rebuilt the export locally and re-verified it on both my project as well as the MRP I provided.

Thanks for the review!

@adamscott adamscott self-requested a review October 23, 2024 14:55
@Repiteo Repiteo merged commit 1015a48 into godotengine:master Oct 24, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 24, 2024

Thanks, and congratulations on your first contribution! 🎉

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

Successfully merging this pull request may close these issues.

HTTPRequest fails HEAD mode requests in Chrome 113
4 participants