fix(nextjs): Don't report Next.js prerender control flow errors - #23691
Draft
chargome wants to merge 1 commit into
Draft
fix(nextjs): Don't report Next.js prerender control flow errors#23691chargome wants to merge 1 commit into
chargome wants to merge 1 commit into
Conversation
Next.js throws a set of errors to steer rendering rather than to signal a failure. Its `unstable_rethrow` defines the contract any code wrapping user land in a try/catch has to honor, but the Sentry wrappers only recognized redirects and not-founds. Everything else - most visibly the `HANGING_PROMISE_REJECTION` that Cache Components produces for uncached `fetch()` during a prerender - was reported as an error. Filtering was also coupled to `getActiveSpan()` being truthy, so with no active span nothing was filtered at all, including redirects and not-founds. Fixes #23592 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
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
1 participant
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.
Warning
WIP — open for early feedback. See Open questions below; the e2e variant change in particular is a judgement call I'd like a second opinion on.
Next.js throws a set of errors to steer rendering rather than to signal a failure.
unstable_rethrowis the contract for anything that wraps user land in atry/catch, and our wrappers only honored half of it:NEXT_REDIRECT;…/NEXT_NOT_FOUNDHANGING_PROMISE_REJECTIONNEXT_PRERENDER_INTERRUPTEDDYNAMIC_SERVER_USAGEBAILOUT_TO_CLIENT_SIDE_RENDERINGRoot cause
Under Cache Components an uncached
fetch()inside a prerender is never issued. Next hands out a promise that never settles and rejects it once the prerender is aborted. React discards that rejection — it never reaches the user — but our server component wrapper observed it and reported it, which is where the reporter's 10k+handledevents with no user impact came from.A second, independent bug amplified this. Since #18408 the filtering sat behind
getActiveSpan():Before that refactor
startSpanManualguaranteed a span, so the filters always ran. Afterwards, no active span means nothing is filtered — including plainredirect()andnotFound(). That is exactly the Cache Components situation, sincenextSpan.ts#shouldNoopSpanskips span creation there and the prerender runs detached from thehttp.serverspan. Both wrappers now decide independently of span presence.Decisions
Cause chains are walked.
unstable_rethrowrecurses througherror.cause, so the shared digest helper does too (depth-capped against self-referencing causes). This also changes the existing redirect/not-found checks, which previously only looked at the top-level error — more correct, but it is a behavior change worth noticing.Filtering is applied in more than the wrappers.
captureRequestErrorand theDropReactControlFlowErrorsevent processor got the same check, as a net for paths the wrappers don't cover.Verifying this was harder than expected
A plain request does not reproduce it — Next replays the build-time shell. It needs a runtime prefetch (
RSC: 1+Next-Router-Prefetch: 2), which makes Next re-run a prerender at request time. I found that by bisecting against a throwaway Next 16.3.3 app rather than guessing, and the e2e test drives it the same way.More importantly:
wrappingLoaderis webpack-only. Turbopack builds getgenerateValueInjectionRulesand nothing else, so server components are never wrapped there and no error reaches Sentry at all — I confirmed this with a diagnostic run. My first version of the e2e test passed without the fix for exactly this reason. The reporter is therefore almost certainly on a webpack build, consistent with their "Sentry trace wrapper in the SSR chunk", and thecaptureRequestError/ event-processor changes are defense-in-depth rather than the live path.The unit tests fail 7/11 with the wrapper fix reverted; the e2e test fails without the fix and passes with it, in both prod and dev.
Open questions
nextjs-16-cacheComponentscarried"//": "TODO: Add variants for webpack once supported", so I added one — without it the new test is decorative in CI. All 9 tests pass under it, so the TODO looks stale, but this does mean the app's whole suite now also runs under webpack. Happy to drop it if that's unwanted here.DOMException(name: AbortError,code: 20,"This operation was aborted") escaping the same wrapper on routes awaitingparams/searchParamsunder an app-shell prefetch (Next-Router-Prefetch: 3). It has no digest, so this fix doesn't catch it, and it isn't inunstable_rethroweither. Blanket-filtering AbortErrors would swallow genuine ones, so I left it alone — probably its own issue.Fixes #23592