Skip to content

[Fizz] Push a stalled await from debug info to the ownerStack/debugTask #33634

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

Merged
merged 3 commits into from
Jun 25, 2025

Conversation

sebmarkbage
Copy link
Collaborator

If an aborted task is not rendering, then this is an async abort. Conceptually it's as if the abort happened inside the async gap. The abort reason's stack frame won't have that on the stack so instead we use the owner stack and debug task of any halted async debug info.

One thing that's a bit awkward is that if you do have a sync abort and you use that error as the "reason" then that thing still has a sync stack in a different component. In another approach I was exploring having different error objects for each component but I don't think that's worth it.

@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jun 25, 2025
@react-sizebot
Copy link

react-sizebot commented Jun 25, 2025

Comparing: b42341d...4ca194f

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.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.57 kB 530.57 kB = 93.67 kB 93.67 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 651.66 kB 651.66 kB = 114.79 kB 114.79 kB
facebook-www/ReactDOM-prod.classic.js = 674.81 kB 674.81 kB = 118.78 kB 118.78 kB
facebook-www/ReactDOM-prod.modern.js = 665.30 kB 665.30 kB = 117.19 kB 117.19 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-server/cjs/react-server.development.js +0.74% 212.53 kB 214.10 kB +0.56% 36.95 kB 37.16 kB
oss-experimental/react-markup/cjs/react-markup.development.js +0.40% 393.47 kB 395.05 kB +0.29% 70.86 kB 71.06 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +0.37% 421.26 kB 422.83 kB +0.28% 75.10 kB 75.31 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.37% 421.26 kB 422.83 kB +0.28% 75.10 kB 75.31 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +0.35% 452.01 kB 453.59 kB +0.37% 79.23 kB 79.53 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +0.35% 453.02 kB 454.59 kB +0.38% 79.46 kB 79.76 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +0.34% 459.01 kB 460.58 kB +0.37% 79.22 kB 79.51 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +0.27% 381.34 kB 382.36 kB +0.24% 73.12 kB 73.30 kB
oss-experimental/react-markup/cjs/react-markup.react-server.development.js +0.26% 607.77 kB 609.34 kB +0.20% 108.39 kB 108.61 kB

Generated by 🚫 dangerJS against 4ca194f

Comment on lines +2155 to +2156
// Mark the end time of the await. If we're aborting then we don't emit this
// to signal that this never resolved inside this render.
if (request.status !== ABORTING) {
markOperationEndTime(request, task, endTime);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that not cause an infinite span in the performance track?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because we skip writing it if there's no end time.

However, I do want to add some kind of marker in the track to log a halted entry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be useful to have some kind of end marker to know what time it was aborted though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardanishsingh1

This comment was marked as spam.

dcq01

This comment was marked as spam.

@sebmarkbage sebmarkbage merged commit cee7939 into facebook:main Jun 25, 2025
241 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 25, 2025
…sk (#33634)

If an aborted task is not rendering, then this is an async abort.
Conceptually it's as if the abort happened inside the async gap. The
abort reason's stack frame won't have that on the stack so instead we
use the owner stack and debug task of any halted async debug info.

One thing that's a bit awkward is that if you do have a sync abort and
you use that error as the "reason" then that thing still has a sync
stack in a different component. In another approach I was exploring
having different error objects for each component but I don't think
that's worth it.

DiffTrain build for [cee7939](cee7939)
unstubbable added a commit that referenced this pull request Jun 25, 2025
#33644)

This PR adds tests for the Node.js and Edge builds to verify that
component stacks and owner stacks of halted components appear as
expected, now that recent enhancements for those have been implemented
(the latest one being #33634).

---------

Co-authored-by: Sebastian "Sebbie" Silbermann <silbermann.sebastian@gmail.com>
github-actions bot pushed a commit that referenced this pull request Jun 25, 2025
#33644)

This PR adds tests for the Node.js and Edge builds to verify that
component stacks and owner stacks of halted components appear as
expected, now that recent enhancements for those have been implemented
(the latest one being #33634).

---------

Co-authored-by: Sebastian "Sebbie" Silbermann <silbermann.sebastian@gmail.com>

DiffTrain build for [9b2a545](9b2a545)
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.

7 participants