Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 7c622f8

Browse files
fix(react): Patch spanEnd for potentially cancelled lazy-route pageloads
1 parent 8e3afe2 commit 7c622f8

File tree

3 files changed

+291
-14
lines changed

3 files changed

+291
-14
lines changed

‎dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,3 +377,124 @@ test('Allows legitimate POP navigation (back/forward) after pageload completes',
377377
expect(backNavigationEvent.transaction).toBe('/');
378378
expect(backNavigationEvent.contexts?.trace?.op).toBe('navigation');
379379
});
380+
381+
test('Updates pageload transaction name correctly when span is cancelled early (document.hidden simulation)', async ({
382+
page,
383+
}) => {
384+
const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
385+
return (
386+
!!transactionEvent?.transaction &&
387+
transactionEvent.contexts?.trace?.op === 'pageload' &&
388+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
389+
);
390+
});
391+
392+
// Set up the page to simulate document.hidden before navigation
393+
await page.addInitScript(() => {
394+
// Wait a bit for Sentry to initialize and start the pageload span
395+
setTimeout(() => {
396+
// Override document.hidden to simulate tab switching
397+
Object.defineProperty(document, 'hidden', {
398+
configurable: true,
399+
get: function () {
400+
return true;
401+
},
402+
});
403+
404+
// Dispatch visibilitychange event to trigger the idle span cancellation logic
405+
document.dispatchEvent(new Event('visibilitychange'));
406+
}, 100); // Small delay to ensure the span has started
407+
});
408+
409+
// Navigate to the lazy route URL
410+
await page.goto('/lazy/inner/1/2/3');
411+
412+
const event = await transactionPromise;
413+
414+
// Verify the lazy route content eventually loads (even though span was cancelled early)
415+
const lazyRouteContent = page.locator('id=innermost-lazy-route');
416+
await expect(lazyRouteContent).toBeVisible();
417+
418+
// Validate that the transaction event has the correct parameterized route name
419+
// even though the span was cancelled early due to document.hidden
420+
expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
421+
expect(event.type).toBe('transaction');
422+
expect(event.contexts?.trace?.op).toBe('pageload');
423+
424+
// Check if the span was indeed cancelled (should have idle_span_finish_reason attribute)
425+
const idleSpanFinishReason = event.contexts?.trace?.data?.['sentry.idle_span_finish_reason'];
426+
if (idleSpanFinishReason) {
427+
// If the span was cancelled due to visibility change, verify it still got the right name
428+
expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason);
429+
}
430+
});
431+
432+
test('Updates navigation transaction name correctly when span is cancelled early (document.hidden simulation)', async ({
433+
page,
434+
}) => {
435+
// First go to home page
436+
await page.goto('/');
437+
438+
const navigationPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => {
439+
return (
440+
!!transactionEvent?.transaction &&
441+
transactionEvent.contexts?.trace?.op === 'navigation' &&
442+
transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId'
443+
);
444+
});
445+
446+
// Set up a listener to simulate document.hidden after clicking the navigation link
447+
await page.evaluate(() => {
448+
// Override document.hidden to simulate tab switching
449+
let hiddenValue = false;
450+
Object.defineProperty(document, 'hidden', {
451+
configurable: true,
452+
get: function () {
453+
return hiddenValue;
454+
},
455+
});
456+
457+
// Listen for clicks on the navigation link and simulate document.hidden shortly after
458+
document.addEventListener(
459+
'click',
460+
() => {
461+
setTimeout(() => {
462+
hiddenValue = true;
463+
// Dispatch visibilitychange event to trigger the idle span cancellation logic
464+
document.dispatchEvent(new Event('visibilitychange'));
465+
}, 50); // Small delay to ensure the navigation span has started
466+
},
467+
{ once: true },
468+
);
469+
});
470+
471+
// Click the navigation link to navigate to the lazy route
472+
const navigationLink = page.locator('id=navigation');
473+
await expect(navigationLink).toBeVisible();
474+
await navigationLink.click();
475+
476+
const event = await navigationPromise;
477+
478+
// Verify the lazy route content eventually loads (even though span was cancelled early)
479+
const lazyRouteContent = page.locator('id=innermost-lazy-route');
480+
await expect(lazyRouteContent).toBeVisible();
481+
482+
// Validate that the transaction event has the correct parameterized route name
483+
// even though the span was cancelled early due to document.hidden
484+
expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId');
485+
expect(event.type).toBe('transaction');
486+
expect(event.contexts?.trace?.op).toBe('navigation');
487+
488+
// Check if the span was indeed cancelled (should have cancellation_reason attribute or idle_span_finish_reason)
489+
const cancellationReason = event.contexts?.trace?.data?.['sentry.cancellation_reason'];
490+
const idleSpanFinishReason = event.contexts?.trace?.data?.['sentry.idle_span_finish_reason'];
491+
492+
// Verify that the span was cancelled due to document.hidden
493+
if (cancellationReason) {
494+
expect(cancellationReason).toBe('document.hidden');
495+
}
496+
497+
if (idleSpanFinishReason) {
498+
expect(['externalFinish', 'cancelled']).toContain(idleSpanFinishReason);
499+
}
500+
});

‎packages/react/src/reactrouter-compat-utils/instrumentation.tsx‎

Lines changed: 133 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -667,14 +667,19 @@ export function handleNavigation(opts: {
667667

668668
// Cross usage can result in multiple navigation spans being created without this check
669669
if (!isAlreadyInNavigationSpan) {
670-
startBrowserTracingNavigationSpan(client, {
670+
constnavigationSpan=startBrowserTracingNavigationSpan(client, {
671671
name,
672672
attributes: {
673673
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
674674
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
675675
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`,
676676
},
677677
});
678+
679+
// Patch navigation span to handle early cancellation (e.g., document.hidden)
680+
if (navigationSpan) {
681+
patchNavigationSpanEnd(navigationSpan, location, routes, basename, allRoutes);
682+
}
678683
}
679684
}
680685
}
@@ -727,27 +732,141 @@ function updatePageloadTransaction({
727732
: (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]);
728733

729734
if (branches) {
730-
let name,
731-
source: TransactionSource = 'url';
732-
733-
const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes);
734-
735-
if (isInDescendantRoute) {
736-
name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location));
737-
source = 'route';
738-
}
739-
740-
if (!isInDescendantRoute || !name) {
741-
[name, source] = getNormalizedName(routes, location, branches, basename);
742-
}
735+
const [name, source] = getTransactionNameAndSource(location, routes, branches, basename, allRoutes);
743736

744737
getCurrentScope().setTransactionName(name || '/');
745738

746739
if (activeRootSpan) {
747740
activeRootSpan.updateName(name);
748741
activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
742+
743+
// Patch span.end() to ensure we update the name one last time before the span is sent
744+
patchPageloadSpanEnd(activeRootSpan, location, routes, basename, allRoutes);
745+
}
746+
}
747+
}
748+
749+
/**
750+
* Extracts the transaction name and source from the route information.
751+
*/
752+
function getTransactionNameAndSource(
753+
location: Location,
754+
routes: RouteObject[],
755+
branches: RouteMatch[],
756+
basename: string | undefined,
757+
allRoutes: RouteObject[] | undefined,
758+
): [string, TransactionSource] {
759+
let name: string | undefined;
760+
let source: TransactionSource = 'url';
761+
762+
const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes);
763+
764+
if (isInDescendantRoute) {
765+
name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location));
766+
source = 'route';
767+
}
768+
769+
if (!isInDescendantRoute || !name) {
770+
[name, source] = getNormalizedName(routes, location, branches, basename);
771+
}
772+
773+
return [name || '/', source];
774+
}
775+
776+
/**
777+
* Patches the span.end() method to update the transaction name one last time before the span is sent.
778+
* This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading.
779+
*/
780+
function patchPageloadSpanEnd(
781+
span: Span,
782+
location: Location,
783+
routes: RouteObject[],
784+
basename: string | undefined,
785+
allRoutes: RouteObject[] | undefined,
786+
): void {
787+
const hasEndBeenPatched = (span as { __sentry_pageload_end_patched__?: boolean })?.__sentry_pageload_end_patched__;
788+
789+
if (hasEndBeenPatched || !span.end) {
790+
return;
791+
}
792+
793+
const originalEnd = span.end.bind(span);
794+
795+
span.end = function patchedEnd(...args) {
796+
// Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet)
797+
const spanJson = spanToJSON(span);
798+
const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
799+
if (currentSource !== 'route') {
800+
// Last chance to update the transaction name with the latest route info
801+
const branches = _matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[];
802+
803+
if (branches) {
804+
const [latestName, latestSource] = getTransactionNameAndSource(location, routes, branches, basename, allRoutes);
805+
806+
span.updateName(latestName);
807+
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, latestSource);
808+
}
749809
}
810+
811+
return originalEnd(...args);
812+
};
813+
814+
// Mark this span as having its end() method patched to prevent duplicate patching
815+
addNonEnumerableProperty(
816+
span as { __sentry_pageload_end_patched__?: boolean },
817+
'__sentry_pageload_end_patched__',
818+
true,
819+
);
820+
}
821+
822+
/**
823+
* Patches the navigation span.end() method to update the transaction name one last time before the span is sent.
824+
* This handles cases where the span is cancelled early (e.g., document.hidden) before lazy routes have finished loading.
825+
*/
826+
function patchNavigationSpanEnd(
827+
span: Span,
828+
location: Location,
829+
routes: RouteObject[],
830+
basename: string | undefined,
831+
allRoutes: RouteObject[] | undefined,
832+
): void {
833+
const hasEndBeenPatched = (span as { __sentry_navigation_end_patched__?: boolean })
834+
?.__sentry_navigation_end_patched__;
835+
836+
if (hasEndBeenPatched || !span.end) {
837+
return;
750838
}
839+
840+
const originalEnd = span.end.bind(span);
841+
842+
span.end = function patchedEnd(...args) {
843+
// Only update if the span source is not already 'route' (i.e., it hasn't been parameterized yet)
844+
const spanJson = spanToJSON(span);
845+
const currentSource = spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
846+
if (currentSource !== 'route') {
847+
// Last chance to update the transaction name with the latest route info
848+
const branches = _matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[];
849+
850+
if (branches) {
851+
const [name, source] = resolveRouteNameAndSource(location, routes, allRoutes || routes, branches, basename);
852+
853+
// Only update if we have a valid name and the span hasn't finished
854+
if (name && !spanJson.timestamp) {
855+
span.updateName(name);
856+
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
857+
}
858+
}
859+
}
860+
861+
return originalEnd(...args);
862+
};
863+
864+
// Mark this span as having its end() method patched to prevent duplicate patching
865+
addNonEnumerableProperty(
866+
span as { __sentry_navigation_end_patched__?: boolean },
867+
'__sentry_navigation_end_patched__',
868+
true,
869+
);
751870
}
752871

753872
// eslint-disable-next-line @typescript-eslint/no-explicit-any

‎packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,4 +140,41 @@ describe('reactrouter-compat-utils/instrumentation', () => {
140140
expect(typeof integration.afterAllSetup).toBe('function');
141141
});
142142
});
143+
144+
describe('span.end() patching for early cancellation', () => {
145+
it('should update transaction name when span.end() is called during cancellation', () => {
146+
const mockEnd = vi.fn();
147+
let patchedEnd: ((...args: any[]) => any) | null = null;
148+
149+
const updateNameMock = vi.fn();
150+
const setAttributeMock = vi.fn();
151+
152+
const testSpan = {
153+
updateName: updateNameMock,
154+
setAttribute: setAttributeMock,
155+
get end() {
156+
return patchedEnd || mockEnd;
157+
},
158+
set end(fn: (...args: any[]) => any) {
159+
patchedEnd = fn;
160+
},
161+
} as unknown as Span;
162+
163+
// Simulate the patching behavior
164+
const originalEnd = testSpan.end.bind(testSpan);
165+
(testSpan as any).end = function patchedEndFn(...args: any[]) {
166+
// This simulates what happens in the actual implementation
167+
updateNameMock('Updated Route');
168+
setAttributeMock('sentry.source', 'route');
169+
return originalEnd(...args);
170+
};
171+
172+
// Call the patched end
173+
testSpan.end(12345);
174+
175+
expect(updateNameMock).toHaveBeenCalledWith('Updated Route');
176+
expect(setAttributeMock).toHaveBeenCalledWith('sentry.source', 'route');
177+
expect(mockEnd).toHaveBeenCalledWith(12345);
178+
});
179+
});
143180
});

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /