Add unstable_retry() to error.js#89685
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
packages/next/src/next-devtools/userspace/app/app-dev-overlay-error-boundary.tsx
Outdated
Show resolved
Hide resolved
Tests Passed |
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles: **400 kB** → **400 kB**
|
| Canary | PR | Change | |
|---|---|---|---|
| middleware-b..fest.js gzip | 764 B | 764 B | ✓ |
| Total | 764 B | 764 B | ✓ |
Build Details
Build Manifests
| Canary | PR | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 451 B | 452 B | ✓ |
| Total | 451 B | 452 B |
📦 Webpack
Client
Main Bundles
| Canary | PR | Change | |
|---|---|---|---|
| 5528-HASH.js gzip | 5.54 kB | N/A | - |
| 6280-HASH.js gzip | 58.3 kB | N/A | - |
| 6335.HASH.js gzip | 169 B | N/A | - |
| 912-HASH.js gzip | 4.59 kB | N/A | - |
| e8aec2e4-HASH.js gzip | 62.6 kB | N/A | - |
| framework-HASH.js gzip | 59.7 kB | 59.7 kB | ✓ |
| main-app-HASH.js gzip | 255 B | 254 B | ✓ |
| main-HASH.js gzip | 39.1 kB | 39.1 kB | ✓ |
| webpack-HASH.js gzip | 1.68 kB | 1.68 kB | ✓ |
| 262-HASH.js gzip | N/A | 4.59 kB | - |
| 2889.HASH.js gzip | N/A | 169 B | - |
| 5602-HASH.js gzip | N/A | 5.55 kB | - |
| 6948ada0-HASH.js gzip | N/A | 62.6 kB | - |
| 9544-HASH.js gzip | N/A | 59.1 kB | - |
| Total | 232 kB | 233 kB |
Polyfills
| Canary | PR | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 39.4 kB | 39.4 kB | ✓ |
| Total | 39.4 kB | 39.4 kB | ✓ |
Pages
| Canary | PR | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 194 B | 194 B | ✓ |
| _error-HASH.js gzip | 183 B | 180 B | 🟢 3 B (-2%) |
| css-HASH.js gzip | 331 B | 330 B | ✓ |
| dynamic-HASH.js gzip | 1.81 kB | 1.81 kB | ✓ |
| edge-ssr-HASH.js gzip | 256 B | 256 B | ✓ |
| head-HASH.js gzip | 351 B | 352 B | ✓ |
| hooks-HASH.js gzip | 384 B | 383 B | ✓ |
| image-HASH.js gzip | 580 B | 581 B | ✓ |
| index-HASH.js gzip | 260 B | 260 B | ✓ |
| link-HASH.js gzip | 2.5 kB | 2.5 kB | ✓ |
| routerDirect..HASH.js gzip | 320 B | 319 B | ✓ |
| script-HASH.js gzip | 386 B | 386 B | ✓ |
| withRouter-HASH.js gzip | 315 B | 315 B | ✓ |
| 1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
| Total | 7.97 kB | 7.97 kB | ✅ -2 B |
Server
Edge SSR
| Canary | PR | Change | |
|---|---|---|---|
| edge-ssr.js gzip | 125 kB | 125 kB | ✓ |
| page.js gzip | 254 kB | 255 kB | ✓ |
| Total | 379 kB | 379 kB |
Middleware
| Canary | PR | Change | |
|---|---|---|---|
| middleware-b..fest.js gzip | 616 B | 617 B | ✓ |
| middleware-r..fest.js gzip | 156 B | 155 B | ✓ |
| middleware.js gzip | 43.9 kB | 44 kB | ✓ |
| edge-runtime..pack.js gzip | 842 B | 842 B | ✓ |
| Total | 45.5 kB | 45.6 kB |
Build Details
Build Manifests
| Canary | PR | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 715 B | 718 B | ✓ |
| Total | 715 B | 718 B |
Build Cache
| Canary | PR | Change | |
|---|---|---|---|
| 0.pack gzip | 4.01 MB | 4.03 MB | 🔴 +14 kB (+0%) |
| index.pack gzip | 102 kB | 102 kB | ✓ |
| index.pack.old gzip | 103 kB | 102 kB | ✓ |
| Total | 4.22 MB | 4.23 MB |
🔄 Shared (bundler-independent)
Runtimes
| Canary | PR | Change | |
|---|---|---|---|
| app-page-exp...dev.js gzip | 320 kB | 320 kB | ✓ |
| app-page-exp..prod.js gzip | 170 kB | 170 kB | ✓ |
| app-page-tur...dev.js gzip | 319 kB | 319 kB | ✓ |
| app-page-tur..prod.js gzip | 169 kB | 169 kB | ✓ |
| app-page-tur...dev.js gzip | 316 kB | 316 kB | ✓ |
| app-page-tur..prod.js gzip | 168 kB | 168 kB | ✓ |
| app-page.run...dev.js gzip | 316 kB | 316 kB | ✓ |
| app-page.run..prod.js gzip | 168 kB | 168 kB | ✓ |
| app-route-ex...dev.js gzip | 70.8 kB | 70.8 kB | ✓ |
| app-route-ex..prod.js gzip | 49.2 kB | 49.2 kB | ✓ |
| app-route-tu...dev.js gzip | 70.8 kB | 70.8 kB | ✓ |
| app-route-tu..prod.js gzip | 49.2 kB | 49.2 kB | ✓ |
| app-route-tu...dev.js gzip | 70.4 kB | 70.4 kB | ✓ |
| app-route-tu..prod.js gzip | 49 kB | 49 kB | ✓ |
| app-route.ru...dev.js gzip | 70.4 kB | 70.4 kB | ✓ |
| app-route.ru..prod.js gzip | 49 kB | 49 kB | ✓ |
| dist_client_...dev.js gzip | 324 B | 324 B | ✓ |
| dist_client_...dev.js gzip | 326 B | 326 B | ✓ |
| dist_client_...dev.js gzip | 318 B | 318 B | ✓ |
| dist_client_...dev.js gzip | 317 B | 317 B | ✓ |
| pages-api-tu...dev.js gzip | 43.2 kB | 43.2 kB | ✓ |
| pages-api-tu..prod.js gzip | 32.9 kB | 32.9 kB | ✓ |
| pages-api.ru...dev.js gzip | 43.2 kB | 43.2 kB | ✓ |
| pages-api.ru..prod.js gzip | 32.8 kB | 32.8 kB | ✓ |
| pages-turbo....dev.js gzip | 52.5 kB | 52.5 kB | ✓ |
| pages-turbo...prod.js gzip | 38.5 kB | 38.5 kB | ✓ |
| pages.runtim...dev.js gzip | 52.5 kB | 52.5 kB | ✓ |
| pages.runtim..prod.js gzip | 38.4 kB | 38.4 kB | ✓ |
| server.runti..prod.js gzip | 61.9 kB | 61.9 kB | ✓ |
| Total | 2.82 MB | 2.82 MB |
📝 Changed Files (8 files)
Files with changes:
app-page-exp..ntime.dev.jsapp-page-exp..time.prod.jsapp-page-tur..ntime.dev.jsapp-page-tur..time.prod.jsapp-page-tur..ntime.dev.jsapp-page-tur..time.prod.jsapp-page.runtime.dev.jsapp-page.runtime.prod.js
View diffs
app-page-exp..ntime.dev.js
failed to diffapp-page-exp..time.prod.js
Diff too large to display
app-page-tur..ntime.dev.js
failed to diffapp-page-tur..time.prod.js
Diff too large to display
app-page-tur..ntime.dev.js
failed to diffapp-page-tur..time.prod.js
failed to diffapp-page.runtime.dev.js
failed to diffapp-page.runtime.prod.js
Diff too large to display
📎 Tarball URL
next@https://vercel-packages.vercel.app/next/prs/89685/next
4bc9128 to
3cd7ff9
Compare
retry(), componentStack, and ownerStack to error.js
3cd7ff9 to
e5e0973
Compare
564cf7e to
2a614e7
Compare
f47659f to
bd6fd7a
Compare
bd6fd7a to
b8e507c
Compare
1f6797f to
7e8af59
Compare
b8e507c to
f6e1243
Compare
retry(), componentStack, and ownerStack to error.jsunstable_retry(), componentStack, and ownerStack to error.js
40c132c to
29a7740
Compare
29a7740 to
93d8795
Compare
93d8795 to
24b79b2
Compare
| ) | ||
| } | ||
|
|
||
| let hasThrown = false |
There was a problem hiding this comment.
It should probably set a globalThis to false and then in the test itself you set it to true to switch from erroring to recovered. by using a render side effect the test could change subtly in the future based on multiple renders that might make it no longer assert what we expect
There was a problem hiding this comment.
Next test spins up a new process, so handling globalThis inside suites is not shared between the Jest worker, so I used a search query and a route to enable/disable the "should throw" value.
| ).toMatchInlineSnapshot(` | ||
| [ | ||
| "at ErrorBoundary", | ||
| "at OuterLayoutRouter", |
There was a problem hiding this comment.
This isn't the owner stack we want. we want the owner stack of the component that errored. We should talk to Sebbie about where to call this so we get the right stack
There was a problem hiding this comment.
As discussed, splitted from this PR, should follow up
The module-scoped `hasThrown` variable flipped during render, which is fragile if React re-renders multiple times. Use a `globalThis` flag toggled via a route handler so the test explicitly controls recovery. #89685 (comment)
…errors Add Container wrapper around the throwing server component so componentStack and ownerStack reflect a realistic hierarchy. Render componentStack/ownerStack in the server-component error boundary and add inline snapshot assertions. #89685 (comment)
unstable_retry(), componentStack, and ownerStack to error.jsunstable_retry() to error.js
gnoff
left a comment
There was a problem hiding this comment.
I think you should keep the parent stack thought you could land separately
Extends the error components API to give better control over recovery. Previously, the `reset` prop only cleared the error state and re-rendered the children. However, this only handles a temporary error during rendering. The error can be from data fetching, or from a RSC phase etc. So in these cases `reset` alone is not enough. The users would even need to implement a retry logic with `router.refresh()`. Therefore this commit adds a new `retry` prop which calls `router.refresh()` and `reset` inside a `startTransition` to provide a built-in retry logic. This feature is expected to preferred over than the `reset` prop. Only the cases where you'd choose `reset` is when you have a reason to do sync reset without loading any new data. This commit also adds `componentStack` and `ownerStack` (dev-only) to the error components API for better DX and debugging error handling.
error-boundary.tsx imported publicAppRouterInstance from app-router-instance.ts, which transitively pulls in the entire App Router module graph (router-reducer → fetch-server-response → react-server-dom-webpack/client). This broke any non-App-Router context (e.g. Pages Router) that imports a module depending on error-boundary.tsx, since react-server-dom-webpack/client is unavailable there. Replace with AppRouterContext from the lightweight shared-runtime module, which only contains type imports and React context creation — no heavy runtime dependencies. The error boundary already renders inside the AppRouterContext.Provider scope, so this.context always has the router instance for segment-level boundaries.
The module-scoped `hasThrown` variable flipped during render, which is fragile if React re-renders multiple times. Use a `globalThis` flag toggled via a route handler so the test explicitly controls recovery. #89685 (comment)
…errors Add Container wrapper around the throwing server component so componentStack and ownerStack reflect a realistic hierarchy. Render componentStack/ownerStack in the server-component error boundary and add inline snapshot assertions. #89685 (comment)
The ownerStack captured in the error boundary returns the error boundary's owner stack instead of the errored component's. Remove componentStack and ownerStack from error.js props to address them separately with a correct capture strategy.
CI no longer produces ClientPageRoot in the redbox stack trace.
65c45ec to
c858df2
Compare

Extends the error components API to give better control over recovery. Previously, the
reset()prop only cleared the error state and re-rendered the children. However, this only handles a temporary rendering error.The error can be due to data fetching or an RSC phase. In these cases,
reset()alone is not sufficient. Users would even need to implement retry logic usingrouter.refresh().Therefore, this PR adds a new
unstable_retry()prop that callsrouter.refresh()andreset()within astartTransition()to provide built-in retry logic. This feature is expected to be preferred over thereset()prop. Only the cases where you'd choosereset()are when you have a reason to do a sync reset without loading any new data.Closes NAR-767