Release: develop -> main - #1234
Merged
Merged
Conversation
…1232) * Report a failure once, and repair the stored answer that caused it Two defects, both measured in production today. The report guard lived in a ref inside the error screen, so it lasted exactly as long as one mount. The router remounts the same route repeatedly, each remount is a fresh component, and each one reported the same failure again: a single open page produced 19-30 reports per second, ~68'000 an hour. That fills the ingest rate limit and pushes out the reports of every other customer, so the flood also blinds the thing it floods. The guard now lives in the module and keys on type, message and route for a minute. The second defect is why the failure never ended. A reload re-reads index.html but not the stored answer for the chunk, and a chunk URL is stable as long as its content is. A customer holding an app shell under that URL is served it again on every attempt, for the year those assets are considered fresh -- reload repeats the failure instead of clearing it. The failing address is now re-requested with `cache: 'reload'` before the reload, which forces the browser to the network and makes intermediate caches revalidate. The reload follows either way: it is the recovery and must not depend on the repair succeeding. Only same-origin addresses are touched, since the message is derived from an error object an embedded bundler could supply. What this does not fix: entries already held in the CDN edge cache. A cache hit there never reaches the origin, so those have to be purged out of band. * Guarantee the reload, and find the address on every path that reports one Three defects in the previous commit. The reload waited on the repair with no bound. A request that never settles would have withheld the recovery indefinitely -- worse than before this existed, when the reload was immediate. It is now bounded by a two-second budget as well as by the request settling, whichever comes first, and runs exactly once either way. The global error and unhandledrejection handler called the recovery without the error, so chunk failures outside the router -- during startup, or from an import() in a handler -- reloaded without ever replacing the stored answer. That path kept the defect this change exists to remove. The address was read only out of the message text and only in absolute form. It is now taken from the `request` property webpack sets on the error where present, resolved against the document so a relative address works, and read from the dynamic-import wording too, which isChunkLoadError already accepted. Same-origin confinement is unchanged and now applies after resolution. * Never let looking up the address cost the reload Finding the failing address reads from a value we were handed: the property can be a getter that throws, and deriving the message runs String() on it, which a hostile toString turns into a throw. Either would have propagated out after the reload guard was already written -- leaving the customer with no repair and no reload until it expired, which is worse than not attempting the repair at all. The lookup is now contained. Finding no address stays harmless: the caller reloads without repairing, which is what it did before any of this existed. * Classify a failure without letting the value stop us Classification is the first thing the global handler does with a thrown value, and it derived the message to do it -- which calls String() on that value. A hostile toString therefore threw before the handler reached the recovery, and the customer stayed on the broken page. Contained now, with the name checked first: reading it cannot run code of the value's own choosing. This was not introduced here -- the classification is unchanged from before this branch -- but it sits directly in the path this change exists to make reliable. The test asserts through the event rather than by calling the recovery directly, because that is exactly the difference: the earlier unit test passed while this path was broken. Verified by removing the fix and confirming the new test fails, then restoring it. * Say what the name read actually does The comment claimed that reading the name cannot run the value's own code. It can: a getter or a proxy trap sits behind a plain property access just as well. The code was already right -- the read is isolated and contained -- but the reason given for it was not. Comment only, no behaviour change.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
August 1, 2026 09:34
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist