-
Notifications
You must be signed in to change notification settings - Fork 48.8k
[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
Conversation
Comparing: b42341d...4ca194f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show |
// 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); | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
if (endTimeIdx > -1) { |
However, I do want to add some kind of marker in the track to log a halted entry.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
046b550
to
4ca194f
Compare
…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)
#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>
#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)
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.