Repository navigation
Fix theme dev 502s on storefront responses with large headers - #8939
Open
rjmcgee53192 wants to merge 2 commits into
Open
rjmcgee53192 wants to merge 2 commits into
rjmcgee53192 wants to merge 2 commits into
Conversation
Pin a 128 KiB header-size cap for theme dev instead of inheriting Node's drifting defaults: install an undici Agent with that maxHeaderSize as the global fetch dispatcher when a dev server starts (covering the storefront proxy and renderer), pass the same cap to the local dev server so large browser Cookie headers are accepted, and surface the nested cause code (e.g. UND_ERR_HEADERS_OVERFLOW) in fetch error info so future failures are diagnosable from the 502 page. Fixes Shopify#8717
Author
|
signed |
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.
WHY are these changes introduced?
Fixes #8717
shopify theme devrenders every page as a 502 once the browser has cookies for the store: the storefront's response header block (a largeSet-Cookiecollection) exceeds the header-size cap of Node's bundled fetch, the resultingUND_ERR_HEADERS_OVERFLOWis wrapped in a genericTypeError: fetch failed, and the proxy surfaces a bare 502. The CLI pinned no header limit of its own, so the effective cap was whatever the Node version under it happened to default to that release.WHAT is this pull request doing?
Pins a 128 KiB header-size cap explicitly for
theme dev, on both hops the issue maps:storefront-fetchutility in@shopify/themecreates an undiciAgentwithmaxHeaderSizeof 128 KiB and installs it as the global fetch dispatcher when a dev server starts. The storefront proxy and renderer both use the globalfetch, so this covers every upstream storefront request without changing those call sites. 128 KiB matches the largest mainstream CDN header ceiling and covers the measured failing responses (~40 KB and up) with headroom; it stays an upper-bound check, not a preallocated buffer.maxHeaderSize, so the request side stops failing once the browser's cookie jar for the local origin — which accumulates the storefront's cookies — grows past Node's default request-header cap.extractFetchErrorInfonow walks the error'scausechain and surfaces the firstcodeit finds (for exampleUND_ERR_HEADERS_OVERFLOW) in the headline shown on the 502 page, instead of only the wrappedfetch failedmessage. This failure took aNODE_OPTIONSfetch shim to diagnose; the next one shouldn't.Adds
undici@7.28.0as a direct dependency of@shopify/theme(already present in the workspace lockfile) and a patch changeset for@shopify/theme.How to manually test your changes?
shopify theme devagainst a store whose storefront sets a largeSet-Cookiecollection on cookie'd requests (the reporter's case: a Plus storefront, response header block over ~40 KB), using a browser that already has cookies for the store.Set-Cookieheaders and confirm the page loads, and that a deliberately broken upstream now shows aCause: …line with the underlying error code on the 502 page.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset addNote on pnpm-lock.yaml
The lockfile update for this change is a 3-line addition (the
undiciimporter entry for@shopify/theme, specifier/version 7.28.0; undici 7.28.0 is already present in the lockfile's packages section). It will land as a follow-up commit on this branch shortly.