From 488ffe0ce1ccd4101d40f0edbad8f69a59ac00f9 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Mon, 24 Aug 2026 15:37:21 -0400 Subject: [PATCH] feat(core): Add route provider API for parameterized route resolution Framework SDKs can register a provider that resolves a URL to a low-cardinality route name, so integrations stop each reaching for the route their own way. Wires up `bfcacheIntegration` as the first consumer: its segment name ends up as a metric dimension, where an unparameterized URL is unbounded cardinality. --- packages/browser/src/exports.ts | 10 + packages/browser/src/integrations/bfcache.ts | 20 +- .../browser/test/integrations/bfcache.test.ts | 50 +++- packages/core/src/browser-exports.ts | 10 + packages/core/src/routing.ts | 171 ++++++++++++ packages/core/test/lib/routing.test.ts | 247 ++++++++++++++++++ 6 files changed, 498 insertions(+), 10 deletions(-) create mode 100644 packages/core/src/routing.ts create mode 100644 packages/core/test/lib/routing.test.ts diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 5716c98db98c..541f4655b83f 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -83,6 +83,16 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, } from '@sentry/core/browser'; +export { + createCachedRouteProvider, + createUrlRouteProvider, + resolveCurrentRoute, + getRouteProvider, + resolveRoute, + setRouteProvider, +} from '@sentry/core/browser'; +export type { CachedRouteProvider, RouteProvider } from '@sentry/core/browser'; + export { WINDOW } from './helpers'; export { BrowserClient } from './client'; export { makeFetchTransport } from './transports/fetch'; diff --git a/packages/browser/src/integrations/bfcache.ts b/packages/browser/src/integrations/bfcache.ts index 81616cbaef9c..e418eeaf5a06 100644 --- a/packages/browser/src/integrations/bfcache.ts +++ b/packages/browser/src/integrations/bfcache.ts @@ -7,7 +7,7 @@ import { SENTRY_ORIGIN, } from '@sentry/conventions/attributes'; import type { IntegrationFn, SpanAttributes } from '@sentry/core/browser'; -import { debug, defineIntegration, getCurrentScope, metrics } from '@sentry/core/browser'; +import { debug, defineIntegration, getCurrentScope, metrics, resolveCurrentRoute } from '@sentry/core/browser'; import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../helpers'; @@ -127,14 +127,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/bfcache.test.ts b/packages/browser/test/integrations/bfcache.test.ts index 8ac43575f37d..e91b07d34031 100644 --- a/packages/browser/test/integrations/bfcache.test.ts +++ b/packages/browser/test/integrations/bfcache.test.ts @@ -1,8 +1,52 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { debug } from '@sentry/core/browser'; -import { _collectNotRestoredReasons, _resolveMaxReasons } from '../../src/integrations/bfcache'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { debug, getCurrentScope, setCurrentClient, setRouteProvider } from '@sentry/core/browser'; +import { BrowserClient } from '../../src/client'; +import { _collectNotRestoredReasons, _getSegmentName, _resolveMaxReasons } from '../../src/integrations/bfcache'; +import { WINDOW } from '../../src/helpers'; +import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; describe('bfcacheIntegration', () => { + 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('/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: '/users/42' }; + + expect(_getSegmentName()).toBe('/users/42'); + }); + }); + describe('_resolveMaxReasons', () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/packages/core/src/browser-exports.ts b/packages/core/src/browser-exports.ts index 24ccbb96c66b..bab6b8ea86dc 100644 --- a/packages/core/src/browser-exports.ts +++ b/packages/core/src/browser-exports.ts @@ -13,6 +13,16 @@ export { export { startIdleSpan } from './tracing/idleSpan'; +export { + createCachedRouteProvider, + createUrlRouteProvider, + resolveCurrentRoute, + getRouteProvider, + resolveRoute, + setRouteProvider, +} from './routing'; +export type { CachedRouteProvider, RouteProvider } from './routing'; + export { spanStreamingIntegration } from './integrations/browserSpanStreaming'; export { diff --git a/packages/core/src/routing.ts b/packages/core/src/routing.ts new file mode 100644 index 000000000000..b12d201d8776 --- /dev/null +++ b/packages/core/src/routing.ts @@ -0,0 +1,171 @@ +import type { Client } from './client'; +import { getClient } from './currentScopes'; +import { DEBUG_BUILD } from './debug-build'; +import { getLocationHref } from './utils/browser'; +import { LRUMap } from './utils/lru'; +import { debug } from './utils/debug-logger'; + +/** + * 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 route name for a specific URL, e.g. `/users/42` -> `/users/:id`. + * + * 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: URL): 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. + * + * Register during an integration's `setup` rather than `afterAllSetup`: the pageload span is named + * while `browserTracingIntegration` sets up, so a provider registered later 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 && CLIENT_ROUTE_PROVIDERS.has(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, if any. + */ +export function getRouteProvider(client: Client | undefined = getClient()): RouteProvider | undefined { + return client && CLIENT_ROUTE_PROVIDERS.get(client); +} + +/** + * 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' ? toURLObject(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: URL) => string | undefined): RouteProvider { + return { + resolveRoute: resolveRouteFromUrl, + resolveCurrentRoute: () => { + const urlObject = toURLObject(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); + } + }, + }; +} + +/** + * Normalizes to a real `URL` so providers never have to parse, and relative locations (which memory + * routers hand around) resolve against the document. + */ +function toURLObject(url: string): URL | undefined { + try { + return new URL(url, getLocationHref() || undefined); + } catch { + return 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/core/test/lib/routing.test.ts b/packages/core/test/lib/routing.test.ts new file mode 100644 index 000000000000..25d9d337cdc8 --- /dev/null +++ b/packages/core/test/lib/routing.test.ts @@ -0,0 +1,247 @@ +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 { getCurrentScope } from '../../src/currentScopes'; +import { debug } from '../../src/utils/debug-logger'; +import { setCurrentClient } from '../../src/sdk'; +import { GLOBAL_OBJ } from '../../src/utils/worldwide'; +import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; + +function setLocationHref(href: string): void { + (GLOBAL_OBJ as { document?: unknown }).document = { location: { href } }; +} + +function makeClient(): TestClient { + const client = new TestClient(getDefaultTestClientOptions({ 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('/users/7'); + + expect(resolveSpy).toHaveBeenCalledWith(new URL('https://example.com/users/7')); + }); + + 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('setRouteProvider', () => { + it('scopes the provider to its client', () => { + const otherClient = new TestClient(getDefaultTestClientOptions()); + 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('/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 resolving a URL the router has navigated away from', () => { + const provider = createCachedRouteProvider(); + provider.record('/posts/hello', '/posts/:slug'); + provider.record('/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('/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 === '/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(); + }); + }); +});