diff --git a/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts b/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts index 31e6c83eabbe..2a157ff659c6 100644 --- a/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts @@ -277,6 +277,33 @@ test('a hit carries the parameterized route that was on the scope before the fre expect(attr(hit, 'sentry.segment.name')).toBe('/users/:id'); }); +// A registered route provider is preferred over the scope, which is what lets a framework SDK name the +// segment even when nothing stamped a route on the scope. The scope holds a different name here to prove it. +test('a hit carries the route resolved by a registered route provider', async ({ page }) => { + const hitPromise = waitForMetric(PROXY_SERVER_NAME, metric => isNavigation(metric, 'hit')); + + await page.goto('/'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 1'); + + await page.evaluate(() => { + const { Sentry } = window as unknown as { Sentry: typeof import('@sentry/browser') }; + Sentry.getCurrentScope().setTransactionName('/from-scope'); + Sentry.setRouteProvider({ resolveRoute: () => '/users/:id', resolveCurrentRoute: () => '/users/:id' }); + }); + + await page.click('#to-page-2'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 2'); + await page.waitForTimeout(500); + + await page.evaluate(() => history.back()); + await page.waitForFunction(() => (window as unknown as { __bfcacheRestored?: boolean }).__bfcacheRestored === true, { + timeout: 5000, + }); + + const hit = await hitPromise; + expect(attr(hit, 'sentry.segment.name')).toBe('/users/:id'); +}); + // Without a routing integration the scope has no transaction name, so the segment name falls back to // `location.pathname` (page 1 is served at '/'). This matches how browserTracing names an unrouted pageload. test('a hit falls back to the raw pathname when no route is on the scope', async ({ page }) => { diff --git a/packages/browser-utils/src/index.ts b/packages/browser-utils/src/index.ts index 7c3b08c40e3b..52da63c3d6b6 100644 --- a/packages/browser-utils/src/index.ts +++ b/packages/browser-utils/src/index.ts @@ -29,6 +29,16 @@ export { isBotUserAgent } from './isBotUserAgent'; export { getLocationHref } from './getLocationHref'; +export { + createCachedRouteProvider, + createUrlRouteProvider, + getRouteProvider, + resolveCurrentRoute, + resolveRoute, + setRouteProvider, +} from './routing'; +export type { CachedRouteProvider, RouteProvider } from './routing'; + export { userTimingIntegration } from './performance/userTiming'; export { extractNetworkProtocol } from './performance/utils'; diff --git a/packages/browser-utils/src/routing.ts b/packages/browser-utils/src/routing.ts new file mode 100644 index 000000000000..6e8022cd6337 --- /dev/null +++ b/packages/browser-utils/src/routing.ts @@ -0,0 +1,179 @@ +import type { Client } from '@sentry/core'; +import { debug, getClient, LRUMap, parseStringToURLObject } from '@sentry/core'; +import { DEBUG_BUILD } from './debug-build'; +import { getLocationHref } from './getLocationHref'; + +/** The parts of a URL a route depends on. A `URL` satisfies it. */ +type RouteUrl = Pick; + +/** + * Resolves URLs to low-cardinality route names. + * + * Framework SDKs register one so that everything the SDK names after a route (span names, the scope's + * transaction name, metric and span segment attributes) gets the parameterized route instead of the raw + * URL, without each integration having to reach into the framework's router itself. + * + * A provider only answers "which route is this", never what the caller does with the answer. + */ +export interface RouteProvider { + /** + * Resolves a URL path template for a specific URL, e.g. `/users/42` -> `/users/:id`. + * + * Must return a path template, never a route identifier. Routers that name routes independently of + * their path (Vue Router's `route.name`, Ember's `posts.show`) have to return the matched path + * instead: callers set `url.template` from this, and an identifier is not a template. An SDK that + * wants to name its span after the identifier still can, on the span itself. + * + * Returns `undefined` when the URL matches no known route. Must answer for the URL it is given rather + * than for wherever the router currently is, so that callers can resolve a URL they captured earlier + * (a web vital reported after a soft navigation, for example). + */ + resolveRoute(url: RouteUrl): string | undefined; + + /** + * Resolves the route the app is currently on. + * + * Routers whose location lives in the address bar can delegate to `resolveRoute`, which is what + * {@link createUrlRouteProvider} does. Routers that keep their own location (memory and hash routers) + * have to answer from that location instead: for those, `location.href` is the unchanging shell URL + * and would bucket every route together. + */ + resolveCurrentRoute(): string | undefined; +} + +const CLIENT_ROUTE_PROVIDERS = new WeakMap(); + +/** + * Registers the route provider for a client, replacing any previously registered one, including the one + * passed as the `routeProvider` option. + * + * Prefer the `routeProvider` option where the provider is known at `init`: the pageload span is named + * while `browserTracingIntegration` sets up, so a provider registered after `init` can only rename it + * after the fact. + * + * A client holds one provider. An app running two routers (a framework migration, or a shell plus an + * island) registers twice and the last one wins, so the first router's routes stop resolving. + */ +export function setRouteProvider(provider: RouteProvider, client: Client | undefined = getClient()): void { + if (!client) { + DEBUG_BUILD && debug.warn('Cannot set a route provider without a client.'); + return; + } + + if (DEBUG_BUILD && getRouteProvider(client)) { + debug.warn( + 'A route provider is already registered for this client and will be replaced. Routes only the previous provider knows about will no longer resolve.', + ); + } + + CLIENT_ROUTE_PROVIDERS.set(client, provider); +} + +/** + * Returns the route provider registered for a client, falling back to its `routeProvider` option. + */ +export function getRouteProvider(client: Client | undefined = getClient()): RouteProvider | undefined { + if (!client) { + return undefined; + } + + return CLIENT_ROUTE_PROVIDERS.get(client) ?? (client.getOptions() as { routeProvider?: RouteProvider }).routeProvider; +} + +/** + * Resolves a URL to a low-cardinality route name, e.g. `/users/42` -> `/users/:id`. + * + * Returns `undefined` when no route provider is registered or the URL matches no route. Callers pick + * their own fallback, because the right one differs: a span name falls back to a low-cardinality + * constant, the scope's transaction name to the raw path. + */ +export function resolveRoute(url: string | URL, client: Client | undefined = getClient()): string | undefined { + const provider = getRouteProvider(client); + if (!provider) { + return undefined; + } + + const urlObject = typeof url === 'string' ? parseLocation(url) : url; + if (!urlObject) { + return undefined; + } + + return callProvider(() => provider.resolveRoute(urlObject)); +} + +/** + * Resolves the route the app is currently on. + * + * Returns `undefined` when no route provider is registered or the current location matches no route. + */ +export function resolveCurrentRoute(client: Client | undefined = getClient()): string | undefined { + const provider = getRouteProvider(client); + + return provider && callProvider(() => provider.resolveCurrentRoute()); +} + +/** + * Builds a {@link RouteProvider} for a router whose location is the browser's, which covers every + * router except memory and hash routers. + */ +export function createUrlRouteProvider(resolveRouteFromUrl: (url: RouteUrl) => string | undefined): RouteProvider { + return { + resolveRoute: resolveRouteFromUrl, + resolveCurrentRoute: () => { + const urlObject = parseLocation(getLocationHref()); + + return urlObject && resolveRouteFromUrl(urlObject); + }, + }; +} + +/** + * A {@link RouteProvider} that answers from routes it has been told about, rather than by matching. + */ +export interface CachedRouteProvider extends RouteProvider { + /** Records the route name a router reported for a path. Ignores empty values. */ + record(pathname: string | undefined, routeName: string | null | undefined): void; +} + +/** + * Builds a route provider for a router with no usable matcher, which can only report the route it is + * on as it gets there (SvelteKit's `page.route.id`, Solid Router's current matches). + * + * A URL the app has not visited resolves to `undefined`, which includes the first pageload until the + * router reports. Backed by an LRU so a long-lived app visiting many URLs can't grow it without end, + * and so routes that keep being resolved outlive ones passed through once. + */ +export function createCachedRouteProvider(maxEntries: number = 50): CachedRouteProvider { + const routeNames = new LRUMap(maxEntries); + + return { + ...createUrlRouteProvider(url => routeNames.get(url.pathname)), + record(pathname, routeName) { + if (pathname && routeName) { + routeNames.set(pathname, routeName); + } + }, + }; +} + +/** + * Parses up front so providers never have to, and resolves relative locations (which memory routers + * hand around) against the document. + */ +function parseLocation(url: string): RouteUrl | undefined { + // Without a document `getLocationHref()` is empty, which would otherwise parse as `/`. + return url ? parseStringToURLObject(url, getLocationHref() || undefined) : undefined; +} + +/** + * Route providers are framework code we don't control, so a throw must not take down whatever the SDK + * was naming. + */ +function callProvider(resolve: () => string | undefined): string | undefined { + try { + return resolve() || undefined; + } catch (error) { + DEBUG_BUILD && debug.warn('Route provider threw while resolving a route:', error); + return undefined; + } +} diff --git a/packages/browser-utils/test/routing.test.ts b/packages/browser-utils/test/routing.test.ts new file mode 100644 index 000000000000..fac88a352087 --- /dev/null +++ b/packages/browser-utils/test/routing.test.ts @@ -0,0 +1,289 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + createCachedRouteProvider, + createUrlRouteProvider, + resolveCurrentRoute, + getRouteProvider, + resolveRoute, + setRouteProvider, +} from '../src/routing'; +import type { RouteProvider } from '../src/routing'; +import { debug, getCurrentScope, GLOBAL_OBJ, setCurrentClient } from '@sentry/core'; +import { getDefaultClientOptions, TestClient } from './utils/TestClient'; + +function setLocationHref(href: string): void { + (GLOBAL_OBJ as { document?: unknown }).document = { location: { href } }; +} + +function makeClient(): TestClient { + const client = new TestClient(getDefaultClientOptions({ dsn: 'https://public@dsn.ingest.sentry.io/1337' })); + setCurrentClient(client); + client.init(); + + return client; +} + +describe('routing', () => { + let client: TestClient; + + beforeEach(() => { + client = makeClient(); + setLocationHref('https://example.com/users/42?q=1#frag'); + }); + + afterEach(() => { + delete (GLOBAL_OBJ as { document?: unknown }).document; + vi.restoreAllMocks(); + }); + + describe('without a registered provider', () => { + it('returns undefined rather than falling back to the raw path', () => { + expect(resolveRoute('https://example.com/users/42')).toBeUndefined(); + expect(resolveCurrentRoute()).toBeUndefined(); + expect(getRouteProvider()).toBeUndefined(); + }); + }); + + describe('resolveRoute', () => { + it('hands the provider a parsed URL so it never has to parse itself', () => { + const resolveSpy = vi.fn().mockReturnValue('/users/:id'); + setRouteProvider({ resolveRoute: resolveSpy, resolveCurrentRoute: () => undefined }); + + expect(resolveRoute('https://example.com/users/42?q=1')).toBe('/users/:id'); + expect(resolveSpy).toHaveBeenCalledWith(new URL('https://example.com/users/42?q=1')); + }); + + it('accepts a URL object as-is', () => { + const url = new URL('https://example.com/users/42'); + const resolveSpy = vi.fn().mockReturnValue('/users/:id'); + setRouteProvider({ resolveRoute: resolveSpy, resolveCurrentRoute: () => undefined }); + + expect(resolveRoute(url)).toBe('/users/:id'); + expect(resolveSpy).toHaveBeenCalledWith(url); + }); + + it('resolves a relative location against the document, which memory routers rely on', () => { + const resolveSpy = vi.fn().mockReturnValue('/users/:id'); + setRouteProvider({ resolveRoute: resolveSpy, resolveCurrentRoute: () => undefined }); + + resolveRoute('7'); + + expect(resolveSpy).toHaveBeenCalledWith(expect.objectContaining({ pathname: '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/users/7', search: '', hash: '' })); + }); + + it('resolves a URL the router has already navigated away from', () => { + setRouteProvider({ + resolveRoute: url => (url.pathname.startsWith('/posts/') ? '/posts/:slug' : undefined), + resolveCurrentRoute: () => '/users/:id', + }); + + expect(resolveRoute('https://example.com/posts/hello')).toBe('/posts/:slug'); + expect(resolveCurrentRoute()).toBe('/users/:id'); + }); + + it('returns undefined for an unparseable URL without calling the provider', () => { + const resolveSpy = vi.fn(); + setRouteProvider({ resolveRoute: resolveSpy, resolveCurrentRoute: () => undefined }); + + expect(resolveRoute('http://')).toBeUndefined(); + expect(resolveSpy).not.toHaveBeenCalled(); + }); + + it('normalizes an empty route name to undefined', () => { + setRouteProvider({ resolveRoute: () => '', resolveCurrentRoute: () => '' }); + + expect(resolveRoute('https://example.com/users/42')).toBeUndefined(); + expect(resolveCurrentRoute()).toBeUndefined(); + }); + }); + + describe('provider errors', () => { + it('swallows a throwing provider instead of taking down the caller', () => { + setRouteProvider({ + resolveRoute: () => { + throw new Error('router blew up'); + }, + resolveCurrentRoute: () => { + throw new Error('router blew up'); + }, + }); + + expect(resolveRoute('https://example.com/users/42')).toBeUndefined(); + expect(resolveCurrentRoute()).toBeUndefined(); + }); + }); + + describe('routeProvider option', () => { + function makeClientWithProvider(provider: RouteProvider): TestClient { + const optionClient = new TestClient({ ...getDefaultClientOptions(), routeProvider: provider } as ReturnType< + typeof getDefaultClientOptions + >); + setCurrentClient(optionClient); + optionClient.init(); + + return optionClient; + } + + it('resolves through the provider passed as an option', () => { + const optionClient = makeClientWithProvider({ + resolveRoute: () => '/users/:id', + resolveCurrentRoute: () => '/users/:id', + }); + + expect(getRouteProvider(optionClient)).toBeDefined(); + expect(resolveCurrentRoute(optionClient)).toBe('/users/:id'); + }); + + it('is replaced by a provider registered at runtime', () => { + vi.spyOn(debug, 'warn').mockImplementation(() => {}); + const optionClient = makeClientWithProvider({ + resolveRoute: () => '/option', + resolveCurrentRoute: () => '/option', + }); + + setRouteProvider({ resolveRoute: () => '/runtime', resolveCurrentRoute: () => '/runtime' }, optionClient); + + expect(resolveCurrentRoute(optionClient)).toBe('/runtime'); + }); + }); + + describe('setRouteProvider', () => { + it('scopes the provider to its client', () => { + const otherClient = new TestClient(getDefaultClientOptions()); + setRouteProvider({ resolveRoute: () => '/users/:id', resolveCurrentRoute: () => '/users/:id' }, client); + + expect(resolveCurrentRoute(client)).toBe('/users/:id'); + expect(resolveCurrentRoute(otherClient)).toBeUndefined(); + }); + + it('replaces a previously registered provider and warns', () => { + const warnSpy = vi.spyOn(debug, 'warn').mockImplementation(() => {}); + setRouteProvider({ resolveRoute: () => '/first', resolveCurrentRoute: () => '/first' }); + setRouteProvider({ resolveRoute: () => '/second', resolveCurrentRoute: () => '/second' }); + + expect(resolveCurrentRoute()).toBe('/second'); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it('does not warn when registering the first provider', () => { + const warnSpy = vi.spyOn(debug, 'warn').mockImplementation(() => {}); + setRouteProvider({ resolveRoute: () => '/first', resolveCurrentRoute: () => '/first' }); + + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('warns per client rather than globally', () => { + const otherClient = makeClient(); + const warnSpy = vi.spyOn(debug, 'warn').mockImplementation(() => {}); + const provider = { resolveRoute: () => '/users/:id', resolveCurrentRoute: () => '/users/:id' }; + + setRouteProvider(provider, client); + setRouteProvider(provider, otherClient); + + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('is a no-op when there is no current client', () => { + getCurrentScope().setClient(undefined); + const provider: RouteProvider = { resolveRoute: () => '/users/:id', resolveCurrentRoute: () => '/users/:id' }; + + expect(() => setRouteProvider(provider)).not.toThrow(); + expect(getRouteProvider()).toBeUndefined(); + expect(resolveCurrentRoute()).toBeUndefined(); + }); + }); + + describe('createCachedRouteProvider', () => { + it('resolves a path the router has reported', () => { + const provider = createCachedRouteProvider(); + provider.record('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/users/42', '/users/:id'); + setRouteProvider(provider); + + expect(resolveRoute('https://example.com/users/42')).toBe('/users/:id'); + expect(resolveCurrentRoute()).toBe('/users/:id'); + }); + + it('returns undefined for a path the router has not reported yet', () => { + const provider = createCachedRouteProvider(); + setRouteProvider(provider); + + expect(resolveRoute('https://example.com/users/42')).toBeUndefined(); + }); + + it('keeps other routes when re-recording one into a full cache', () => { + const provider = createCachedRouteProvider(2); + provider.record('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/users/42', '/users/:id'); + provider.record('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/posts/hello', '/posts/:slug'); + + provider.record('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/posts/hello', '/posts/:slug'); + + expect(provider.resolveRoute(new URL('https://example.com/posts/hello'))).toBe('/posts/:slug'); + expect(provider.resolveRoute(new URL('https://example.com/users/42'))).toBe('/users/:id'); + }); + + it('keeps resolving a URL the router has navigated away from', () => { + const provider = createCachedRouteProvider(); + provider.record('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/posts/hello', '/posts/:slug'); + provider.record('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/users/42', '/users/:id'); + setRouteProvider(provider); + + expect(resolveRoute('https://example.com/posts/hello')).toBe('/posts/:slug'); + }); + + it('ignores empty paths and route names', () => { + const provider = createCachedRouteProvider(); + provider.record(undefined, '/users/:id'); + provider.record('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/users/42', null); + setRouteProvider(provider); + + expect(resolveRoute('https://example.com/users/42')).toBeUndefined(); + }); + + it('evicts the oldest entry once the cache is full', () => { + const provider = createCachedRouteProvider(2); + provider.record('/a', '/a'); + provider.record('/b', '/b'); + provider.record('/c', '/c'); + setRouteProvider(provider); + + expect(resolveRoute('https://example.com/a')).toBeUndefined(); + expect(resolveRoute('https://example.com/b')).toBe('/b'); + expect(resolveRoute('https://example.com/c')).toBe('/c'); + }); + + it('keeps a recently resolved path alive past newer entries', () => { + const provider = createCachedRouteProvider(2); + provider.record('/a', '/a'); + provider.record('/b', '/b'); + setRouteProvider(provider); + + // Resolving `/a` makes it the most recently used, so `/b` is evicted instead. + expect(resolveRoute('https://example.com/a')).toBe('/a'); + provider.record('/c', '/c'); + + expect(resolveRoute('https://example.com/a')).toBe('/a'); + expect(resolveRoute('https://example.com/b')).toBeUndefined(); + }); + }); + + describe('createUrlRouteProvider', () => { + it('derives the current route from the document location', () => { + setRouteProvider(createUrlRouteProvider(url => (url.pathname === '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/users/42' ? '/users/:id' : undefined))); + + expect(resolveCurrentRoute()).toBe('/users/:id'); + }); + + it('returns undefined when the current location matches no route', () => { + setRouteProvider(createUrlRouteProvider(() => undefined)); + + expect(resolveCurrentRoute()).toBeUndefined(); + }); + + it('returns undefined when there is no document location to read', () => { + delete (GLOBAL_OBJ as { document?: unknown }).document; + setRouteProvider(createUrlRouteProvider(() => '/users/:id')); + + expect(resolveCurrentRoute()).toBeUndefined(); + }); + }); +}); diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 0221ca2d30ce..c0129c6a1d81 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -8,6 +8,7 @@ import type { SeverityLevel, } from '@sentry/core'; import type { BrowserClientReplayOptions } from '@sentry/core/browser'; +import type { RouteProvider } from '@sentry/browser-utils'; import { addAutoIpAddressToSession, applySdkMetadata, Client, getSDKSource } from '@sentry/core'; import { eventFromException, eventFromMessage } from './eventbuilder'; import { WINDOW } from './helpers'; @@ -76,6 +77,14 @@ type BrowserSpecificOptions = BrowserClientReplayOptions & * IMPORTANT: Only set this option to `true` while developing, not in production! */ spotlight?: boolean | string; + + /** + * Resolves URLs to low-cardinality route names, e.g. `/users/42` -> `/users/:id`. + * + * Framework SDKs set this for you. Set it yourself to name routes for a router the SDK has no + * integration for. `setRouteProvider` replaces it at runtime. + */ + routeProvider?: RouteProvider; }; /** * Configuration options for the Sentry Browser SDK. diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index d6ee777fb402..dc8bde1ec231 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -40,6 +40,15 @@ export { fetchStreamPerformanceIntegration } from './integrations/fetchStreamPer export { webVitalsIntegration } from './integrations/webVitals'; export { userTimingIntegration } from './integrations/usertiming'; export { bfcacheMetricsIntegration } from './integrations/bfcacheMetrics'; +export { + createCachedRouteProvider, + createUrlRouteProvider, + getRouteProvider, + resolveCurrentRoute, + resolveRoute, + setRouteProvider, +} from '@sentry/browser-utils'; +export type { CachedRouteProvider, RouteProvider } from '@sentry/browser-utils'; export { interactionsIntegration } from './integrations/interactions'; export type { RequestInstrumentationOptions } from './tracing/request'; diff --git a/packages/browser/src/integrations/bfcacheMetrics.ts b/packages/browser/src/integrations/bfcacheMetrics.ts index 5dd076c2783a..78610dc3b2a2 100644 --- a/packages/browser/src/integrations/bfcacheMetrics.ts +++ b/packages/browser/src/integrations/bfcacheMetrics.ts @@ -8,6 +8,7 @@ import { } from '@sentry/conventions/attributes'; import type { IntegrationFn, SpanAttributes } from '@sentry/core'; import { debug, defineIntegration, getCurrentScope, metrics } from '@sentry/core'; +import { resolveCurrentRoute } from '@sentry/browser-utils'; import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../helpers'; @@ -129,14 +130,20 @@ function _captureBFCacheReason({ reason, frame }: CollectedReason, routeName?: s } /** - * The segment name for a bfcache navigation, read from the scope rather than any span. + * The segment name for a bfcache navigation. * - * A hit restore is silent to tracing (no pageload span), but the frozen scope still holds the last - * transaction name a downstream SDK (Vue/React/etc.) set before the freeze, so we reuse that. On a miss the - * page reloads with a fresh scope, so this is the new pageload name. Falls back to the raw pathname when unset. + * A registered route provider is preferred because it is parameterized, which matters more here than + * elsewhere: this ends up as a metric dimension, where a raw URL is unbounded cardinality. + * + * Without one we fall back to the scope. A hit restore is silent to tracing (no pageload span), but the + * frozen scope still holds the last transaction name a downstream SDK (Vue/React/etc.) set before the + * freeze, so we reuse that. On a miss the page reloads with a fresh scope, so this is the new pageload + * name. Falls back to the raw pathname when unset. + * + * Exported for tests only. */ -function _getSegmentName(): string | undefined { - return getCurrentScope().getScopeData().transactionName || WINDOW.location?.pathname; +export function _getSegmentName(): string | undefined { + return resolveCurrentRoute() || getCurrentScope().getScopeData().transactionName || WINDOW.location?.pathname; } /** diff --git a/packages/browser/test/integrations/bfcacheMetrics.test.ts b/packages/browser/test/integrations/bfcacheMetrics.test.ts index 42b51dd58d14..74ec02ea9a66 100644 --- a/packages/browser/test/integrations/bfcacheMetrics.test.ts +++ b/packages/browser/test/integrations/bfcacheMetrics.test.ts @@ -1,8 +1,53 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { debug } from '@sentry/core'; -import { _collectNotRestoredReasons, _resolveMaxReasons } from '../../src/integrations/bfcacheMetrics'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { debug, getCurrentScope, setCurrentClient } from '@sentry/core'; +import { setRouteProvider } from '@sentry/browser-utils'; +import { BrowserClient } from '../../src/client'; +import { _collectNotRestoredReasons, _getSegmentName, _resolveMaxReasons } from '../../src/integrations/bfcacheMetrics'; +import { WINDOW } from '../../src/helpers'; +import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; describe('bfcacheMetricsIntegration', () => { + describe('_getSegmentName', () => { + beforeEach(() => { + getCurrentScope().setTransactionName(undefined); + const client = new BrowserClient(getDefaultBrowserClientOptions()); + setCurrentClient(client); + client.init(); + }); + + afterEach(() => { + delete (WINDOW as { location?: unknown }).location; + getCurrentScope().setTransactionName(undefined); + getCurrentScope().setClient(undefined); + }); + + it('prefers the parameterized route from a registered provider', () => { + getCurrentScope().setTransactionName('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/users/42'); + setRouteProvider({ resolveRoute: () => '/users/:id', resolveCurrentRoute: () => '/users/:id' }); + + expect(_getSegmentName()).toBe('/users/:id'); + }); + + it('falls back to the scope when no provider is registered', () => { + getCurrentScope().setTransactionName('/users/:id'); + + expect(_getSegmentName()).toBe('/users/:id'); + }); + + it('falls back to the scope when the provider matches no route', () => { + getCurrentScope().setTransactionName('/users/:id'); + setRouteProvider({ resolveRoute: () => undefined, resolveCurrentRoute: () => undefined }); + + expect(_getSegmentName()).toBe('/users/:id'); + }); + + it('falls back to the raw pathname when nothing else knows the route', () => { + (WINDOW as { location?: unknown }).location = { pathname: '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/users/42' }; + + expect(_getSegmentName()).toBe('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/users/42'); + }); + }); + describe('_resolveMaxReasons', () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/packages/core/src/utils/lru.ts b/packages/core/src/utils/lru.ts index 3158dff7d413..f0b000f22c5f 100644 --- a/packages/core/src/utils/lru.ts +++ b/packages/core/src/utils/lru.ts @@ -25,7 +25,10 @@ export class LRUMap { /** Insert an entry and evict an older entry if we've reached maxSize */ public set(key: K, value: V): void { - if (this._cache.size >= this._maxSize) { + // `Map.set` keeps an existing key in place, so it has to be removed to become the most recently used. + if (this._cache.has(key)) { + this._cache.delete(key); + } else if (this._cache.size >= this._maxSize) { // keys() returns an iterator in insertion order so keys().next() gives us the oldest key // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const nextKey = this._cache.keys().next().value!; diff --git a/packages/core/test/lib/utils/lru.test.ts b/packages/core/test/lib/utils/lru.test.ts index 5940f10684e1..1fd60b792f5a 100644 --- a/packages/core/test/lib/utils/lru.test.ts +++ b/packages/core/test/lib/utils/lru.test.ts @@ -27,6 +27,18 @@ describe('LRUMap', () => { expect(map.keys()).toEqual(['a', 'd', 'e']); }); + test('updates an existing entry without evicting another one', () => { + const map = new LRUMap(3); + map.set('a', '1'); + map.set('b', '2'); + map.set('c', '3'); + + map.set('b', '4'); + + expect(map.keys()).toEqual(['a', 'c', 'b']); + expect(map.get('b')).toEqual('4'); + }); + test('removes and returns entry', () => { const map = new LRUMap(3); map.set('a', '1');