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

[Fizz] Reset error component stack and fix error messages #27456

Merged
merged 3 commits into from
Oct 4, 2023

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Oct 4, 2023

The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an error happens. That resets lastBoundaryErrorComponentStackDev to null but if we don't, it just lingers and we don't set it to anything new then which leaks the previous component stack into the next time we have an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse problem that abortRemainingReplayNodes can call captureBoundaryErrorDetailsDev more than one time. So the second boundary won't get a stack.

We probably should try to figure out an alternative way to carry along the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady event if we error a replay. That event is a bit weird for resuming because we're probably really supposed to just invoke it immediately if we have already flushed the shell in the prerender which is always atm. Right now, it gets invoked later than necessary because you could have a resumed hole ready before a sibling in the shell is ready and that's blocked.

@sebmarkbage sebmarkbage requested a review from gnoff October 4, 2023 03:55
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Oct 4, 2023
@react-sizebot
Copy link

react-sizebot commented Oct 4, 2023

Comparing: 85c2b51...104995f

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 167.58 kB 167.58 kB = 52.14 kB 52.14 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 176.22 kB 176.22 kB = 54.84 kB 54.84 kB
facebook-www/ReactDOM-prod.classic.js = 564.39 kB 564.39 kB = 99.37 kB 99.37 kB
facebook-www/ReactDOM-prod.modern.js = 548.12 kB 548.12 kB = 96.45 kB 96.45 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-server/cjs/react-server.production.min.js +0.75% 33.55 kB 33.81 kB +0.50% 10.60 kB 10.65 kB
oss-stable/react-server/cjs/react-server.production.min.js +0.75% 33.55 kB 33.81 kB +0.50% 10.60 kB 10.65 kB
oss-experimental/react-server/cjs/react-server.production.min.js +0.68% 37.06 kB 37.32 kB +0.40% 11.61 kB 11.66 kB
oss-experimental/react-server/cjs/react-server.development.js +0.40% 197.86 kB 198.66 kB +0.15% 45.99 kB 46.06 kB
oss-stable-semver/react-server/cjs/react-server.development.js +0.38% 179.95 kB 180.64 kB +0.17% 41.81 kB 41.88 kB
oss-stable/react-server/cjs/react-server.development.js +0.38% 179.95 kB 180.64 kB +0.17% 41.81 kB 41.88 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.35% 72.81 kB 73.06 kB +0.24% 21.78 kB 21.83 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.35% 72.83 kB 73.09 kB +0.25% 21.81 kB 21.86 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.production.min.js +0.35% 73.04 kB 73.29 kB +0.27% 22.04 kB 22.10 kB
oss-stable/react-dom/cjs/react-dom-server.bun.production.min.js +0.34% 73.06 kB 73.32 kB +0.27% 22.06 kB 22.12 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.production.min.js +0.34% 74.93 kB 75.18 kB +0.20% 23.03 kB 23.08 kB
oss-stable/react-dom/cjs/react-dom-server.edge.production.min.js +0.34% 74.96 kB 75.21 kB +0.20% 23.06 kB 23.11 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.production.min.js +0.34% 75.01 kB 75.26 kB +0.22% 23.09 kB 23.14 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.min.js +0.34% 75.04 kB 75.29 kB +0.22% 23.12 kB 23.17 kB
facebook-www/ReactDOMServer-prod.modern.js +0.34% 175.81 kB 176.40 kB +0.20% 31.84 kB 31.90 kB
facebook-www/ReactDOMServer-prod.classic.js +0.33% 177.68 kB 178.27 kB +0.17% 32.18 kB 32.23 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.min.js +0.33% 78.11 kB 78.36 kB +0.21% 23.69 kB 23.74 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.32% 79.92 kB 80.18 kB +0.22% 24.42 kB 24.47 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.min.js +0.31% 82.37 kB 82.62 kB +0.18% 25.11 kB 25.15 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js +0.31% 183.97 kB 184.54 kB +0.15% 33.81 kB 33.86 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.31% 67.27 kB 67.48 kB +0.22% 19.99 kB 20.04 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.31% 67.30 kB 67.50 kB +0.22% 20.02 kB 20.06 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.31% 67.43 kB 67.64 kB +0.25% 20.34 kB 20.39 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.31% 67.46 kB 67.66 kB +0.25% 20.37 kB 20.42 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.min.js +0.30% 84.16 kB 84.41 kB +0.19% 25.92 kB 25.97 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js +0.29% 70.19 kB 70.39 kB +0.15% 21.79 kB 21.82 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js +0.29% 70.21 kB 70.42 kB +0.15% 21.81 kB 21.84 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js +0.29% 70.05 kB 70.25 kB +0.16% 21.49 kB 21.52 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js +0.29% 70.07 kB 70.28 kB +0.16% 21.51 kB 21.55 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.28% 74.13 kB 74.34 kB +0.30% 22.90 kB 22.97 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.28% 73.98 kB 74.19 kB +0.23% 22.57 kB 22.63 kB
oss-experimental/react-dom/umd/react-dom-server.browser.production.min.js +0.26% 79.01 kB 79.22 kB +0.16% 24.62 kB 24.66 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.min.js +0.26% 78.89 kB 79.09 kB +0.19% 24.24 kB 24.29 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +0.20% 397.02 kB 397.81 kB +0.08% 87.97 kB 88.04 kB

Generated by 🚫 dangerJS against 104995f

@sebmarkbage sebmarkbage merged commit 0fba3ec into facebook:main Oct 4, 2023
2 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 4, 2023
The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.

DiffTrain build for [0fba3ec](0fba3ec)
Umeshmalik pushed a commit to Umeshmalik/react that referenced this pull request Oct 8, 2023
…7456)

The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.
alunyov pushed a commit to alunyov/react that referenced this pull request Oct 11, 2023
…7456)

The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.
ztanner added a commit to vercel/next.js that referenced this pull request Oct 16, 2023
…experimental prefix for server action APIs (#56809)

The latest React canary builds have a few changes that need to be
adopted for compatability.

1. the `useFormState` and `useFormStatus` hooks in `react-dom` and the
`formData` opiont in `react-dom/server` are no longer prefixed with
`experimental_`
2. server content (an undocumented React feature) has been removed. Next
only had trivial intenral use of this API and did not expose a coherent
feature to Next users (no ability to seed context on refetches). It is
still possible that some users used the React server context APIs which
is why this should go into Next 14.

### React upstream changes

- facebook/react#27513
- facebook/react#27514
- facebook/react#27511
- facebook/react#27508
- facebook/react#27502
- facebook/react#27474
- facebook/react#26789
- facebook/react#27500
- facebook/react#27488
- facebook/react#27458
- facebook/react#27471
- facebook/react#27470
- facebook/react#27464
- facebook/react#27456
- facebook/react#27462
- facebook/react#27461
- facebook/react#27460
- facebook/react#27459
- facebook/react#27454
- facebook/react#27457
- facebook/react#27453
- facebook/react#27401
- facebook/react#27443
- facebook/react#27445
- facebook/react#27364
- facebook/react#27440
- facebook/react#27436

---------

Co-authored-by: Zack Tanner <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Jiachi Liu <[email protected]>
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…7456)

The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.

DiffTrain build for commit 0fba3ec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants