Skip to content

Commit

Permalink
Handle potential missing point stack execution points (#9885)
Browse files Browse the repository at this point in the history
* Bump protocol package

* Update logic to handle possible missing execution points in stacks
  • Loading branch information
markerikson committed Nov 9, 2023
1 parent 3b4a139 commit 14a3f1e
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 20 deletions.
2 changes: 1 addition & 1 deletion package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export const reactRenderQueuedJumpLocationCache: Cache<
earliestAppCodeFrame.index
);

if (!pointDescription) {
return undefined;
}

return {
location,
point: {
Expand Down
2 changes: 1 addition & 1 deletion packages/replay-next/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/replay-next/src/suspense/PointStackCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function updateMappedLocationForPointStackFrame(
frame: PointStackFrame
) {
updateMappedLocation(sources, frame.functionLocation);
if (frame.point.frame) {
if (frame.point?.frame) {
updateMappedLocation(sources, frame.point.frame);
}
}
Expand Down
11 changes: 10 additions & 1 deletion packages/replay-next/src/suspense/ResumeTargetCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,16 @@ export const resumeTargetCache = createCache<

if (frameIndex > 0) {
const pointStack = await pointStackCache.readAsync(0, frameIndex, replayClient, pausePoint);
currentPoint = pointStack[frameIndex].point.point;
const frame = pointStack[frameIndex];
if (!frame.point) {
// Avoid stepping in the top frame - we need to step in the _current_ frame.
// Note that it's unlikely that we _will_ hit this case.
// Per Josh, the most likely reason a stack frame would _not_ have an execution point
// is if there's no actual code inside, such as `class B extends A` where B has no constructor.
// We _could_ look for the next frame that _does_ have an execution point instead.
return;
}
currentPoint = frame.point.point;
}
}

Expand Down
19 changes: 16 additions & 3 deletions src/ui/suspense/frameCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export interface FormattedPointStackFrame {
source: Source;
functionLocation: Location;
executionLocation: Location;
point: PointDescription;
point?: PointDescription;
functionDetails?: FormattedEventListener;
}

Expand Down Expand Up @@ -119,7 +119,7 @@ export const formattedPointStackCache: Cache<
pointStack.map(async frame => {
updateMappedLocationForPointStackFrame(sourcesById, frame);
const functionLocation = getPreferredLocation(sourcesById, [], frame.functionLocation);
const executionLocation = getPreferredLocation(sourcesById, [], frame.point.frame ?? []);
const executionLocation = getPreferredLocation(sourcesById, [], frame.point?.frame ?? []);

const source = sourcesById.get(functionLocation.sourceId)!;

Expand Down Expand Up @@ -158,7 +158,20 @@ export const formattedPointStackCache: Cache<

let resultPoint = point;
if (earliestAppCodeFrame) {
resultPoint = earliestAppCodeFrame.point;
if (earliestAppCodeFrame.point) {
// We _want_ to go to this line of code at the time that it ran.
// Technically the backend might not return an execution point for this stack frame.
// Handle that (hopefully unlikely) case by still using the original point
// at the "current" part of the stack trace, which would mean that we'd be showing
// the earlier stack frame but be paused at the "current" time.
// In discussion with Josh, this situation should almost never happen.
// The most plausible reason to be missing an execution point is something like
// `class B extends A`, where `B` has no constructor.
// Realistically, we're looking at React or Redux user app logic here,
// so we should always have actual code and thus an execution point.
resultPoint = earliestAppCodeFrame.point;
}

const formattedFunction = await formatFunctionDetailsFromLocation(
replayClient,
"unknown",
Expand Down
19 changes: 6 additions & 13 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3665,17 +3665,10 @@ __metadata:
languageName: node
linkType: hard

"@replayio/protocol@npm:^0.60.0":
version: 0.60.0
resolution: "@replayio/protocol@npm:0.60.0"
checksum: 5b50218fda4357551006ee61076ba3fe3f24ad877a937d787dcd4cdd3deae21fcd84a1e1c75b23520c84d7281b5c75e9096b84fed3725d2e9fce63dca0e552f4
languageName: node
linkType: hard

"@replayio/protocol@npm:^0.62.0":
version: 0.62.0
resolution: "@replayio/protocol@npm:0.62.0"
checksum: 38b3c04d9e69ec3679a3e3bf2423527f59a3e72c3a64f1f2dd2449bb4459f117e156b650a0409fd199f888baec90409c2d78224e3b9a6f092494fdf63a5b58ca
"@replayio/protocol@npm:^0.63.0":
version: 0.63.0
resolution: "@replayio/protocol@npm:0.63.0"
checksum: 4e737fa07042bc23c7632a8adeaf2c449e1270675cf2eb988346853f71416f73b7b14a3507125a362567a6d71e661fc349a936172541dccc38b5b8aa6a07ec99
languageName: node
linkType: hard

Expand Down Expand Up @@ -14987,7 +14980,7 @@ __metadata:
"@next/bundle-analyzer": ^12.2.0
"@reduxjs/toolkit": ^1.8.4
"@replayio/overboard": ^0.4.1
"@replayio/protocol": ^0.62.0
"@replayio/protocol": ^0.63.0
"@replayio/react-devtools-inline": 4.28.6
"@replayio/replay": ^0.12.4
"@sentry/react": ^7.9.0
Expand Down Expand Up @@ -15292,7 +15285,7 @@ __metadata:
"@lezer/common_replay_next": "npm:@lezer/common@1.0.1"
"@lezer/highlight_replay_next": "npm:@lezer/highlight@1.1.1"
"@playwright/test": ^1.35.0
"@replayio/protocol": ^0.60.0
"@replayio/protocol": ^0.63.0
"@testing-library/jest-dom": ^5.16.3
"@testing-library/react": ^13.2.0
"@types/uuid": ^7.0.3
Expand Down

1 comment on commit 14a3f1e

@vercel
Copy link

@vercel vercel bot commented on 14a3f1e Nov 9, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

devtools – ./

devtools-git-main-recordreplay.vercel.app
app.replay.io
devtools-recordreplay.vercel.app

Please sign in to comment.