Skip to content

Commit

Permalink
Update logic to handle possible missing execution points in stacks
Browse files Browse the repository at this point in the history
  • Loading branch information
markerikson committed Nov 9, 2023
1 parent ee42b19 commit ca88571
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 5 deletions.
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/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

0 comments on commit ca88571

Please sign in to comment.