diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a88fc83987f..7e6f8752691a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,48 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 10.8.0 + +### Important Changes + +- **feat(sveltekit): Add Compatibility for builtin SvelteKit Tracing ([#17423](https://github.com/getsentry/sentry-javascript/pull/17423))** + + This release makes the `@sentry/sveltekit` SDK compatible with SvelteKit's native [observability support](https://svelte.dev/docs/kit/observability) introduced in SvelteKit version `2.31.0`. + If you enable both, instrumentation and tracing, the SDK will now initialize early enough to set up additional instrumentation like database queries and it will pick up spans emitted from SvelteKit. + + We will follow up with docs how to set up the SDK soon. + For now, If you're on SvelteKit version `2.31.0` or newer, you can easily opt into the new feature: + + 1. Enable [experimental tracing and instrumentation support](https://svelte.dev/docs/kit/observability) in `svelte.config.js`: + 2. Move your `Sentry.init()` call from `src/hooks.server.(js|ts)` to the new `instrumentation.server.(js|ts)` file: + + ```ts + // instrumentation.server.ts + import * as Sentry from '@sentry/sveltekit'; + + Sentry.init({ + dsn: '...', + // rest of your config + }); + ``` + + The rest of your Sentry config in `hooks.server.ts` (`sentryHandle` and `handleErrorWithSentry`) should stay the same. + + If you prefer to stay on the hooks-file based config for now, the SDK will continue to work as previously. + + Thanks to the Svelte team and @elliott-with-the-longest-name-on-github for implementing observability support and for reviewing our PR! + +### Other Changes + +- fix(react): Avoid multiple name updates on navigation spans ([#17438](https://github.com/getsentry/sentry-javascript/pull/17438)) + +
+ Internal Changes + +- test(profiling): Add tests for current state of profiling ([#17470](https://github.com/getsentry/sentry-javascript/pull/17470)) + +
+ ## 10.7.0 ### Important Changes diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js new file mode 100644 index 000000000000..230e9ee1fb9e --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/subject.js @@ -0,0 +1,26 @@ +import * as Sentry from '@sentry/browser'; +import { browserProfilingIntegration } from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [browserProfilingIntegration()], + tracesSampleRate: 1, + profilesSampleRate: 1, +}); + +function fibonacci(n) { + if (n <= 1) { + return n; + } + return fibonacci(n - 1) + fibonacci(n - 2); +} + +await Sentry.startSpanManual({ name: 'root-fibonacci-2', parentSpan: null, forceTransaction: true }, async span => { + fibonacci(30); + + // Timeout to prevent flaky tests. Integration samples every 20ms, if function is too fast it might not get sampled + await new Promise(resolve => setTimeout(resolve, 21)); + span.end(); +}); diff --git a/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts new file mode 100644 index 000000000000..35f4e17bec0a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/profiling/legacyMode/test.ts @@ -0,0 +1,117 @@ +import { expect } from '@playwright/test'; +import type { Event, Profile } from '@sentry/core'; +import { sentryTest } from '../../../utils/fixtures'; +import { + properEnvelopeRequestParser, + shouldSkipTracingTest, + waitForTransactionRequestOnUrl, +} from '../../../utils/helpers'; + +sentryTest( + 'does not send profile envelope when document-policy is not set', + async ({ page, getLocalTestUrl, browserName }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + // Profiling only works when tracing is enabled + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const req = await waitForTransactionRequestOnUrl(page, url); + const transactionEvent = properEnvelopeRequestParser(req, 0); + const profileEvent = properEnvelopeRequestParser(req, 1); + + expect(transactionEvent).toBeDefined(); + expect(profileEvent).toBeUndefined(); + }, +); + +sentryTest('sends profile envelope in legacy mode', async ({ page, getLocalTestUrl, browserName }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + // Profiling only works when tracing is enabled + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname, responseHeaders: { 'Document-Policy': 'js-profiling' } }); + + const req = await waitForTransactionRequestOnUrl(page, url); + const profileEvent = properEnvelopeRequestParser(req, 1); + expect(profileEvent).toBeDefined(); + + const profile = profileEvent.profile; + expect(profileEvent.profile).toBeDefined(); + + expect(profile.samples).toBeDefined(); + expect(profile.stacks).toBeDefined(); + expect(profile.frames).toBeDefined(); + expect(profile.thread_metadata).toBeDefined(); + + // Samples + expect(profile.samples.length).toBeGreaterThanOrEqual(2); + for (const sample of profile.samples) { + expect(typeof sample.elapsed_since_start_ns).toBe('string'); + expect(sample.elapsed_since_start_ns).toMatch(/^\d+$/); // Numeric string + expect(parseInt(sample.elapsed_since_start_ns, 10)).toBeGreaterThanOrEqual(0); + + expect(typeof sample.stack_id).toBe('number'); + expect(sample.stack_id).toBeGreaterThanOrEqual(0); + expect(sample.thread_id).toBe('0'); // Should be main thread + } + + // Stacks + expect(profile.stacks.length).toBeGreaterThan(0); + for (const stack of profile.stacks) { + expect(Array.isArray(stack)).toBe(true); + for (const frameIndex of stack) { + expect(typeof frameIndex).toBe('number'); + expect(frameIndex).toBeGreaterThanOrEqual(0); + expect(frameIndex).toBeLessThan(profile.frames.length); + } + } + + // Frames + expect(profile.frames.length).toBeGreaterThan(0); + for (const frame of profile.frames) { + expect(frame).toHaveProperty('function'); + expect(frame).toHaveProperty('abs_path'); + expect(frame).toHaveProperty('lineno'); + expect(frame).toHaveProperty('colno'); + + expect(typeof frame.function).toBe('string'); + expect(typeof frame.abs_path).toBe('string'); + expect(typeof frame.lineno).toBe('number'); + expect(typeof frame.colno).toBe('number'); + } + + const functionNames = profile.frames.map(frame => frame.function).filter(name => name !== ''); + + if ((process.env.PW_BUNDLE || '').endsWith('min')) { + // Function names are minified in minified bundles + expect(functionNames.length).toBeGreaterThan(0); + expect((functionNames as string[]).every(name => name?.length > 0)).toBe(true); // Just make sure they're not empty strings + } else { + expect(functionNames).toEqual( + expect.arrayContaining([ + '_startRootSpan', + 'withScope', + 'createChildOrRootSpan', + 'startSpanManual', + 'startProfileForSpan', + 'startJSSelfProfile', + ]), + ); + } + + expect(profile.thread_metadata).toHaveProperty('0'); + expect(profile.thread_metadata['0']).toHaveProperty('name'); + expect(profile.thread_metadata['0'].name).toBe('main'); + + // Test that profile duration makes sense (should be > 20ms based on test setup) + const startTime = parseInt(profile.samples[0].elapsed_since_start_ns, 10); + const endTime = parseInt(profile.samples[profile.samples.length - 1].elapsed_since_start_ns, 10); + const durationNs = endTime - startTime; + const durationMs = durationNs / 1_000_000; // Convert ns to ms + + // Should be at least 20ms based on our setTimeout(21) in the test + expect(durationMs).toBeGreaterThan(20); +}); diff --git a/dev-packages/browser-integration-tests/utils/fixtures.ts b/dev-packages/browser-integration-tests/utils/fixtures.ts index adebce59edef..7cedc1e4001a 100644 --- a/dev-packages/browser-integration-tests/utils/fixtures.ts +++ b/dev-packages/browser-integration-tests/utils/fixtures.ts @@ -35,6 +35,7 @@ export type TestFixtures = { skipRouteHandler?: boolean; skipDsnRouteHandler?: boolean; handleLazyLoadedFeedback?: boolean; + responseHeaders?: Record; }) => Promise; forceFlushReplay: () => Promise; enableConsole: () => void; @@ -59,7 +60,13 @@ const sentryTest = base.extend({ getLocalTestUrl: ({ page }, use) => { return use( - async ({ testDir, skipRouteHandler = false, skipDsnRouteHandler = false, handleLazyLoadedFeedback = false }) => { + async ({ + testDir, + skipRouteHandler = false, + skipDsnRouteHandler = false, + handleLazyLoadedFeedback = false, + responseHeaders = {}, + }) => { const pagePath = `${TEST_HOST}/index.html`; const tmpDir = path.join(testDir, 'dist', crypto.randomUUID()); @@ -86,7 +93,9 @@ const sentryTest = base.extend({ const file = route.request().url().split('/').pop(); const filePath = path.resolve(tmpDir, `./${file}`); - return fs.existsSync(filePath) ? route.fulfill({ path: filePath }) : route.continue(); + return fs.existsSync(filePath) + ? route.fulfill({ path: filePath, headers: responseHeaders }) + : route.continue(); }); if (handleLazyLoadedFeedback) { diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index ff25097ccf50..bd505473f9b7 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -36,6 +36,7 @@ const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record = { feedbackIntegration: 'feedback', moduleMetadataIntegration: 'modulemetadata', graphqlClientIntegration: 'graphqlclient', + browserProfilingIntegration: 'browserprofiling', // technically, this is not an integration, but let's add it anyway for simplicity makeMultiplexedTransport: 'multiplexedtransport', }; diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index 5a9d8a351449..42657ab14731 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -29,6 +29,7 @@ export const envelopeParser = (request: Request | null): unknown[] => { }); }; +// Rather use the `properEnvelopeRequestParser`, as the `envelopeParser` does not follow the envelope spec. export const envelopeRequestParser = (request: Request | null, envelopeIndex = 2): T => { return envelopeParser(request)[envelopeIndex] as T; }; @@ -79,8 +80,12 @@ function getEventAndTraceHeader(envelope: EventEnvelope): EventAndTraceHeader { return [event, trace]; } -export const properEnvelopeRequestParser = (request: Request | null, envelopeIndex = 1): T => { - return properEnvelopeParser(request)[0]?.[envelopeIndex] as T; +export const properEnvelopeRequestParser = ( + request: Request | null, + envelopeItemIndex: number, + envelopeIndex = 1, // 1 is usually the payload of the envelope (0 is the header) +): T => { + return properEnvelopeParser(request)[envelopeItemIndex]?.[envelopeIndex] as T; }; export const properFullEnvelopeRequestParser = (request: Request | null): T => { diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx index 4570c23d06f5..c6c3cf74a3c7 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -50,6 +50,12 @@ const router = sentryCreateBrowserRouter( lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes), }, }, + { + path: '/another-lazy', + handle: { + lazyChildren: () => import('./pages/AnotherLazyRoutes').then(module => module.anotherNestedRoutes), + }, + }, { path: '/static', element: <>Hello World, diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/AnotherLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/AnotherLazyRoutes.tsx new file mode 100644 index 000000000000..9d6818a64398 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/AnotherLazyRoutes.tsx @@ -0,0 +1,41 @@ +import React from 'react'; +import { Link } from 'react-router-dom'; + +export const anotherNestedRoutes = [ + { + path: 'sub', + children: [ + { + index: true, + element: ( +
+ Another Lazy Route + + Navigate to Inner Lazy Route + +
+ ), + }, + { + path: ':id', + children: [ + { + index: true, + element:
Another Lazy Route with ID
, + }, + { + path: ':subId', + element: ( +
+ Another Deep Lazy Route + + Navigate to Inner from Deep + +
+ ), + }, + ], + }, + ], + }, +]; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx index e24b1b7cbff7..aefa39d63811 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx @@ -7,6 +7,14 @@ const Index = () => { navigate +
+ + Navigate to Another Lazy Route + +
+ + Navigate to Another Deep Lazy Route + ); }; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx index e8385e54dab5..42324bb09391 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { Link } from 'react-router-dom'; export const someMoreNestedRoutes = [ { @@ -24,9 +25,15 @@ export const someMoreNestedRoutes = [ }, { path: ':someAnotherId', - element:
- Rendered -
, + element: ( +
+ Rendered +
+ + Navigate to Another Lazy Route + +
+ ), }, ], }, diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts index 74a1fcff1faa..240e16a1cf95 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts @@ -54,3 +54,111 @@ test('Creates a navigation transaction inside a lazy route', async ({ page }) => expect(event.type).toBe('transaction'); expect(event.contexts?.trace?.op).toBe('navigation'); }); + +test('Creates navigation transactions between two different lazy routes', async ({ page }) => { + // First, navigate to the "another-lazy" route + const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/another-lazy/sub/:id/:subId' + ); + }); + + await page.goto('/'); + + // Navigate to another lazy route first + const navigationToAnotherDeep = page.locator('id=navigation-to-another-deep'); + await expect(navigationToAnotherDeep).toBeVisible(); + await navigationToAnotherDeep.click(); + + const firstEvent = await firstTransactionPromise; + + // Check if the first lazy route content is rendered + const anotherLazyContent = page.locator('id=another-lazy-route-deep'); + await expect(anotherLazyContent).toBeVisible(); + + // Validate the first transaction event + expect(firstEvent.transaction).toBe('/another-lazy/sub/:id/:subId'); + expect(firstEvent.type).toBe('transaction'); + expect(firstEvent.contexts?.trace?.op).toBe('navigation'); + + // Now navigate from the first lazy route to the second lazy route + const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + // Click the navigation link from within the first lazy route to the second lazy route + const navigationToInnerFromDeep = page.locator('id=navigate-to-inner-from-deep'); + await expect(navigationToInnerFromDeep).toBeVisible(); + await navigationToInnerFromDeep.click(); + + const secondEvent = await secondTransactionPromise; + + // Check if the second lazy route content is rendered + const innerLazyContent = page.locator('id=innermost-lazy-route'); + await expect(innerLazyContent).toBeVisible(); + + // Validate the second transaction event + expect(secondEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(secondEvent.type).toBe('transaction'); + expect(secondEvent.contexts?.trace?.op).toBe('navigation'); +}); + +test('Creates navigation transactions from inner lazy route to another lazy route', async ({ page }) => { + // First, navigate to the inner lazy route + const firstTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + await page.goto('/'); + + // Navigate to inner lazy route first + const navigationToInner = page.locator('id=navigation'); + await expect(navigationToInner).toBeVisible(); + await navigationToInner.click(); + + const firstEvent = await firstTransactionPromise; + + // Check if the inner lazy route content is rendered + const innerLazyContent = page.locator('id=innermost-lazy-route'); + await expect(innerLazyContent).toBeVisible(); + + // Validate the first transaction event + expect(firstEvent.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(firstEvent.type).toBe('transaction'); + expect(firstEvent.contexts?.trace?.op).toBe('navigation'); + + // Now navigate from the inner lazy route to another lazy route + const secondTransactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/another-lazy/sub/:id/:subId' + ); + }); + + // Click the navigation link from within the inner lazy route to another lazy route + const navigationToAnotherFromInner = page.locator('id=navigate-to-another-from-inner'); + await expect(navigationToAnotherFromInner).toBeVisible(); + await navigationToAnotherFromInner.click(); + + const secondEvent = await secondTransactionPromise; + + // Check if the another lazy route content is rendered + const anotherLazyContent = page.locator('id=another-lazy-route-deep'); + await expect(anotherLazyContent).toBeVisible(); + + // Validate the second transaction event + expect(secondEvent.transaction).toBe('/another-lazy/sub/:id/:subId'); + expect(secondEvent.type).toBe('transaction'); + expect(secondEvent.contexts?.trace?.op).toBe('navigation'); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/.gitignore b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/.gitignore new file mode 100644 index 000000000000..87c54ab857fc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/.gitignore @@ -0,0 +1,15 @@ +.DS_Store +node_modules +/build +/.svelte-kit +/package +.env +.env.* +!.env.example +vite.config.js.timestamp-* +vite.config.ts.timestamp-* + +#temporarily excluding my remote function routes. To be removed with the next PR: +./src/routes/remote-functions/data.remote.ts +./src/routes/remote-functions/+page.svelte +./tests/tracing.remote-functions.ts diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/.npmrc b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/README.md b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/README.md new file mode 100644 index 000000000000..684cabccfe02 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/README.md @@ -0,0 +1,41 @@ +# create-svelte + +Everything you need to build a Svelte project, powered by +[`create-svelte`](https://github.com/sveltejs/kit/tree/main/packages/create-svelte). + +## Creating a project + +If you're seeing this, you've probably already done this step. Congrats! + +```bash +# create a new project in the current directory +npm create svelte@latest + +# create a new project in my-app +npm create svelte@latest my-app +``` + +## Developing + +Once you've created a project and installed dependencies with `npm install` (or `pnpm install` or `yarn`), start a +development server: + +```bash +npm run dev + +# or start the server and open the app in a new browser tab +npm run dev -- --open +``` + +## Building + +To create a production version of your app: + +```bash +npm run build +``` + +You can preview the production build with `npm run preview`. + +> To deploy your app, you may need to install an [adapter](https://kit.svelte.dev/docs/adapters) for your target +> environment. diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/package.json b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/package.json new file mode 100644 index 000000000000..03e5441733da --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/package.json @@ -0,0 +1,37 @@ +{ + "name": "sveltekit-2-kit-tracing", + "version": "0.0.1", + "private": true, + "scripts": { + "dev": "vite dev", + "build": "vite build", + "preview": "vite preview", + "proxy": "node start-event-proxy.mjs", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json", + "check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch", + "test:prod": "TEST_ENV=production playwright test", + "test:build": "pnpm install && pnpm build", + "test:assert": "pnpm test:prod" + }, + "dependencies": { + "@sentry/sveltekit": "latest || *", + "@spotlightjs/spotlight": "2.0.0-alpha.1" + }, + "devDependencies": { + "@playwright/test": "~1.53.2", + "@sentry-internal/test-utils": "link:../../../test-utils", + "@sveltejs/adapter-node": "^5.3.1", + "@sveltejs/kit": "^2.31.0", + "@sveltejs/vite-plugin-svelte": "^6.1.3", + "svelte": "^5.38.3", + "svelte-check": "^4.3.1", + "tslib": "^2.4.1", + "typescript": "^5.0.0", + "vite": "^7.1.3" + }, + "type": "module", + "volta": { + "extends": "../../package.json" + } +} diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/playwright.config.mjs new file mode 100644 index 000000000000..6ae8142df247 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/playwright.config.mjs @@ -0,0 +1,8 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: 'ORIGIN=http://localhost:3030 node ./build/index.js', + port: 3030, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/app.d.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/app.d.ts new file mode 100644 index 000000000000..ede601ab93e2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/app.d.ts @@ -0,0 +1,13 @@ +// See https://kit.svelte.dev/docs/types#app +// for information about these interfaces +declare global { + namespace App { + // interface Error {} + // interface Locals {} + // interface PageData {} + // interface PageState {} + // interface Platform {} + } +} + +export {}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/app.html b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/app.html new file mode 100644 index 000000000000..77a5ff52c923 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/app.html @@ -0,0 +1,12 @@ + + + + + + + %sveltekit.head% + + +
%sveltekit.body%
+ + diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/hooks.client.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/hooks.client.ts new file mode 100644 index 000000000000..91592e7ab932 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/hooks.client.ts @@ -0,0 +1,23 @@ +import { env } from '$env/dynamic/public'; +import * as Sentry from '@sentry/sveltekit'; +import * as Spotlight from '@spotlightjs/spotlight'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: env.PUBLIC_E2E_TEST_DSN, + debug: !!env.PUBLIC_DEBUG, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, +}); + +const myErrorHandler = ({ error, event }: any) => { + console.error('An error occurred on the client side:', error, event); +}; + +export const handleError = Sentry.handleErrorWithSentry(myErrorHandler); + +if (import.meta.env.DEV) { + Spotlight.init({ + injectImmediately: true, + }); +} diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/hooks.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/hooks.server.ts new file mode 100644 index 000000000000..de32b0f9901b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/hooks.server.ts @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/sveltekit'; +import { setupSidecar } from '@spotlightjs/spotlight/sidecar'; +import { sequence } from '@sveltejs/kit/hooks'; + +// not logging anything to console to avoid noise in the test output +export const handleError = Sentry.handleErrorWithSentry(() => {}); + +export const handle = sequence(Sentry.sentryHandle()); + +if (import.meta.env.DEV) { + setupSidecar(); +} diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/instrumentation.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/instrumentation.server.ts new file mode 100644 index 000000000000..136b51a44dee --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/instrumentation.server.ts @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/sveltekit'; +import { E2E_TEST_DSN } from '$env/static/private'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: E2E_TEST_DSN, + debug: !!process.env.DEBUG, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, + spotlight: import.meta.env.DEV, +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/+layout.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/+layout.svelte new file mode 100644 index 000000000000..d1fadd2ea5a3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/+layout.svelte @@ -0,0 +1,15 @@ + + +

Sveltekit E2E Test app

+
+ +
diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/+page.svelte new file mode 100644 index 000000000000..95b18ea87844 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/+page.svelte @@ -0,0 +1,47 @@ +

Welcome to SvelteKit 2 with Svelte 5!

+

Visit kit.svelte.dev to read the documentation

+ + diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/api/users/+server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/api/users/+server.ts new file mode 100644 index 000000000000..d0e4371c594b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/api/users/+server.ts @@ -0,0 +1,3 @@ +export const GET = () => { + return new Response(JSON.stringify({ users: ['alice', 'bob', 'carol'] })); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.server.ts new file mode 100644 index 000000000000..b07376ba97c9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.server.ts @@ -0,0 +1,5 @@ +import type { PageServerLoad } from './$types'; + +export const load = (async _event => { + return { name: 'building (server)' }; +}) satisfies PageServerLoad; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.svelte new file mode 100644 index 000000000000..b27edb70053d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.svelte @@ -0,0 +1,21 @@ + + +

Check Build

+ +

+ This route only exists to check that Typescript definitions + and auto instrumentation are working when the project is built. +

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.ts new file mode 100644 index 000000000000..049acdc1fafa --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/building/+page.ts @@ -0,0 +1,5 @@ +import type { PageLoad } from './$types'; + +export const load = (async _event => { + return { name: 'building' }; +}) satisfies PageLoad; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/client-error/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/client-error/+page.svelte new file mode 100644 index 000000000000..ba6b464e9324 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/client-error/+page.svelte @@ -0,0 +1,9 @@ + + +

Client error

+ + diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/+page.svelte new file mode 100644 index 000000000000..eff3fa3f2e8d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/+page.svelte @@ -0,0 +1,15 @@ + +

Demonstrating Component Tracking

+ + + + diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component1.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component1.svelte new file mode 100644 index 000000000000..a675711e4b68 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component1.svelte @@ -0,0 +1,10 @@ + +

Howdy, I'm component 1

+ + diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component2.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component2.svelte new file mode 100644 index 000000000000..2b2f38308077 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component2.svelte @@ -0,0 +1,9 @@ + +

Howdy, I'm component 2

+ + diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component3.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component3.svelte new file mode 100644 index 000000000000..9b4e028f78e7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/components/Component3.svelte @@ -0,0 +1,6 @@ + + +

Howdy, I'm component 3

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/form-action/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/form-action/+page.server.ts new file mode 100644 index 000000000000..5efea8345851 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/form-action/+page.server.ts @@ -0,0 +1,7 @@ +export const actions = { + default: async ({ request }) => { + const formData = await request.formData(); + const name = formData.get('name'); + return { name }; + }, +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/form-action/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/form-action/+page.svelte new file mode 100644 index 000000000000..25b71a1c9fc7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/form-action/+page.svelte @@ -0,0 +1,15 @@ + + +
+ + + +
+ +{#if form?.name} +

Hello {form.name}

+{/if} diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/nav1/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/nav1/+page.svelte new file mode 100644 index 000000000000..31abffc512a2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/nav1/+page.svelte @@ -0,0 +1 @@ +

Navigation 1

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/nav2/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/nav2/+page.svelte new file mode 100644 index 000000000000..20b44bb32da9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/nav2/+page.svelte @@ -0,0 +1 @@ +

Navigation 2

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/redirect1/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/redirect1/+page.ts new file mode 100644 index 000000000000..3f462bf810fd --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/redirect1/+page.ts @@ -0,0 +1,5 @@ +import { redirect } from '@sveltejs/kit'; + +export const load = async () => { + redirect(301, '/redirect2'); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/redirect2/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/redirect2/+page.ts new file mode 100644 index 000000000000..99a810761d18 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/redirect2/+page.ts @@ -0,0 +1,5 @@ +import { redirect } from '@sveltejs/kit'; + +export const load = async () => { + redirect(301, '/users/789'); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-error/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-error/+page.server.ts new file mode 100644 index 000000000000..17dd53fb5bbb --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-error/+page.server.ts @@ -0,0 +1,6 @@ +export const load = async () => { + throw new Error('Server Load Error'); + return { + msg: 'Hello World', + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-error/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-error/+page.svelte new file mode 100644 index 000000000000..3a0942971d06 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-error/+page.svelte @@ -0,0 +1,9 @@ + + +

Server load error

+ +

+ Message: {data.msg} +

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-fetch/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-fetch/+page.server.ts new file mode 100644 index 000000000000..709e52bcf351 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-fetch/+page.server.ts @@ -0,0 +1,5 @@ +export const load = async ({ fetch }) => { + const res = await fetch('/api/users'); + const data = await res.json(); + return { data }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-fetch/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-fetch/+page.svelte new file mode 100644 index 000000000000..f7f814d31b4d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-load-fetch/+page.svelte @@ -0,0 +1,8 @@ + + +
+

Server Load Fetch

+

{JSON.stringify(data, null, 2)}

+
diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+page.svelte new file mode 100644 index 000000000000..3d682e7e3462 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+page.svelte @@ -0,0 +1,9 @@ + + +

Server Route error

+ +

+ Message: {data.msg} +

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+page.ts new file mode 100644 index 000000000000..298240827714 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+page.ts @@ -0,0 +1,7 @@ +export const load = async ({ fetch }) => { + const res = await fetch('/server-route-error'); + const data = await res.json(); + return { + msg: data, + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+server.ts new file mode 100644 index 000000000000..f1a4b94b7706 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/server-route-error/+server.ts @@ -0,0 +1,6 @@ +export const GET = async () => { + throw new Error('Server Route Error'); + return { + msg: 'Hello World', + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-error/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-error/+page.svelte new file mode 100644 index 000000000000..dc2d311a0ece --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-error/+page.svelte @@ -0,0 +1,17 @@ + + +

Universal load error

+ +

+ To trigger from client: Load on another route, then navigate to this route. +

+ +

+ To trigger from server: Load on this route +

+ +

+ Message: {data.msg} +

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-error/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-error/+page.ts new file mode 100644 index 000000000000..3d72bf4a890f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-error/+page.ts @@ -0,0 +1,8 @@ +import { browser } from '$app/environment'; + +export const load = async () => { + throw new Error(`Universal Load Error (${browser ? 'browser' : 'server'})`); + return { + msg: 'Hello World', + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-fetch/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-fetch/+page.svelte new file mode 100644 index 000000000000..563c51e8c850 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-fetch/+page.svelte @@ -0,0 +1,14 @@ + + +

Fetching in universal load

+ +

Here's a list of a few users:

+ +
    + {#each data.users as user} +
  • {user}
  • + {/each} +
diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-fetch/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-fetch/+page.ts new file mode 100644 index 000000000000..63c1ee68e1cb --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/universal-load-fetch/+page.ts @@ -0,0 +1,5 @@ +export const load = async ({ fetch }) => { + const usersRes = await fetch('/api/users'); + const data = await usersRes.json(); + return { users: data.users }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/+page.server.ts new file mode 100644 index 000000000000..a34c5450f682 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/+page.server.ts @@ -0,0 +1,5 @@ +export const load = async () => { + return { + msg: 'Hi everyone!', + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/+page.svelte new file mode 100644 index 000000000000..aa804a4518fa --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/+page.svelte @@ -0,0 +1,10 @@ + +

+ All Users: +

+ +

+ message: {data.msg} +

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/[id]/+page.server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/[id]/+page.server.ts new file mode 100644 index 000000000000..9388f3927018 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/[id]/+page.server.ts @@ -0,0 +1,5 @@ +export const load = async ({ params }) => { + return { + msg: `This is a special message for user ${params.id}`, + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/[id]/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/[id]/+page.svelte new file mode 100644 index 000000000000..d348a8c57dad --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/src/routes/users/[id]/+page.svelte @@ -0,0 +1,14 @@ + + +

Route with dynamic params

+ +

+ User id: {$page.params.id} +

+ +

+ Secret message for user: {data.msg} +

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/start-event-proxy.mjs new file mode 100644 index 000000000000..eea0add65374 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'sveltekit-2-kit-tracing', +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/static/favicon.png b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/static/favicon.png new file mode 100644 index 000000000000..825b9e65af7c Binary files /dev/null and b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/static/favicon.png differ diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/svelte.config.js b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/svelte.config.js new file mode 100644 index 000000000000..a31271021354 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/svelte.config.js @@ -0,0 +1,32 @@ +import adapter from '@sveltejs/adapter-node'; +import { vitePreprocess } from '@sveltejs/vite-plugin-svelte'; + +/** @type {import('@sveltejs/kit').Config} */ +const config = { + // Consult https://kit.svelte.dev/docs/integrations#preprocessors + // for more information about preprocessors + preprocess: vitePreprocess(), + + kit: { + // adapter-auto only supports some environments, see https://kit.svelte.dev/docs/adapter-auto for a list. + // If your environment is not supported, or you settled on a specific environment, switch out the adapter. + // See https://kit.svelte.dev/docs/adapters for more information about adapters. + adapter: adapter(), + experimental: { + instrumentation: { + server: true, + }, + tracing: { + server: true, + }, + remoteFunctions: true, + }, + }, + compilerOptions: { + experimental: { + async: true, // for remote functions + }, + }, +}; + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/errors.client.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/errors.client.test.ts new file mode 100644 index 000000000000..47c1e49b2b87 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/errors.client.test.ts @@ -0,0 +1,56 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; +import { waitForInitialPageload } from './utils'; + +test.describe('client-side errors', () => { + test('captures error thrown on click', async ({ page }) => { + await waitForInitialPageload(page, { route: '/client-error' }); + + const errorEventPromise = waitForError('sveltekit-2-kit-tracing', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Click Error'; + }); + + await page.getByText('Throw error').click(); + + await expect(errorEventPromise).resolves.toBeDefined(); + + const errorEvent = await errorEventPromise; + + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + expect(errorEventFrames?.[errorEventFrames?.length - 1]).toEqual( + expect.objectContaining({ + function: expect.stringContaining('HTMLButtonElement'), + lineno: 1, + in_app: true, + }), + ); + + expect(errorEvent.transaction).toEqual('/client-error'); + }); + + test('captures universal load error', async ({ page }) => { + await waitForInitialPageload(page); + await page.reload(); + + const errorEventPromise = waitForError('sveltekit-2-kit-tracing', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Universal Load Error (browser)'; + }); + + // navigating triggers the error on the client + await page.getByText('Universal Load error').click(); + + const errorEvent = await errorEventPromise; + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + const lastFrame = errorEventFrames?.[errorEventFrames?.length - 1]; + expect(lastFrame).toEqual( + expect.objectContaining({ + lineno: 1, + in_app: true, + }), + ); + + expect(errorEvent.transaction).toEqual('/universal-load-error'); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/errors.server.test.ts new file mode 100644 index 000000000000..e60531b43d6e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/errors.server.test.ts @@ -0,0 +1,91 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test.describe('server-side errors', () => { + test('captures universal load error', async ({ page }) => { + const errorEventPromise = waitForError('sveltekit-2-kit-tracing', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Universal Load Error (server)'; + }); + + await page.goto('/universal-load-error'); + + const errorEvent = await errorEventPromise; + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + expect(errorEventFrames?.[errorEventFrames?.length - 1]).toEqual( + expect.objectContaining({ + function: 'load', + in_app: true, + }), + ); + + expect(errorEvent.request).toEqual({ + cookies: {}, + headers: expect.objectContaining({ + accept: expect.any(String), + 'user-agent': expect.any(String), + }), + method: 'GET', + // SvelteKit's node adapter defaults to https in the protocol even if served on http + url: 'http://localhost:3030/universal-load-error', + }); + }); + + test('captures server load error', async ({ page }) => { + const errorEventPromise = waitForError('sveltekit-2-kit-tracing', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Server Load Error'; + }); + + await page.goto('/server-load-error'); + + const errorEvent = await errorEventPromise; + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + expect(errorEventFrames?.[errorEventFrames?.length - 1]).toEqual( + expect.objectContaining({ + function: 'load', + in_app: true, + }), + ); + + expect(errorEvent.request).toEqual({ + cookies: {}, + headers: expect.objectContaining({ + accept: expect.any(String), + 'user-agent': expect.any(String), + }), + method: 'GET', + url: 'http://localhost:3030/server-load-error', + }); + }); + + test('captures server route (GET) error', async ({ page }) => { + const errorEventPromise = waitForError('sveltekit-2-kit-tracing', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Server Route Error'; + }); + + await page.goto('/server-route-error'); + + const errorEvent = await errorEventPromise; + const errorEventFrames = errorEvent.exception?.values?.[0]?.stacktrace?.frames; + + expect(errorEventFrames?.[errorEventFrames?.length - 1]).toEqual( + expect.objectContaining({ + filename: expect.stringMatching(/app:\/\/\/_server.ts-.+.js/), + function: 'GET', + in_app: true, + }), + ); + + expect(errorEvent.transaction).toEqual('GET /server-route-error'); + + expect(errorEvent.request).toEqual({ + cookies: {}, + headers: expect.objectContaining({ + accept: expect.any(String), + }), + method: 'GET', + url: 'http://localhost:3030/server-route-error', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.client.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.client.test.ts new file mode 100644 index 000000000000..e9f4a9c1425f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.client.test.ts @@ -0,0 +1,115 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { waitForInitialPageload } from './utils'; + +test.describe('client-specific performance events', () => { + test('multiple navigations have distinct traces', async ({ page }) => { + const navigationTxn1EventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/nav1' && txnEvent.contexts?.trace?.op === 'navigation'; + }); + + const navigationTxn2EventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/' && txnEvent.contexts?.trace?.op === 'navigation'; + }); + + const navigationTxn3EventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/nav2' && txnEvent.contexts?.trace?.op === 'navigation'; + }); + + await waitForInitialPageload(page); + + await page.getByText('Nav 1').click(); + const navigationTxn1Event = await navigationTxn1EventPromise; + + await page.goBack(); + const navigationTxn2Event = await navigationTxn2EventPromise; + + await page.getByText('Nav 2').click(); + const navigationTxn3Event = await navigationTxn3EventPromise; + + expect(navigationTxn1Event).toMatchObject({ + transaction: '/nav1', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + }); + + expect(navigationTxn2Event).toMatchObject({ + transaction: '/', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + }); + + expect(navigationTxn3Event).toMatchObject({ + transaction: '/nav2', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + }, + }); + + // traces should NOT be connected + expect(navigationTxn1Event.contexts?.trace?.trace_id).not.toBe(navigationTxn2Event.contexts?.trace?.trace_id); + expect(navigationTxn2Event.contexts?.trace?.trace_id).not.toBe(navigationTxn3Event.contexts?.trace?.trace_id); + expect(navigationTxn1Event.contexts?.trace?.trace_id).not.toBe(navigationTxn3Event.contexts?.trace?.trace_id); + }); + + test('records manually added component tracking spans', async ({ page }) => { + const componentTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/components'; + }); + + await waitForInitialPageload(page); + + await page.getByText('Component Tracking').click(); + + const componentTxnEvent = await componentTxnEventPromise; + + expect(componentTxnEvent.spans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + data: { 'sentry.op': 'ui.svelte.init', 'sentry.origin': 'auto.ui.svelte' }, + description: '', + op: 'ui.svelte.init', + origin: 'auto.ui.svelte', + }), + expect.objectContaining({ + data: { 'sentry.op': 'ui.svelte.init', 'sentry.origin': 'auto.ui.svelte' }, + description: '', + op: 'ui.svelte.init', + origin: 'auto.ui.svelte', + }), + expect.objectContaining({ + data: { 'sentry.op': 'ui.svelte.init', 'sentry.origin': 'auto.ui.svelte' }, + description: '', + op: 'ui.svelte.init', + origin: 'auto.ui.svelte', + }), + expect.objectContaining({ + data: { 'sentry.op': 'ui.svelte.init', 'sentry.origin': 'auto.ui.svelte' }, + description: '', + op: 'ui.svelte.init', + origin: 'auto.ui.svelte', + }), + ]), + ); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.server.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.server.test.ts new file mode 100644 index 000000000000..c6de70d0e6a1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.server.test.ts @@ -0,0 +1,190 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/sveltekit'; + +test('server pageload request span has nested request span for sub request', async ({ page }) => { + const serverTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === 'GET /server-load-fetch'; + }); + + await page.goto('/server-load-fetch'); + + const serverTxnEvent = await serverTxnEventPromise; + const spans = serverTxnEvent.spans; + + expect(serverTxnEvent).toMatchObject({ + transaction: 'GET /server-load-fetch', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.sveltekit', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', + 'http.method': 'GET', + 'http.route': '/server-load-fetch', + 'sveltekit.tracing.original_name': 'sveltekit.handle.root', + }, + }, + }, + }); + + expect(spans).toHaveLength(6); + + expect(spans).toEqual( + expect.arrayContaining([ + // initial resolve span: + expect.objectContaining({ + data: expect.objectContaining({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.resolve', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', + 'http.route': '/server-load-fetch', + }), + op: 'function.sveltekit.resolve', + description: 'sveltekit.resolve', + origin: 'auto.http.sveltekit', + status: 'ok', + }), + + // sequenced handler span: + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.origin': 'auto.function.sveltekit.handle', + 'sentry.op': 'function.sveltekit.handle', + }), + description: 'sveltekit.handle.sequenced.sentryRequestHandler', + op: 'function.sveltekit.handle', + origin: 'auto.function.sveltekit.handle', + status: 'ok', + }), + + // load span where the server load function initiates the sub request: + expect.objectContaining({ + data: expect.objectContaining({ + 'http.route': '/server-load-fetch', + 'sentry.op': 'function.sveltekit.load', + 'sentry.origin': 'auto.function.sveltekit.load', + 'sveltekit.load.environment': 'server', + 'sveltekit.load.node_id': 'src/routes/server-load-fetch/+page.server.ts', + 'sveltekit.load.node_type': '+page.server', + }), + description: 'sveltekit.load', + op: 'function.sveltekit.load', + origin: 'auto.function.sveltekit.load', + status: 'ok', + }), + + // sub request http.server span: + expect.objectContaining({ + data: expect.objectContaining({ + 'http.method': 'GET', + 'http.route': '/api/users', + 'http.url': 'http://localhost:3030/api/users', + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.sveltekit', + 'sentry.source': 'route', + 'sveltekit.is_data_request': false, + 'sveltekit.is_sub_request': true, + 'sveltekit.tracing.original_name': 'sveltekit.handle.root', + url: 'http://localhost:3030/api/users', + }), + description: 'GET /api/users', + op: 'http.server', + origin: 'auto.http.sveltekit', + status: 'ok', + }), + + // sub requestsequenced handler span: + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.origin': 'auto.function.sveltekit.handle', + 'sentry.op': 'function.sveltekit.handle', + }), + description: 'sveltekit.handle.sequenced.sentryRequestHandler', + op: 'function.sveltekit.handle', + origin: 'auto.function.sveltekit.handle', + status: 'ok', + }), + + // sub request resolve span: + expect.objectContaining({ + data: expect.objectContaining({ + 'http.route': '/api/users', + 'sentry.op': 'function.sveltekit.resolve', + 'sentry.origin': 'auto.http.sveltekit', + }), + description: 'sveltekit.resolve', + op: 'function.sveltekit.resolve', + origin: 'auto.http.sveltekit', + status: 'ok', + }), + ]), + ); + + expect(serverTxnEvent.request).toEqual({ + cookies: {}, + headers: expect.objectContaining({ + accept: expect.any(String), + 'user-agent': expect.any(String), + }), + method: 'GET', + url: 'http://localhost:3030/server-load-fetch', + }); +}); + +test('server trace includes form action span', async ({ page }) => { + const serverTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === 'POST /form-action'; + }); + + await page.goto('/form-action'); + + await page.locator('#inputName').fill('H4cktor'); + await page.locator('#buttonSubmit').click(); + + const serverTxnEvent = await serverTxnEventPromise; + + expect(serverTxnEvent).toMatchObject({ + transaction: 'POST /form-action', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.sveltekit', + }, + }, + }); + + expect(serverTxnEvent.spans).toHaveLength(3); + + expect(serverTxnEvent.spans).toEqual( + expect.arrayContaining([ + // sequenced handler span + expect.objectContaining({ + description: 'sveltekit.handle.sequenced.sentryRequestHandler', + op: 'function.sveltekit.handle', + origin: 'auto.function.sveltekit.handle', + }), + + // resolve span + expect.objectContaining({ + description: 'sveltekit.resolve', + op: 'function.sveltekit.resolve', + origin: 'auto.http.sveltekit', + }), + + // form action span + expect.objectContaining({ + description: 'sveltekit.form_action', + op: 'function.sveltekit.form_action', + origin: 'auto.function.sveltekit.action', + data: expect.objectContaining({ + 'sveltekit.form_action.name': 'default', + }), + }), + ]), + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.test.ts new file mode 100644 index 000000000000..004182b32fb3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/tracing.test.ts @@ -0,0 +1,387 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { waitForInitialPageload } from './utils'; + +test('capture a distributed pageload trace', async ({ page }) => { + const clientTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/users/[id]'; + }); + + const serverTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === 'GET /users/[id]'; + }); + + const [_, clientTxnEvent, serverTxnEvent] = await Promise.all([ + page.goto('/users/123xyz'), + clientTxnEventPromise, + serverTxnEventPromise, + expect(page.getByText('User id: 123xyz')).toBeVisible(), + ]); + + expect(clientTxnEvent).toMatchObject({ + transaction: '/users/[id]', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.sveltekit', + }, + }, + }); + + expect(serverTxnEvent).toMatchObject({ + transaction: 'GET /users/[id]', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.sveltekit', + }, + }, + }); + + expect(clientTxnEvent.spans?.length).toBeGreaterThan(5); + + const serverKitResolveSpan = serverTxnEvent.spans?.find(s => s.description === 'sveltekit.resolve'); + expect(serverKitResolveSpan).toMatchObject({ + description: 'sveltekit.resolve', + op: 'function.sveltekit.resolve', + origin: 'auto.http.sveltekit', + status: 'ok', + }); + + // connected trace + expect(clientTxnEvent.contexts?.trace?.trace_id).toBe(serverTxnEvent.contexts?.trace?.trace_id); + + // Sveltekit resolve span is the parent span of the client span + expect(clientTxnEvent.contexts?.trace?.parent_span_id).toBe(serverKitResolveSpan?.span_id); +}); + +test('capture a distributed navigation trace', async ({ page }) => { + const clientNavigationTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/users' && txnEvent.contexts?.trace?.op === 'navigation'; + }); + + const serverTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === 'GET /users'; + }); + + await waitForInitialPageload(page); + + // navigation to page + const clickPromise = page.getByText('Route with Server Load').click(); + + const [clientTxnEvent, serverTxnEvent, _1, _2] = await Promise.all([ + clientNavigationTxnEventPromise, + serverTxnEventPromise, + clickPromise, + expect(page.getByText('Hi everyone')).toBeVisible(), + ]); + + expect(clientTxnEvent).toMatchObject({ + transaction: '/users', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + }, + }, + }); + + expect(serverTxnEvent).toMatchObject({ + transaction: 'GET /users', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.sveltekit', + }, + }, + }); + + // trace is connected + expect(clientTxnEvent.contexts?.trace?.trace_id).toBe(serverTxnEvent.contexts?.trace?.trace_id); +}); + +test('record client-side universal load fetch span and trace', async ({ page }) => { + await waitForInitialPageload(page); + + const clientNavigationTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === '/universal-load-fetch' && txnEvent.contexts?.trace?.op === 'navigation'; + }); + + // this transaction should be created because of the fetch call + // it should also be part of the trace + const serverTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.transaction === 'GET /api/users'; + }); + + // navigation to page + const clickPromise = page.getByText('Route with fetch in universal load').click(); + + const [clientTxnEvent, serverTxnEvent, _1, _2] = await Promise.all([ + clientNavigationTxnEventPromise, + serverTxnEventPromise, + clickPromise, + expect(page.getByText('alice')).toBeVisible(), + ]); + + expect(clientTxnEvent).toMatchObject({ + transaction: '/universal-load-fetch', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + }, + }, + }); + + expect(serverTxnEvent).toMatchObject({ + transaction: 'GET /api/users', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.sveltekit', + }, + }, + }); + + // trace is connected + expect(clientTxnEvent.contexts?.trace?.trace_id).toBe(serverTxnEvent.contexts?.trace?.trace_id); + + const clientFetchSpan = clientTxnEvent.spans?.find(s => s.op === 'http.client'); + + expect(clientFetchSpan).toMatchObject({ + description: expect.stringMatching(/^GET.*\/api\/users/), + op: 'http.client', + origin: 'auto.http.browser', + data: { + url: expect.stringContaining('/api/users'), + type: 'fetch', + 'http.method': 'GET', + 'http.response.status_code': 200, + 'network.protocol.version': '1.1', + 'network.protocol.name': 'http', + 'http.request.redirect_start': expect.any(Number), + 'http.request.fetch_start': expect.any(Number), + 'http.request.domain_lookup_start': expect.any(Number), + 'http.request.domain_lookup_end': expect.any(Number), + 'http.request.connect_start': expect.any(Number), + 'http.request.secure_connection_start': expect.any(Number), + 'http.request.connection_end': expect.any(Number), + 'http.request.request_start': expect.any(Number), + 'http.request.response_start': expect.any(Number), + 'http.request.response_end': expect.any(Number), + }, + }); +}); + +test('captures a navigation transaction directly after pageload', async ({ page }) => { + const clientPageloadTxnPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'pageload'; + }); + + const clientNavigationTxnPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'navigation'; + }); + + await waitForInitialPageload(page, { route: '/' }); + + const navigationClickPromise = page.locator('#routeWithParamsLink').click(); + + const [pageloadTxnEvent, navigationTxnEvent, _] = await Promise.all([ + clientPageloadTxnPromise, + clientNavigationTxnPromise, + navigationClickPromise, + ]); + + expect(pageloadTxnEvent).toMatchObject({ + transaction: '/', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.sveltekit', + }, + }, + }); + + expect(navigationTxnEvent).toMatchObject({ + transaction: '/users/[id]', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'link', + }, + }, + }, + }); + + const routingSpans = navigationTxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(routingSpans).toHaveLength(1); + + const routingSpan = routingSpans && routingSpans[0]; + expect(routingSpan).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'link', + }, + }); +}); + +test('captures one navigation transaction per redirect', async ({ page }) => { + const clientNavigationRedirect1TxnPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'navigation' && txnEvent?.transaction === '/redirect1'; + }); + + const clientNavigationRedirect2TxnPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'navigation' && txnEvent?.transaction === '/redirect2'; + }); + + const clientNavigationRedirect3TxnPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'navigation' && txnEvent?.transaction === '/users/[id]'; + }); + + await waitForInitialPageload(page, { route: '/' }); + + const navigationClickPromise = page.locator('#redirectLink').click(); + + const [redirect1TxnEvent, redirect2TxnEvent, redirect3TxnEvent, _] = await Promise.all([ + clientNavigationRedirect1TxnPromise, + clientNavigationRedirect2TxnPromise, + clientNavigationRedirect3TxnPromise, + navigationClickPromise, + ]); + + expect(redirect1TxnEvent).toMatchObject({ + transaction: '/redirect1', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.origin': 'auto.navigation.sveltekit', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + 'sentry.sveltekit.navigation.type': 'link', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect1', + 'sentry.sample_rate': 1, + }, + }, + }, + }); + + const redirect1Spans = redirect1TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(redirect1Spans).toHaveLength(1); + + const redirect1Span = redirect1Spans && redirect1Spans[0]; + expect(redirect1Span).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect1', + 'sentry.sveltekit.navigation.type': 'link', + }, + }); + + expect(redirect2TxnEvent).toMatchObject({ + transaction: '/redirect2', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.origin': 'auto.navigation.sveltekit', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + 'sentry.sveltekit.navigation.type': 'goto', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect2', + 'sentry.sample_rate': 1, + }, + }, + }, + }); + + const redirect2Spans = redirect2TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(redirect2Spans).toHaveLength(1); + + const redirect2Span = redirect2Spans && redirect2Spans[0]; + expect(redirect2Span).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect2', + 'sentry.sveltekit.navigation.type': 'goto', + }, + }); + + expect(redirect3TxnEvent).toMatchObject({ + transaction: '/users/[id]', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.origin': 'auto.navigation.sveltekit', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + 'sentry.sveltekit.navigation.type': 'goto', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sample_rate': 1, + }, + }, + }, + }); + + const redirect3Spans = redirect3TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(redirect3Spans).toHaveLength(1); + + const redirect3Span = redirect3Spans && redirect3Spans[0]; + expect(redirect3Span).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'goto', + }, + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/utils.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/utils.ts new file mode 100644 index 000000000000..a628f558a4bf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tests/utils.ts @@ -0,0 +1,49 @@ +import { Page } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +/** + * Helper function that waits for the initial pageload to complete. + * + * This function + * - loads the given route ("/" by default) + * - waits for SvelteKit's hydration + * - waits for the pageload transaction to be sent (doesn't assert on it though) + * + * Useful for tests that test outcomes of _navigations_ after an initial pageload. + * Waiting on the pageload transaction excludes edge cases where navigations occur + * so quickly that the pageload idle transaction is still active. This might lead + * to cases where the routing span would be attached to the pageload transaction + * and hence eliminates a lot of flakiness. + * + */ +export async function waitForInitialPageload( + page: Page, + opts?: { route?: string; parameterizedRoute?: string; debug?: boolean }, +) { + const route = opts?.route ?? '/'; + const txnName = opts?.parameterizedRoute ?? route; + const debug = opts?.debug ?? false; + + const clientPageloadTxnEventPromise = waitForTransaction('sveltekit-2-kit-tracing', txnEvent => { + debug && + console.log({ + txn: txnEvent?.transaction, + op: txnEvent.contexts?.trace?.op, + trace: txnEvent.contexts?.trace?.trace_id, + span: txnEvent.contexts?.trace?.span_id, + parent: txnEvent.contexts?.trace?.parent_span_id, + }); + + return txnEvent?.transaction === txnName && txnEvent.contexts?.trace?.op === 'pageload'; + }); + + await Promise.all([ + page.goto(route), + // the test app adds the "hydrated" class to the body when hydrating + page.waitForSelector('body.hydrated'), + // also waiting for the initial pageload txn so that later navigations don't interfere + clientPageloadTxnEventPromise, + ]); + + debug && console.log('hydrated'); +} diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tsconfig.json b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tsconfig.json new file mode 100644 index 000000000000..ba6aa4e6610a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/tsconfig.json @@ -0,0 +1,19 @@ +{ + "extends": "./.svelte-kit/tsconfig.json", + "compilerOptions": { + "allowJs": true, + "checkJs": true, + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "resolveJsonModule": true, + "skipLibCheck": true, + "sourceMap": true, + "strict": true, + "moduleResolution": "bundler" + }, + // Path aliases are handled by https://kit.svelte.dev/docs/configuration#alias + // except $lib which is handled by https://kit.svelte.dev/docs/configuration#files + // + // If you want to overwrite includes/excludes, make sure to copy over the relevant includes/excludes + // from the referenced tsconfig.json - TypeScript does not merge them in +} diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/vite.config.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/vite.config.ts new file mode 100644 index 000000000000..1a410bee7e11 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2-kit-tracing/vite.config.ts @@ -0,0 +1,12 @@ +import { sentrySvelteKit } from '@sentry/sveltekit'; +import { sveltekit } from '@sveltejs/kit/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + plugins: [ + sentrySvelteKit({ + autoUploadSourceMaps: false, + }), + sveltekit(), + ], +}); diff --git a/packages/react/src/reactrouter-compat-utils/index.ts b/packages/react/src/reactrouter-compat-utils/index.ts new file mode 100644 index 000000000000..94f879017c13 --- /dev/null +++ b/packages/react/src/reactrouter-compat-utils/index.ts @@ -0,0 +1,34 @@ +// These exports provide utility functions for React Router v6 compatibility (as of now v6 and v7) + +// Main exports from instrumentation +export type { ReactRouterOptions } from './instrumentation'; +export { + createReactRouterV6CompatibleTracingIntegration, + createV6CompatibleWithSentryReactRouterRouting, + createV6CompatibleWrapCreateBrowserRouter, + createV6CompatibleWrapCreateMemoryRouter, + createV6CompatibleWrapUseRoutes, + handleNavigation, + handleExistingNavigationSpan, + createNewNavigationSpan, + addResolvedRoutesToParent, + processResolvedRoutes, + updateNavigationSpan, +} from './instrumentation'; + +// Utility exports +export { + resolveRouteNameAndSource, + getNormalizedName, + initializeRouterUtils, + isLikelyLazyRouteContext, + locationIsInsideDescendantRoute, + prefixWithSlash, + rebuildRoutePathFromAllRoutes, + pathEndsWithWildcard, + pathIsWildcardAndHasChildren, + getNumberOfUrlSegments, +} from './utils'; + +// Lazy route exports +export { createAsyncHandlerProxy, handleAsyncHandlerResult, checkRouteForAsyncHandler } from './lazy-routes'; diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx similarity index 60% rename from packages/react/src/reactrouterv6-compat-utils.tsx rename to packages/react/src/reactrouter-compat-utils/instrumentation.tsx index f364facca1db..5012ed1ccc9c 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -22,8 +22,8 @@ import { spanToJSON, } from '@sentry/core'; import * as React from 'react'; -import { DEBUG_BUILD } from './debug-build'; -import { hoistNonReactStatics } from './hoist-non-react-statics'; +import { DEBUG_BUILD } from '../debug-build'; +import { hoistNonReactStatics } from '../hoist-non-react-statics'; import type { Action, AgnosticDataRouteMatch, @@ -39,18 +39,49 @@ import type { UseLocation, UseNavigationType, UseRoutes, -} from './types'; +} from '../types'; +import { checkRouteForAsyncHandler } from './lazy-routes'; +import { + getNormalizedName, + initializeRouterUtils, + isLikelyLazyRouteContext, + locationIsInsideDescendantRoute, + prefixWithSlash, + rebuildRoutePathFromAllRoutes, + resolveRouteNameAndSource, +} from './utils'; let _useEffect: UseEffect; let _useLocation: UseLocation; let _useNavigationType: UseNavigationType; let _createRoutesFromChildren: CreateRoutesFromChildren; let _matchRoutes: MatchRoutes; -let _stripBasename: boolean = false; let _enableAsyncRouteHandlers: boolean = false; const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet(); +/** + * Adds resolved routes as children to the parent route. + * Prevents duplicate routes by checking if they already exist. + */ +export function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { + const existingChildren = parentRoute.children || []; + + const newRoutes = resolvedRoutes.filter( + newRoute => + !existingChildren.some( + existing => + existing === newRoute || + (newRoute.path && existing.path === newRoute.path) || + (newRoute.id && existing.id === newRoute.id), + ), + ); + + if (newRoutes.length > 0) { + parentRoute.children = [...existingChildren, ...newRoutes]; + } +} + export interface ReactRouterOptions { useEffect: UseEffect; useLocation: UseLocation; @@ -78,61 +109,19 @@ type V6CompatibleVersion = '6' | '7'; // Keeping as a global variable for cross-usage in multiple functions const allRoutes = new Set(); -/** - * Adds resolved routes as children to the parent route. - * Prevents duplicate routes by checking if they already exist. - */ -function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { - const existingChildren = parentRoute.children || []; - - const newRoutes = resolvedRoutes.filter( - newRoute => - !existingChildren.some( - existing => - existing === newRoute || - (newRoute.path && existing.path === newRoute.path) || - (newRoute.id && existing.id === newRoute.id), - ), - ); - - if (newRoutes.length > 0) { - parentRoute.children = [...existingChildren, ...newRoutes]; - } -} - -/** - * Handles the result of an async handler function call. - */ -function handleAsyncHandlerResult(result: unknown, route: RouteObject, handlerKey: string): void { - if ( - result && - typeof result === 'object' && - 'then' in result && - typeof (result as Promise).then === 'function' - ) { - (result as Promise) - .then((resolvedRoutes: unknown) => { - if (Array.isArray(resolvedRoutes)) { - processResolvedRoutes(resolvedRoutes, route); - } - }) - .catch((e: unknown) => { - DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); - }); - } else if (Array.isArray(result)) { - processResolvedRoutes(result, route); - } -} - /** * Processes resolved routes by adding them to allRoutes and checking for nested async handlers. */ -function processResolvedRoutes(resolvedRoutes: RouteObject[], parentRoute?: RouteObject): void { +export function processResolvedRoutes( + resolvedRoutes: RouteObject[], + parentRoute?: RouteObject, + currentLocation: Location | null = null, +): void { resolvedRoutes.forEach(child => { allRoutes.add(child); // Only check for async handlers if the feature is enabled if (_enableAsyncRouteHandlers) { - checkRouteForAsyncHandler(child); + checkRouteForAsyncHandler(child, processResolvedRoutes); } }); @@ -141,63 +130,82 @@ function processResolvedRoutes(resolvedRoutes: RouteObject[], parentRoute?: Rout addResolvedRoutesToParent(resolvedRoutes, parentRoute); } - // After processing lazy routes, check if we need to update an active pageload transaction + // After processing lazy routes, check if we need to update an active transaction const activeRootSpan = getActiveRootSpan(); - if (activeRootSpan && spanToJSON(activeRootSpan).op === 'pageload') { - const location = WINDOW.location; + if (activeRootSpan) { + const spanOp = spanToJSON(activeRootSpan).op; + + // Try to use the provided location first, then fall back to global window location if needed + let location = currentLocation; + if (!location) { + if (typeof WINDOW !== 'undefined') { + const globalLocation = WINDOW.location; + if (globalLocation) { + location = { pathname: globalLocation.pathname }; + } + } + } + if (location) { - // Re-run the pageload transaction update with the newly loaded routes - updatePageloadTransaction( - activeRootSpan, - { pathname: location.pathname }, - Array.from(allRoutes), - undefined, - undefined, - Array.from(allRoutes), - ); + if (spanOp === 'pageload') { + // Re-run the pageload transaction update with the newly loaded routes + updatePageloadTransaction({ + activeRootSpan, + location: { pathname: location.pathname }, + routes: Array.from(allRoutes), + allRoutes: Array.from(allRoutes), + }); + } else if (spanOp === 'navigation') { + // For navigation spans, update the name with the newly loaded routes + updateNavigationSpan(activeRootSpan, location, Array.from(allRoutes), false, _matchRoutes); + } } } } /** - * Creates a proxy wrapper for an async handler function. - */ -function createAsyncHandlerProxy( - originalFunction: (...args: unknown[]) => unknown, - route: RouteObject, - handlerKey: string, -): (...args: unknown[]) => unknown { - const proxy = new Proxy(originalFunction, { - apply(target: (...args: unknown[]) => unknown, thisArg, argArray) { - const result = target.apply(thisArg, argArray); - handleAsyncHandlerResult(result, route, handlerKey); - return result; - }, - }); - - addNonEnumerableProperty(proxy, '__sentry_proxied__', true); - - return proxy; -} - -/** - * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. + * Updates a navigation span with the correct route name after lazy routes have been loaded. */ -export function checkRouteForAsyncHandler(route: RouteObject): void { - // Set up proxies for any functions in the route's handle - if (route.handle && typeof route.handle === 'object') { - for (const key of Object.keys(route.handle)) { - const maybeFn = route.handle[key]; - if (typeof maybeFn === 'function' && !(maybeFn as { __sentry_proxied__?: boolean }).__sentry_proxied__) { - route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key); +export function updateNavigationSpan( + activeRootSpan: Span, + location: Location, + allRoutes: RouteObject[], + forceUpdate = false, + matchRoutes: MatchRoutes, +): void { + // Check if this span has already been named to avoid multiple updates + // But allow updates if this is a forced update (e.g., when lazy routes are loaded) + const hasBeenNamed = + !forceUpdate && + ( + activeRootSpan as { + __sentry_navigation_name_set__?: boolean; } - } - } + )?.__sentry_navigation_name_set__; + + if (!hasBeenNamed) { + // Get fresh branches for the current location with all loaded routes + const currentBranches = matchRoutes(allRoutes, location); + const [name, source] = resolveRouteNameAndSource( + location, + allRoutes, + allRoutes, + (currentBranches as RouteMatch[]) || [], + '', + ); - // Recursively check child routes - if (Array.isArray(route.children)) { - for (const child of route.children) { - checkRouteForAsyncHandler(child); + // Only update if we have a valid name and the span hasn't finished + const spanJson = spanToJSON(activeRootSpan); + if (name && !spanJson.timestamp) { + activeRootSpan.updateName(name); + activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + + // Mark this span as having its name set to prevent future updates + addNonEnumerableProperty( + activeRootSpan as { __sentry_navigation_name_set__?: boolean }, + '__sentry_navigation_name_set__', + true, + ); } } } @@ -227,11 +235,14 @@ export function createV6CompatibleWrapCreateBrowserRouter< // Check for async handlers that might contain sub-route declarations (only if enabled) if (_enableAsyncRouteHandlers) { for (const route of routes) { - checkRouteForAsyncHandler(route); + checkRouteForAsyncHandler(route, processResolvedRoutes); } } - const router = createRouterFunction(routes, opts); + // Wrap patchRoutesOnNavigation to detect when lazy routes are loaded + const wrappedOpts = wrapPatchRoutesOnNavigation(opts); + + const router = createRouterFunction(routes, wrappedOpts); const basename = opts?.basename; const activeRootSpan = getActiveRootSpan(); @@ -240,14 +251,13 @@ export function createV6CompatibleWrapCreateBrowserRouter< // This is the earliest convenient time to update the transaction name. // Callbacks to `router.subscribe` are not called for the initial load. if (router.state.historyAction === 'POP' && activeRootSpan) { - updatePageloadTransaction( + updatePageloadTransaction({ activeRootSpan, - router.state.location, + location: router.state.location, routes, - undefined, basename, - Array.from(allRoutes), - ); + allRoutes: Array.from(allRoutes), + }); } router.subscribe((state: RouterState) => { @@ -313,11 +323,14 @@ export function createV6CompatibleWrapCreateMemoryRouter< // Check for async handlers that might contain sub-route declarations (only if enabled) if (_enableAsyncRouteHandlers) { for (const route of routes) { - checkRouteForAsyncHandler(route); + checkRouteForAsyncHandler(route, processResolvedRoutes); } } - const router = createRouterFunction(routes, opts); + // Wrap patchRoutesOnNavigation to detect when lazy routes are loaded + const wrappedOpts = wrapPatchRoutesOnNavigation(opts, true); + + const router = createRouterFunction(routes, wrappedOpts); const basename = opts?.basename; const activeRootSpan = getActiveRootSpan(); @@ -342,7 +355,13 @@ export function createV6CompatibleWrapCreateMemoryRouter< : router.state.location; if (router.state.historyAction === 'POP' && activeRootSpan) { - updatePageloadTransaction(activeRootSpan, location, routes, undefined, basename, Array.from(allRoutes)); + updatePageloadTransaction({ + activeRootSpan, + location, + routes, + basename, + allRoutes: Array.from(allRoutes), + }); } router.subscribe((state: RouterState) => { @@ -398,8 +417,10 @@ export function createReactRouterV6CompatibleTracingIntegration( _useNavigationType = useNavigationType; _matchRoutes = matchRoutes; _createRoutesFromChildren = createRoutesFromChildren; - _stripBasename = stripBasename || false; _enableAsyncRouteHandlers = enableAsyncRouteHandlers; + + // Initialize the router utils with the required dependencies + initializeRouterUtils(matchRoutes, stripBasename || false); }, afterAllSetup(client) { integration.afterAllSetup(client); @@ -457,14 +478,12 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio if (isMountRenderPass.current) { addRoutesToAllRoutes(routes); - updatePageloadTransaction( - getActiveRootSpan(), - normalizedLocation, + updatePageloadTransaction({ + activeRootSpan: getActiveRootSpan(), + location: normalizedLocation, routes, - undefined, - undefined, - Array.from(allRoutes), - ); + allRoutes: Array.from(allRoutes), + }); isMountRenderPass.current = false; } else { handleNavigation({ @@ -486,6 +505,79 @@ export function createV6CompatibleWrapUseRoutes(origUseRoutes: UseRoutes, versio }; } +function wrapPatchRoutesOnNavigation( + opts: Record | undefined, + isMemoryRouter = false, +): Record { + if (!opts || !('patchRoutesOnNavigation' in opts) || typeof opts.patchRoutesOnNavigation !== 'function') { + return opts || {}; + } + + const originalPatchRoutes = opts.patchRoutesOnNavigation; + return { + ...opts, + patchRoutesOnNavigation: async (args: unknown) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const targetPath = (args as any)?.path; + + // For browser router, wrap the patch function to update span during patching + if (!isMemoryRouter) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + const originalPatch = (args as any)?.patch; + if (originalPatch) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + (args as any).patch = (routeId: string, children: RouteObject[]) => { + addRoutesToAllRoutes(children); + const activeRootSpan = getActiveRootSpan(); + if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') { + updateNavigationSpan( + activeRootSpan, + { + pathname: targetPath, + search: '', + hash: '', + state: null, + key: 'default', + }, + Array.from(allRoutes), + true, // forceUpdate = true since we're loading lazy routes + _matchRoutes, + ); + } + return originalPatch(routeId, children); + }; + } + } + + const result = await originalPatchRoutes(args); + + // Update navigation span after routes are patched + const activeRootSpan = getActiveRootSpan(); + if (activeRootSpan && (spanToJSON(activeRootSpan) as { op?: string }).op === 'navigation') { + // For memory routers, we should not access window.location; use targetPath only + const pathname = isMemoryRouter ? targetPath : targetPath || WINDOW.location?.pathname; + if (pathname) { + updateNavigationSpan( + activeRootSpan, + { + pathname, + search: '', + hash: '', + state: null, + key: 'default', + }, + Array.from(allRoutes), + false, // forceUpdate = false since this is after lazy routes are loaded + _matchRoutes, + ); + } + } + + return result; + }, + }; +} + export function handleNavigation(opts: { location: Location; routes: RouteObject[]; @@ -504,107 +596,30 @@ export function handleNavigation(opts: { } if ((navigationType === 'PUSH' || navigationType === 'POP') && branches) { - let name, - source: TransactionSource = 'url'; - const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes || routes); - - if (isInDescendantRoute) { - name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes || routes, location)); - source = 'route'; - } + const [name, source] = resolveRouteNameAndSource( + location, + routes, + allRoutes || routes, + branches as RouteMatch[], + basename, + ); - if (!isInDescendantRoute || !name) { - [name, source] = getNormalizedName(routes, location, branches, basename); - } + // Check if this might be a lazy route context + const isLazyRouteContext = isLikelyLazyRouteContext(allRoutes || routes, location); const activeSpan = getActiveSpan(); - const isAlreadyInNavigationSpan = activeSpan && spanToJSON(activeSpan).op === 'navigation'; + const spanJson = activeSpan && spanToJSON(activeSpan); + const isAlreadyInNavigationSpan = spanJson?.op === 'navigation'; // Cross usage can result in multiple navigation spans being created without this check - if (isAlreadyInNavigationSpan) { - activeSpan?.updateName(name); - activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + if (isAlreadyInNavigationSpan && activeSpan && spanJson) { + handleExistingNavigationSpan(activeSpan, spanJson, name, source, isLazyRouteContext); } else { - startBrowserTracingNavigationSpan(client, { - name, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`, - }, - }); + createNewNavigationSpan(client, name, source, version, isLazyRouteContext); } } } -/** - * Strip the basename from a pathname if exists. - * - * Vendored and modified from `react-router` - * https://github.com/remix-run/react-router/blob/462bb712156a3f739d6139a0f14810b76b002df6/packages/router/utils.ts#L1038 - */ -function stripBasenameFromPathname(pathname: string, basename: string): string { - if (!basename || basename === '/') { - return pathname; - } - - if (!pathname.toLowerCase().startsWith(basename.toLowerCase())) { - return pathname; - } - - // We want to leave trailing slash behavior in the user's control, so if they - // specify a basename with a trailing slash, we should support it - const startIndex = basename.endsWith('/') ? basename.length - 1 : basename.length; - const nextChar = pathname.charAt(startIndex); - if (nextChar && nextChar !== '/') { - // pathname does not start with basename/ - return pathname; - } - - return pathname.slice(startIndex) || '/'; -} - -function sendIndexPath(pathBuilder: string, pathname: string, basename: string): [string, TransactionSource] { - const reconstructedPath = pathBuilder || _stripBasename ? stripBasenameFromPathname(pathname, basename) : pathname; - - const formattedPath = - // If the path ends with a slash, remove it - reconstructedPath[reconstructedPath.length - 1] === '/' - ? reconstructedPath.slice(0, -1) - : // If the path ends with a wildcard, remove it - reconstructedPath.slice(-2) === '/*' - ? reconstructedPath.slice(0, -1) - : reconstructedPath; - - return [formattedPath, 'route']; -} - -function pathEndsWithWildcard(path: string): boolean { - return path.endsWith('*'); -} - -function pathIsWildcardAndHasChildren(path: string, branch: RouteMatch): boolean { - return (pathEndsWithWildcard(path) && !!branch.route.children?.length) || false; -} - -function routeIsDescendant(route: RouteObject): boolean { - return !!(!route.children && route.element && route.path?.endsWith('/*')); -} - -function locationIsInsideDescendantRoute(location: Location, routes: RouteObject[]): boolean { - const matchedRoutes = _matchRoutes(routes, location) as RouteMatch[]; - - if (matchedRoutes) { - for (const match of matchedRoutes) { - if (routeIsDescendant(match.route) && pickSplat(match)) { - return true; - } - } - } - - return false; -} - function addRoutesToAllRoutes(routes: RouteObject[]): void { routes.forEach(route => { const extractedChildRoutes = getChildRoutesRecursively(route); @@ -633,126 +648,21 @@ function getChildRoutesRecursively(route: RouteObject, allRoutes: Set route !== match.route), - { - pathname: strippedPath, - }, - ), - ), - ); - } - } - - return ''; -} - -function getNormalizedName( - routes: RouteObject[], - location: Location, - branches: RouteMatch[], - basename: string = '', -): [string, TransactionSource] { - if (!routes || routes.length === 0) { - return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url']; - } - - let pathBuilder = ''; - - if (branches) { - for (const branch of branches) { - const route = branch.route; - if (route) { - // Early return if index route - if (route.index) { - return sendIndexPath(pathBuilder, branch.pathname, basename); - } - const path = route.path; - - // If path is not a wildcard and has no child routes, append the path - if (path && !pathIsWildcardAndHasChildren(path, branch)) { - const newPath = path[0] === '/' || pathBuilder[pathBuilder.length - 1] === '/' ? path : `/${path}`; - pathBuilder = trimSlash(pathBuilder) + prefixWithSlash(newPath); - - // If the path matches the current location, return the path - if (trimSlash(location.pathname) === trimSlash(basename + branch.pathname)) { - if ( - // If the route defined on the element is something like - // Product} /> - // We should check against the branch.pathname for the number of / separators - getNumberOfUrlSegments(pathBuilder) !== getNumberOfUrlSegments(branch.pathname) && - // We should not count wildcard operators in the url segments calculation - !pathEndsWithWildcard(pathBuilder) - ) { - return [(_stripBasename ? '' : basename) + newPath, 'route']; - } - - // if the last character of the pathbuilder is a wildcard and there are children, remove the wildcard - if (pathIsWildcardAndHasChildren(pathBuilder, branch)) { - pathBuilder = pathBuilder.slice(0, -1); - } - - return [(_stripBasename ? '' : basename) + pathBuilder, 'route']; - } - } - } - } - } - - const fallbackTransactionName = _stripBasename - ? stripBasenameFromPathname(location.pathname, basename) - : location.pathname || '/'; - - return [fallbackTransactionName, 'url']; -} - -function updatePageloadTransaction( - activeRootSpan: Span | undefined, - location: Location, - routes: RouteObject[], - matches?: AgnosticDataRouteMatch, - basename?: string, - allRoutes?: RouteObject[], -): void { +function updatePageloadTransaction({ + activeRootSpan, + location, + routes, + matches, + basename, + allRoutes, +}: { + activeRootSpan: Span | undefined; + location: Location; + routes: RouteObject[]; + matches?: AgnosticDataRouteMatch; + basename?: string; + allRoutes?: RouteObject[]; +}): void { const branches = Array.isArray(matches) ? matches : (_matchRoutes(allRoutes || routes, location, basename) as unknown as RouteMatch[]); @@ -808,7 +718,12 @@ export function createV6CompatibleWithSentryReactRouterRouting

s.length > 0 && s !== ',').length; +export function handleExistingNavigationSpan( + activeSpan: Span, + spanJson: ReturnType, + name: string, + source: TransactionSource, + isLikelyLazyRoute: boolean, +): void { + // Check if we've already set the name for this span using a custom property + const hasBeenNamed = ( + activeSpan as { + __sentry_navigation_name_set__?: boolean; + } + )?.__sentry_navigation_name_set__; + + if (!hasBeenNamed) { + // This is the first time we're setting the name for this span + if (!spanJson.timestamp) { + activeSpan?.updateName(name); + } + + // For lazy routes, don't mark as named yet so it can be updated later + if (!isLikelyLazyRoute) { + addNonEnumerableProperty( + activeSpan as { __sentry_navigation_name_set__?: boolean }, + '__sentry_navigation_name_set__', + true, + ); + } + } + + // Always set the source attribute to keep it consistent with the current route + activeSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); +} + +/** + * Creates a new navigation span + */ +export function createNewNavigationSpan( + client: Client, + name: string, + source: TransactionSource, + version: string, + isLikelyLazyRoute: boolean, +): void { + const newSpan = startBrowserTracingNavigationSpan(client, { + name, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.reactrouter_v${version}`, + }, + }); + + // For lazy routes, don't mark as named yet so it can be updated later when the route loads + if (!isLikelyLazyRoute && newSpan) { + addNonEnumerableProperty( + newSpan as { __sentry_navigation_name_set__?: boolean }, + '__sentry_navigation_name_set__', + true, + ); + } } diff --git a/packages/react/src/reactrouter-compat-utils/lazy-routes.tsx b/packages/react/src/reactrouter-compat-utils/lazy-routes.tsx new file mode 100644 index 000000000000..49923854554c --- /dev/null +++ b/packages/react/src/reactrouter-compat-utils/lazy-routes.tsx @@ -0,0 +1,74 @@ +import { addNonEnumerableProperty, debug, isThenable } from '@sentry/core'; +import { DEBUG_BUILD } from '../debug-build'; +import type { Location, RouteObject } from '../types'; + +/** + * Creates a proxy wrapper for an async handler function. + */ +export function createAsyncHandlerProxy( + originalFunction: (...args: unknown[]) => unknown, + route: RouteObject, + handlerKey: string, + processResolvedRoutes: (resolvedRoutes: RouteObject[], parentRoute?: RouteObject, currentLocation?: Location) => void, +): (...args: unknown[]) => unknown { + const proxy = new Proxy(originalFunction, { + apply(target: (...args: unknown[]) => unknown, thisArg, argArray) { + const result = target.apply(thisArg, argArray); + handleAsyncHandlerResult(result, route, handlerKey, processResolvedRoutes); + return result; + }, + }); + + addNonEnumerableProperty(proxy, '__sentry_proxied__', true); + + return proxy; +} + +/** + * Handles the result of an async handler function call. + */ +export function handleAsyncHandlerResult( + result: unknown, + route: RouteObject, + handlerKey: string, + processResolvedRoutes: (resolvedRoutes: RouteObject[], parentRoute?: RouteObject, currentLocation?: Location) => void, +): void { + if (isThenable(result)) { + (result as Promise) + .then((resolvedRoutes: unknown) => { + if (Array.isArray(resolvedRoutes)) { + processResolvedRoutes(resolvedRoutes, route); + } + }) + .catch((e: unknown) => { + DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); + }); + } else if (Array.isArray(result)) { + processResolvedRoutes(result, route); + } +} + +/** + * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. + */ +export function checkRouteForAsyncHandler( + route: RouteObject, + processResolvedRoutes: (resolvedRoutes: RouteObject[], parentRoute?: RouteObject, currentLocation?: Location) => void, +): void { + // Set up proxies for any functions in the route's handle + if (route.handle && typeof route.handle === 'object') { + for (const key of Object.keys(route.handle)) { + const maybeFn = route.handle[key]; + if (typeof maybeFn === 'function' && !(maybeFn as { __sentry_proxied__?: boolean }).__sentry_proxied__) { + route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key, processResolvedRoutes); + } + } + } + + // Recursively check child routes + if (Array.isArray(route.children)) { + for (const child of route.children) { + checkRouteForAsyncHandler(child, processResolvedRoutes); + } + } +} diff --git a/packages/react/src/reactrouter-compat-utils/utils.ts b/packages/react/src/reactrouter-compat-utils/utils.ts new file mode 100644 index 000000000000..8f7abb7d548e --- /dev/null +++ b/packages/react/src/reactrouter-compat-utils/utils.ts @@ -0,0 +1,286 @@ +import type { TransactionSource } from '@sentry/core'; +import type { Location, MatchRoutes, RouteMatch, RouteObject } from '../types'; + +// Global variables that these utilities depend on +let _matchRoutes: MatchRoutes; +let _stripBasename: boolean = false; + +/** + * Initialize function to set dependencies that the router utilities need. + * Must be called before using any of the exported utility functions. + */ +export function initializeRouterUtils(matchRoutes: MatchRoutes, stripBasename: boolean = false): void { + _matchRoutes = matchRoutes; + _stripBasename = stripBasename; +} + +/** + * Checks if the given routes or location context suggests this might be a lazy route scenario. + * This helps determine if we should delay marking navigation spans as "named" to allow for updates + * when lazy routes are loaded. + */ +export function isLikelyLazyRouteContext(routes: RouteObject[], location: Location): boolean { + // Check if any route in the current match has lazy properties + const hasLazyRoute = routes.some(route => { + return ( + // React Router lazy() route + route.lazy || + // Route with async handlers that might load child routes + (route.handle && + typeof route.handle === 'object' && + Object.values(route.handle).some(handler => typeof handler === 'function')) + ); + }); + + if (hasLazyRoute) { + return true; + } + + // Check if current route is unmatched, which might indicate a lazy route that hasn't loaded yet + const currentMatches = _matchRoutes(routes, location); + if (!currentMatches || currentMatches.length === 0) { + return true; + } + + return false; +} + +// Helper functions +function pickPath(match: RouteMatch): string { + return trimWildcard(match.route.path || ''); +} + +function pickSplat(match: RouteMatch): string { + return match.params['*'] || ''; +} + +function trimWildcard(path: string): string { + return path[path.length - 1] === '*' ? path.slice(0, -1) : path; +} + +function trimSlash(path: string): string { + return path[path.length - 1] === '/' ? path.slice(0, -1) : path; +} + +/** + * Checks if a path ends with a wildcard character (*). + */ +export function pathEndsWithWildcard(path: string): boolean { + return path.endsWith('*'); +} + +/** + * Checks if a path is a wildcard and has child routes. + */ +export function pathIsWildcardAndHasChildren(path: string, branch: RouteMatch): boolean { + return (pathEndsWithWildcard(path) && !!branch.route.children?.length) || false; +} + +function routeIsDescendant(route: RouteObject): boolean { + return !!(!route.children && route.element && route.path?.endsWith('/*')); +} + +function sendIndexPath(pathBuilder: string, pathname: string, basename: string): [string, TransactionSource] { + const reconstructedPath = pathBuilder || _stripBasename ? stripBasenameFromPathname(pathname, basename) : pathname; + + const formattedPath = + // If the path ends with a slash, remove it + reconstructedPath[reconstructedPath.length - 1] === '/' + ? reconstructedPath.slice(0, -1) + : // If the path ends with a wildcard, remove it + reconstructedPath.slice(-2) === '/*' + ? reconstructedPath.slice(0, -1) + : reconstructedPath; + + return [formattedPath, 'route']; +} + +/** + * Returns the number of URL segments in the given URL string. + * Splits at '/' or '\/' to handle regex URLs correctly. + * + * @param url - The URL string to segment. + * @returns The number of segments in the URL. + */ +export function getNumberOfUrlSegments(url: string): number { + // split at '/' or at '\/' to split regex urls correctly + return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length; +} + +/** + * Strip the basename from a pathname if exists. + * + * Vendored and modified from `react-router` + * https://github.com/remix-run/react-router/blob/462bb712156a3f739d6139a0f14810b76b002df6/packages/router/utils.ts#L1038 + */ +function stripBasenameFromPathname(pathname: string, basename: string): string { + if (!basename || basename === '/') { + return pathname; + } + + if (!pathname.toLowerCase().startsWith(basename.toLowerCase())) { + return pathname; + } + + // We want to leave trailing slash behavior in the user's control, so if they + // specify a basename with a trailing slash, we should support it + const startIndex = basename.endsWith('/') ? basename.length - 1 : basename.length; + const nextChar = pathname.charAt(startIndex); + if (nextChar && nextChar !== '/') { + // pathname does not start with basename/ + return pathname; + } + + return pathname.slice(startIndex) || '/'; +} + +// Exported utility functions + +/** + * Ensures a path string starts with a forward slash. + */ +export function prefixWithSlash(path: string): string { + return path[0] === '/' ? path : `/${path}`; +} + +/** + * Rebuilds the route path from all available routes by matching against the current location. + */ +export function rebuildRoutePathFromAllRoutes(allRoutes: RouteObject[], location: Location): string { + const matchedRoutes = _matchRoutes(allRoutes, location) as RouteMatch[]; + + if (!matchedRoutes || matchedRoutes.length === 0) { + return ''; + } + + for (const match of matchedRoutes) { + if (match.route.path && match.route.path !== '*') { + const path = pickPath(match); + const strippedPath = stripBasenameFromPathname(location.pathname, prefixWithSlash(match.pathnameBase)); + + if (location.pathname === strippedPath) { + return trimSlash(strippedPath); + } + + return trimSlash( + trimSlash(path || '') + + prefixWithSlash( + rebuildRoutePathFromAllRoutes( + allRoutes.filter(route => route !== match.route), + { + pathname: strippedPath, + }, + ), + ), + ); + } + } + + return ''; +} + +/** + * Checks if the current location is inside a descendant route (route with splat parameter). + */ +export function locationIsInsideDescendantRoute(location: Location, routes: RouteObject[]): boolean { + const matchedRoutes = _matchRoutes(routes, location) as RouteMatch[]; + + if (matchedRoutes) { + for (const match of matchedRoutes) { + if (routeIsDescendant(match.route) && pickSplat(match)) { + return true; + } + } + } + + return false; +} + +/** + * Gets a normalized route name and transaction source from the current routes and location. + */ +export function getNormalizedName( + routes: RouteObject[], + location: Location, + branches: RouteMatch[], + basename: string = '', +): [string, TransactionSource] { + if (!routes || routes.length === 0) { + return [_stripBasename ? stripBasenameFromPathname(location.pathname, basename) : location.pathname, 'url']; + } + + let pathBuilder = ''; + + if (branches) { + for (const branch of branches) { + const route = branch.route; + if (route) { + // Early return if index route + if (route.index) { + return sendIndexPath(pathBuilder, branch.pathname, basename); + } + const path = route.path; + + // If path is not a wildcard and has no child routes, append the path + if (path && !pathIsWildcardAndHasChildren(path, branch)) { + const newPath = path[0] === '/' || pathBuilder[pathBuilder.length - 1] === '/' ? path : `/${path}`; + pathBuilder = trimSlash(pathBuilder) + prefixWithSlash(newPath); + + // If the path matches the current location, return the path + if (trimSlash(location.pathname) === trimSlash(basename + branch.pathname)) { + if ( + // If the route defined on the element is something like + // Product} /> + // We should check against the branch.pathname for the number of / separators + getNumberOfUrlSegments(pathBuilder) !== getNumberOfUrlSegments(branch.pathname) && + // We should not count wildcard operators in the url segments calculation + !pathEndsWithWildcard(pathBuilder) + ) { + return [(_stripBasename ? '' : basename) + newPath, 'route']; + } + + // if the last character of the pathbuilder is a wildcard and there are children, remove the wildcard + if (pathIsWildcardAndHasChildren(pathBuilder, branch)) { + pathBuilder = pathBuilder.slice(0, -1); + } + + return [(_stripBasename ? '' : basename) + pathBuilder, 'route']; + } + } + } + } + } + + const fallbackTransactionName = _stripBasename + ? stripBasenameFromPathname(location.pathname, basename) + : location.pathname || ''; + + return [fallbackTransactionName, 'url']; +} + +/** + * Shared helper function to resolve route name and source + */ +export function resolveRouteNameAndSource( + location: Location, + routes: RouteObject[], + allRoutes: RouteObject[], + branches: RouteMatch[], + basename: string = '', +): [string, TransactionSource] { + let name: string | undefined; + let source: TransactionSource = 'url'; + + const isInDescendantRoute = locationIsInsideDescendantRoute(location, allRoutes); + + if (isInDescendantRoute) { + name = prefixWithSlash(rebuildRoutePathFromAllRoutes(allRoutes, location)); + source = 'route'; + } + + if (!isInDescendantRoute || !name) { + [name, source] = getNormalizedName(routes, location, branches, basename); + } + + return [name || location.pathname, source]; +} diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index a32e2bb02bf1..0c58bdb68f35 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -1,13 +1,13 @@ import type { browserTracingIntegration } from '@sentry/browser'; import type { Integration } from '@sentry/core'; -import type { ReactRouterOptions } from './reactrouterv6-compat-utils'; +import type { ReactRouterOptions } from './reactrouter-compat-utils'; import { createReactRouterV6CompatibleTracingIntegration, createV6CompatibleWithSentryReactRouterRouting, createV6CompatibleWrapCreateBrowserRouter, createV6CompatibleWrapCreateMemoryRouter, createV6CompatibleWrapUseRoutes, -} from './reactrouterv6-compat-utils'; +} from './reactrouter-compat-utils'; import type { CreateRouterFunction, Router, RouterState, UseRoutes } from './types'; /** diff --git a/packages/react/src/reactrouterv7.tsx b/packages/react/src/reactrouterv7.tsx index 5a80482cd2c3..a25e12d40e68 100644 --- a/packages/react/src/reactrouterv7.tsx +++ b/packages/react/src/reactrouterv7.tsx @@ -1,14 +1,14 @@ // React Router v7 uses the same integration as v6 import type { browserTracingIntegration } from '@sentry/browser'; import type { Integration } from '@sentry/core'; -import type { ReactRouterOptions } from './reactrouterv6-compat-utils'; +import type { ReactRouterOptions } from './reactrouter-compat-utils'; import { createReactRouterV6CompatibleTracingIntegration, createV6CompatibleWithSentryReactRouterRouting, createV6CompatibleWrapCreateBrowserRouter, createV6CompatibleWrapCreateMemoryRouter, createV6CompatibleWrapUseRoutes, -} from './reactrouterv6-compat-utils'; +} from './reactrouter-compat-utils'; import type { CreateRouterFunction, Router, RouterState, UseRoutes } from './types'; /** diff --git a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx new file mode 100644 index 000000000000..840adbf0a816 --- /dev/null +++ b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx @@ -0,0 +1,183 @@ +/** + * @vitest-environment jsdom + */ +import { startBrowserTracingNavigationSpan } from '@sentry/browser'; +import type { Client, Span } from '@sentry/core'; +import { addNonEnumerableProperty } from '@sentry/core'; +import * as React from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + addResolvedRoutesToParent, + createNewNavigationSpan, + createReactRouterV6CompatibleTracingIntegration, + handleExistingNavigationSpan, + updateNavigationSpan, +} from '../../src/reactrouter-compat-utils'; +import type { Location, RouteObject } from '../../src/types'; + +const mockUpdateName = vi.fn(); +const mockSetAttribute = vi.fn(); +const mockSpan = { updateName: mockUpdateName, setAttribute: mockSetAttribute } as unknown as Span; +const mockClient = { addIntegration: vi.fn() } as unknown as Client; + +vi.mock('@sentry/core', async requireActual => { + const actual = await requireActual(); + return { + ...(actual as any), + addNonEnumerableProperty: vi.fn(), + getActiveSpan: vi.fn(() => mockSpan), + getClient: vi.fn(() => mockClient), + getRootSpan: vi.fn(() => mockSpan), + spanToJSON: vi.fn(() => ({ op: 'navigation' })), + }; +}); + +vi.mock('@sentry/browser', async requireActual => { + const actual = await requireActual(); + return { + ...(actual as any), + startBrowserTracingNavigationSpan: vi.fn(), + browserTracingIntegration: vi.fn(() => ({ + setup: vi.fn(), + afterAllSetup: vi.fn(), + name: 'BrowserTracing', + })), + }; +}); + +vi.mock('../../src/reactrouter-compat-utils/utils', () => ({ + resolveRouteNameAndSource: vi.fn(() => ['Test Route', 'route']), + initializeRouterUtils: vi.fn(), + getGlobalLocation: vi.fn(() => ({ pathname: '/test', search: '', hash: '' })), + getGlobalPathname: vi.fn(() => '/test'), +})); + +vi.mock('../../src/reactrouter-compat-utils/lazy-routes', () => ({ + checkRouteForAsyncHandler: vi.fn(), +})); + +describe('reactrouter-compat-utils/instrumentation', () => { + const sampleLocation: Location = { + pathname: '/test', + search: '', + hash: '', + state: null, + key: 'default', + }; + + const sampleRoutes: RouteObject[] = [ + { path: '/', element:

Home
}, + { path: '/about', element:
About
}, + ]; + + const mockMatchRoutes = vi.fn(() => []); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('updateNavigationSpan', () => { + it('should update navigation span name and source when not already named', () => { + updateNavigationSpan(mockSpan, sampleLocation, sampleRoutes, false, mockMatchRoutes); + + expect(mockUpdateName).toHaveBeenCalledWith('Test Route'); + expect(mockSetAttribute).toHaveBeenCalledWith('sentry.source', 'route'); + expect(addNonEnumerableProperty).toHaveBeenCalledWith(mockSpan, '__sentry_navigation_name_set__', true); + }); + + it('should not update when span already has name set', () => { + const spanWithNameSet = { ...mockSpan, __sentry_navigation_name_set__: true }; + + updateNavigationSpan(spanWithNameSet as any, sampleLocation, sampleRoutes, false, mockMatchRoutes); + + expect(mockUpdateName).not.toHaveBeenCalled(); + }); + }); + + describe('handleExistingNavigationSpan', () => { + it('should update span name when not already named', () => { + const spanJson = { op: 'navigation', data: {}, span_id: 'test', start_timestamp: 0, trace_id: 'test' }; + + handleExistingNavigationSpan(mockSpan, spanJson, 'Test Route', 'route', false); + + expect(mockUpdateName).toHaveBeenCalledWith('Test Route'); + expect(mockSetAttribute).toHaveBeenCalledWith('sentry.source', 'route'); + expect(addNonEnumerableProperty).toHaveBeenCalledWith(mockSpan, '__sentry_navigation_name_set__', true); + }); + + it('should not mark as named for lazy routes', () => { + const spanJson = { op: 'navigation', data: {}, span_id: 'test', start_timestamp: 0, trace_id: 'test' }; + + handleExistingNavigationSpan(mockSpan, spanJson, 'Test Route', 'route', true); + + expect(mockUpdateName).toHaveBeenCalledWith('Test Route'); + expect(mockSetAttribute).toHaveBeenCalledWith('sentry.source', 'route'); + expect(addNonEnumerableProperty).not.toHaveBeenCalled(); + }); + }); + + describe('createNewNavigationSpan', () => { + it('should create new navigation span with correct attributes', () => { + createNewNavigationSpan(mockClient, 'Test Route', 'route', '6', false); + + expect(startBrowserTracingNavigationSpan).toHaveBeenCalledWith(mockClient, { + name: 'Test Route', + attributes: { + 'sentry.source': 'route', + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react.reactrouter_v6', + }, + }); + }); + }); + + describe('addResolvedRoutesToParent', () => { + it('should add new routes to parent with no existing children', () => { + const parentRoute: RouteObject = { path: '/parent', element:
Parent
}; + const resolvedRoutes = [{ path: '/child1', element:
Child 1
}]; + + addResolvedRoutesToParent(resolvedRoutes, parentRoute); + + expect(parentRoute.children).toEqual(resolvedRoutes); + }); + + it('should not add duplicate routes by path', () => { + const existingRoute = { path: '/duplicate', element:
Existing
}; + const parentRoute: RouteObject = { + path: '/parent', + element:
Parent
, + children: [existingRoute], + }; + const duplicateRoute = { path: '/duplicate', element:
Duplicate
}; + + addResolvedRoutesToParent([duplicateRoute], parentRoute); + + expect(parentRoute.children).toEqual([existingRoute]); + }); + }); + + describe('createReactRouterV6CompatibleTracingIntegration', () => { + it('should create integration with correct setup', () => { + const mockUseEffect = vi.fn(); + const mockUseLocation = vi.fn(); + const mockUseNavigationType = vi.fn(); + const mockCreateRoutesFromChildren = vi.fn(); + + const integration = createReactRouterV6CompatibleTracingIntegration( + { + useEffect: mockUseEffect, + useLocation: mockUseLocation, + useNavigationType: mockUseNavigationType, + createRoutesFromChildren: mockCreateRoutesFromChildren, + matchRoutes: mockMatchRoutes, + }, + '6', + ); + + expect(integration).toHaveProperty('setup'); + expect(integration).toHaveProperty('afterAllSetup'); + expect(typeof integration.setup).toBe('function'); + expect(typeof integration.afterAllSetup).toBe('function'); + }); + }); +}); diff --git a/packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts b/packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts new file mode 100644 index 000000000000..732b893ea8f8 --- /dev/null +++ b/packages/react/test/reactrouter-compat-utils/lazy-routes.test.ts @@ -0,0 +1,706 @@ +import { addNonEnumerableProperty, debug } from '@sentry/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + checkRouteForAsyncHandler, + createAsyncHandlerProxy, + handleAsyncHandlerResult, +} from '../../src/reactrouter-compat-utils'; +import type { RouteObject } from '../../src/types'; + +vi.mock('@sentry/core', async requireActual => { + const actual = await requireActual(); + return { + ...(actual as any), + addNonEnumerableProperty: vi.fn(), + debug: { + warn: vi.fn(), + }, + }; +}); + +vi.mock('../../src/debug-build', () => ({ + DEBUG_BUILD: true, +})); + +describe('reactrouter-compat-utils/lazy-routes', () => { + let mockProcessResolvedRoutes: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + mockProcessResolvedRoutes = vi.fn(); + }); + + describe('createAsyncHandlerProxy', () => { + it('should create a proxy that calls the original function', () => { + const originalFunction = vi.fn(() => 'result'); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + const result = proxy('arg1', 'arg2'); + + expect(originalFunction).toHaveBeenCalledWith('arg1', 'arg2'); + expect(result).toBe('result'); + }); + + it('should preserve the original function context (this binding)', () => { + const context = { value: 'test' }; + const originalFunction = vi.fn(function (this: typeof context) { + return this.value; + }); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + const result = proxy.call(context); + + expect(originalFunction).toHaveBeenCalledWith(); + expect(result).toBe('test'); + }); + + it('should handle functions with no arguments', () => { + const originalFunction = vi.fn(() => 'no-args-result'); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + const result = proxy(); + + expect(originalFunction).toHaveBeenCalledWith(); + expect(result).toBe('no-args-result'); + }); + + it('should handle functions with many arguments', () => { + const originalFunction = vi.fn((...args) => args.length); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + const result = proxy(1, 2, 3, 4, 5); + + expect(originalFunction).toHaveBeenCalledWith(1, 2, 3, 4, 5); + expect(result).toBe(5); + }); + + it('should mark the proxy with __sentry_proxied__ property', () => { + const originalFunction = vi.fn(); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + expect(addNonEnumerableProperty).toHaveBeenCalledWith(expect.any(Function), '__sentry_proxied__', true); + }); + + it('should call handleAsyncHandlerResult with the function result', () => { + const originalFunction = vi.fn(() => ['route1', 'route2']); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + proxy(); + + // Since handleAsyncHandlerResult is called internally, we verify through its side effects + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(['route1', 'route2'], route); + }); + + it('should handle functions that throw exceptions', () => { + const originalFunction = vi.fn(() => { + throw new Error('Test error'); + }); + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + + expect(() => proxy()).toThrow('Test error'); + expect(originalFunction).toHaveBeenCalled(); + }); + + it('should handle complex route objects', () => { + const originalFunction = vi.fn(() => []); + const route: RouteObject = { + path: '/complex', + id: 'complex-route', + index: false, + caseSensitive: true, + children: [{ path: 'child' }], + element: '
Test
', + }; + const handlerKey = 'complexHandler'; + + const proxy = createAsyncHandlerProxy(originalFunction, route, handlerKey, mockProcessResolvedRoutes); + proxy(); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith([], route); + }); + }); + + describe('handleAsyncHandlerResult', () => { + const route: RouteObject = { path: '/test' }; + const handlerKey = 'testHandler'; + + it('should handle array results directly', () => { + const routes: RouteObject[] = [{ path: '/route1' }, { path: '/route2' }]; + + handleAsyncHandlerResult(routes, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(routes, route); + }); + + it('should handle empty array results', () => { + const routes: RouteObject[] = []; + + handleAsyncHandlerResult(routes, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(routes, route); + }); + + it('should handle Promise results that resolve to arrays', async () => { + const routes: RouteObject[] = [{ path: '/route1' }, { path: '/route2' }]; + const promiseResult = Promise.resolve(routes); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + // Wait for the promise to resolve + await promiseResult; + + // Use setTimeout to wait for the async handling + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(routes, route); + }); + + it('should handle Promise results that resolve to empty arrays', async () => { + const routes: RouteObject[] = []; + const promiseResult = Promise.resolve(routes); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + await promiseResult; + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(routes, route); + }); + + it('should handle Promise results that resolve to non-arrays', async () => { + const promiseResult = Promise.resolve('not an array'); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + await promiseResult; + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should handle Promise results that resolve to null', async () => { + const promiseResult = Promise.resolve(null); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + await promiseResult; + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should handle Promise results that resolve to undefined', async () => { + const promiseResult = Promise.resolve(undefined); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + await promiseResult; + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should handle Promise rejections gracefully', async () => { + const promiseResult = Promise.reject(new Error('Test error')); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + // Wait for the promise to be handled + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(debug.warn).toHaveBeenCalledWith( + expect.stringContaining('Error resolving async handler'), + route, + expect.any(Error), + ); + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should handle Promise rejections with non-Error values', async () => { + const promiseResult = Promise.reject('string error'); + + handleAsyncHandlerResult(promiseResult, route, handlerKey, mockProcessResolvedRoutes); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(debug.warn).toHaveBeenCalledWith( + expect.stringContaining('Error resolving async handler'), + route, + 'string error', + ); + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should ignore non-promise, non-array results', () => { + handleAsyncHandlerResult('string result', route, handlerKey, mockProcessResolvedRoutes); + handleAsyncHandlerResult(123, route, handlerKey, mockProcessResolvedRoutes); + handleAsyncHandlerResult({ not: 'array' }, route, handlerKey, mockProcessResolvedRoutes); + handleAsyncHandlerResult(null, route, handlerKey, mockProcessResolvedRoutes); + handleAsyncHandlerResult(undefined, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should ignore boolean values', () => { + handleAsyncHandlerResult(true, route, handlerKey, mockProcessResolvedRoutes); + handleAsyncHandlerResult(false, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should ignore functions as results', () => { + const functionResult = () => 'test'; + handleAsyncHandlerResult(functionResult, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it("should handle objects that look like promises but aren't", () => { + const fakeThenableButNotPromise = { + then: 'not a function', + }; + + handleAsyncHandlerResult(fakeThenableButNotPromise, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should handle objects that have then property but not a function', () => { + const almostPromise = { + then: null, + }; + + handleAsyncHandlerResult(almostPromise, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).not.toHaveBeenCalled(); + }); + + it('should handle complex route objects in async handling', async () => { + const complexRoute: RouteObject = { + path: '/complex', + id: 'complex-route', + loader: vi.fn(), + element: '
Complex
', + }; + const routes: RouteObject[] = [{ path: '/dynamic1' }, { path: '/dynamic2' }]; + const promiseResult = Promise.resolve(routes); + + handleAsyncHandlerResult(promiseResult, complexRoute, 'complexHandler', mockProcessResolvedRoutes); + + await promiseResult; + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(routes, complexRoute); + }); + + it('should handle nested route objects in arrays', () => { + const routes: RouteObject[] = [ + { + path: '/parent', + children: [{ path: 'child1' }, { path: 'child2', children: [{ path: 'grandchild' }] }], + }, + ]; + + handleAsyncHandlerResult(routes, route, handlerKey, mockProcessResolvedRoutes); + + expect(mockProcessResolvedRoutes).toHaveBeenCalledWith(routes, route); + }); + }); + + describe('checkRouteForAsyncHandler', () => { + it('should proxy functions in route.handle', () => { + const testFunction = vi.fn(); + const route: RouteObject = { + path: '/test', + handle: { + testHandler: testFunction, + notAFunction: 'string value', + }, + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // The function should be replaced with a proxy + expect(route.handle!.testHandler).not.toBe(testFunction); + expect(typeof route.handle!.testHandler).toBe('function'); + expect(route.handle!.notAFunction).toBe('string value'); + }); + + it('should handle multiple functions in route.handle', () => { + const handler1 = vi.fn(); + const handler2 = vi.fn(); + const handler3 = vi.fn(); + const route: RouteObject = { + path: '/test', + handle: { + handler1, + handler2, + handler3, + nonFunction: 'not a function', + anotherNonFunction: 42, + }, + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // All functions should be proxied + expect(route.handle!.handler1).not.toBe(handler1); + expect(route.handle!.handler2).not.toBe(handler2); + expect(route.handle!.handler3).not.toBe(handler3); + expect(typeof route.handle!.handler1).toBe('function'); + expect(typeof route.handle!.handler2).toBe('function'); + expect(typeof route.handle!.handler3).toBe('function'); + + // Non-functions should remain unchanged + expect(route.handle!.nonFunction).toBe('not a function'); + expect(route.handle!.anotherNonFunction).toBe(42); + }); + + it('should not proxy already proxied functions', () => { + const testFunction = vi.fn(); + // Mark function as already proxied + (testFunction as any).__sentry_proxied__ = true; + + const route: RouteObject = { + path: '/test', + handle: { + testHandler: testFunction, + }, + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // The function should remain unchanged + expect(route.handle!.testHandler).toBe(testFunction); + }); + + it('should handle mix of proxied and non-proxied functions', () => { + const proxiedFunction = vi.fn(); + const normalFunction = vi.fn(); + (proxiedFunction as any).__sentry_proxied__ = true; + + const route: RouteObject = { + path: '/test', + handle: { + proxiedHandler: proxiedFunction, + normalHandler: normalFunction, + }, + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // Proxied function should remain unchanged + expect(route.handle!.proxiedHandler).toBe(proxiedFunction); + // Normal function should be proxied + expect(route.handle!.normalHandler).not.toBe(normalFunction); + }); + + it('should recursively check child routes', () => { + const parentFunction = vi.fn(); + const childFunction = vi.fn(); + + const route: RouteObject = { + path: '/parent', + handle: { + parentHandler: parentFunction, + }, + children: [ + { + path: 'child', + handle: { + childHandler: childFunction, + }, + }, + ], + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // Both parent and child functions should be proxied + expect(route.handle!.parentHandler).not.toBe(parentFunction); + expect(route.children![0].handle!.childHandler).not.toBe(childFunction); + }); + + it('should handle children without handle properties', () => { + const parentFunction = vi.fn(); + + const route: RouteObject = { + path: '/parent', + handle: { + parentHandler: parentFunction, + }, + children: [ + { + path: 'child1', + // No handle property + }, + { + path: 'child2', + handle: undefined, + }, + ], + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + + expect(route.handle!.parentHandler).not.toBe(parentFunction); + }); + + it('should handle routes without handle property', () => { + const route: RouteObject = { + path: '/test', + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle routes with null handle property', () => { + const route: RouteObject = { + path: '/test', + handle: null, + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle routes with handle that is not an object', () => { + const route: RouteObject = { + path: '/test', + // @ts-expect-error - Testing edge case + handle: 'not an object', + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle routes with handle that is a function', () => { + const route: RouteObject = { + path: '/test', + // @ts-expect-error - Testing edge case + handle: vi.fn(), + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle routes without children', () => { + const testFunction = vi.fn(); + const route: RouteObject = { + path: '/test', + handle: { + testHandler: testFunction, + }, + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle routes with null children', () => { + const testFunction = vi.fn(); + const route: RouteObject = { + path: '/test', + handle: { + testHandler: testFunction, + }, + children: null, + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle routes with empty children array', () => { + const testFunction = vi.fn(); + const route: RouteObject = { + path: '/test', + handle: { + testHandler: testFunction, + }, + children: [], + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + }); + + it('should handle deeply nested child routes', () => { + const grandParentFunction = vi.fn(); + const parentFunction = vi.fn(); + const childFunction = vi.fn(); + + const route: RouteObject = { + path: '/grandparent', + handle: { + grandParentHandler: grandParentFunction, + }, + children: [ + { + path: 'parent', + handle: { + parentHandler: parentFunction, + }, + children: [ + { + path: 'child', + handle: { + childHandler: childFunction, + }, + }, + ], + }, + ], + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // All functions should be proxied + expect(route.handle!.grandParentHandler).not.toBe(grandParentFunction); + expect(route.children![0].handle!.parentHandler).not.toBe(parentFunction); + expect(route.children![0].children![0].handle!.childHandler).not.toBe(childFunction); + }); + + it('should handle routes with complex nested structures', () => { + const route: RouteObject = { + path: '/complex', + handle: { + handler1: vi.fn(), + handler2: vi.fn(), + }, + children: [ + { + path: 'level1a', + handle: { + level1aHandler: vi.fn(), + }, + children: [ + { + path: 'level2a', + handle: { + level2aHandler: vi.fn(), + }, + }, + { + path: 'level2b', + // No handle + }, + ], + }, + { + path: 'level1b', + // No handle + children: [ + { + path: 'level2c', + handle: { + level2cHandler: vi.fn(), + }, + }, + ], + }, + ], + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + + // Check that functions were proxied at all levels + expect(typeof route.handle!.handler1).toBe('function'); + expect(typeof route.handle!.handler2).toBe('function'); + expect(typeof route.children![0].handle!.level1aHandler).toBe('function'); + expect(typeof route.children![0].children![0].handle!.level2aHandler).toBe('function'); + expect(typeof route.children![1].children![0].handle!.level2cHandler).toBe('function'); + }); + + it('should preserve route properties during processing', () => { + const originalFunction = vi.fn(); + const route: RouteObject = { + path: '/preserve', + id: 'preserve-route', + caseSensitive: true, + index: false, + handle: { + testHandler: originalFunction, + }, + element: '
Test
', + loader: vi.fn(), + }; + + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + + // Route properties should be preserved + expect(route.path).toBe('/preserve'); + expect(route.id).toBe('preserve-route'); + expect(route.caseSensitive).toBe(true); + expect(route.index).toBe(false); + expect(route.element).toBe('
Test
'); + expect(route.loader).toBeDefined(); + + // Only the handler function should be changed + expect(route.handle!.testHandler).not.toBe(originalFunction); + }); + + it('should handle functions with special names', () => { + const route: RouteObject = { + path: '/test', + handle: { + constructor: vi.fn(), + toString: vi.fn(), + valueOf: vi.fn(), + hasOwnProperty: vi.fn(), + '0': vi.fn(), + 'special-name': vi.fn(), + 'with spaces': vi.fn(), + }, + }; + + expect(() => { + checkRouteForAsyncHandler(route, mockProcessResolvedRoutes); + }).not.toThrow(); + + // All functions should be proxied regardless of name + expect(typeof route.handle!.constructor).toBe('function'); + expect(typeof route.handle!.toString).toBe('function'); + expect(typeof route.handle!.valueOf).toBe('function'); + expect(typeof route.handle!.hasOwnProperty).toBe('function'); + expect(typeof route.handle!['0']).toBe('function'); + expect(typeof route.handle!['special-name']).toBe('function'); + expect(typeof route.handle!['with spaces']).toBe('function'); + }); + }); +}); diff --git a/packages/react/test/reactrouter-compat-utils/utils.test.ts b/packages/react/test/reactrouter-compat-utils/utils.test.ts new file mode 100644 index 000000000000..91885940db31 --- /dev/null +++ b/packages/react/test/reactrouter-compat-utils/utils.test.ts @@ -0,0 +1,632 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + getNormalizedName, + getNumberOfUrlSegments, + initializeRouterUtils, + locationIsInsideDescendantRoute, + pathEndsWithWildcard, + pathIsWildcardAndHasChildren, + prefixWithSlash, + rebuildRoutePathFromAllRoutes, + resolveRouteNameAndSource, +} from '../../src/reactrouter-compat-utils'; +import type { Location, MatchRoutes, RouteMatch, RouteObject } from '../../src/types'; + +vi.mock('@sentry/browser', async requireActual => { + const actual = await requireActual(); + return { + ...(actual as any), + WINDOW: { + location: { + pathname: '/test/path', + search: '?query=1', + hash: '#section', + href: 'https://example.com/test/path?query=1#section', + }, + }, + }; +}); + +const mockMatchRoutes = vi.fn(); + +describe('reactrouter-compat-utils/utils', () => { + beforeEach(() => { + vi.clearAllMocks(); + initializeRouterUtils(mockMatchRoutes as MatchRoutes, false); + }); + + describe('initializeRouterUtils', () => { + it('should initialize with matchRoutes function', () => { + expect(() => { + initializeRouterUtils(mockMatchRoutes as MatchRoutes, false); + }).not.toThrow(); + }); + + it('should handle custom matchRoutes function with dev mode true', () => { + const customMatchRoutes = vi.fn(); + expect(() => { + initializeRouterUtils(customMatchRoutes as MatchRoutes, true); + }).not.toThrow(); + }); + + it('should handle custom matchRoutes function without dev mode flag', () => { + const customMatchRoutes = vi.fn(); + expect(() => { + initializeRouterUtils(customMatchRoutes as MatchRoutes); + }).not.toThrow(); + }); + }); + + describe('prefixWithSlash', () => { + it('should add slash to string without leading slash', () => { + expect(prefixWithSlash('path')).toBe('/path'); + }); + + it('should not add slash to string with leading slash', () => { + expect(prefixWithSlash('/path')).toBe('/path'); + }); + + it('should handle empty string', () => { + expect(prefixWithSlash('')).toBe('/'); + }); + }); + + describe('pathEndsWithWildcard', () => { + it('should return true for path ending with /*', () => { + expect(pathEndsWithWildcard('/users/*')).toBe(true); + }); + + it('should return false for path not ending with /*', () => { + expect(pathEndsWithWildcard('/users')).toBe(false); + }); + + it('should return true for path ending with *', () => { + expect(pathEndsWithWildcard('/users*')).toBe(true); + }); + + it('should return false for empty string', () => { + expect(pathEndsWithWildcard('')).toBe(false); + }); + }); + + describe('pathIsWildcardAndHasChildren', () => { + it('should return true for wildcard path with children', () => { + const branch = { + route: { + path: '/users/*', + children: [{ path: 'profile' }], + }, + params: {}, + pathname: '/users', + pathnameBase: '/users', + } as RouteMatch; + + expect(pathIsWildcardAndHasChildren('/users/*', branch)).toBe(true); + }); + + it('should return false for wildcard path without children', () => { + const branch = { + route: { + path: '/users/*', + children: [], + }, + params: {}, + pathname: '/users', + pathnameBase: '/users', + } as RouteMatch; + + expect(pathIsWildcardAndHasChildren('/users/*', branch)).toBe(false); + }); + + it('should return false for non-wildcard path with children', () => { + const branch = { + route: { + path: '/users', + children: [{ path: 'profile' }], + }, + params: {}, + pathname: '/users', + pathnameBase: '/users', + } as RouteMatch; + + expect(pathIsWildcardAndHasChildren('/users', branch)).toBe(false); + }); + + it('should return false for non-wildcard path without children', () => { + const branch = { + route: { + path: '/users', + }, + params: {}, + pathname: '/users', + pathnameBase: '/users', + } as RouteMatch; + + expect(pathIsWildcardAndHasChildren('/users', branch)).toBe(false); + }); + + it('should return false when route has undefined children', () => { + const branch = { + route: { + path: '/users/*', + children: undefined, + }, + params: {}, + pathname: '/users', + pathnameBase: '/users', + } as RouteMatch; + + expect(pathIsWildcardAndHasChildren('/users/*', branch)).toBe(false); + }); + }); + + describe('getNumberOfUrlSegments', () => { + it('should count URL segments correctly', () => { + expect(getNumberOfUrlSegments('/users/123/profile')).toBe(3); + }); + + it('should handle single segment', () => { + expect(getNumberOfUrlSegments('/users')).toBe(1); + }); + + it('should handle root path', () => { + expect(getNumberOfUrlSegments('/')).toBe(0); + }); + + it('should handle empty string', () => { + expect(getNumberOfUrlSegments('')).toBe(0); + }); + + it('should handle regex URLs with escaped slashes', () => { + expect(getNumberOfUrlSegments('/users\\/profile')).toBe(2); + }); + + it('should filter out empty segments and commas', () => { + expect(getNumberOfUrlSegments('/users//profile,test')).toBe(2); + }); + }); + + describe('rebuildRoutePathFromAllRoutes', () => { + it('should return pathname when it matches stripped path', () => { + const allRoutes: RouteObject[] = [{ path: '/users', element: null }]; + + const location: Location = { pathname: '/users' }; + + const mockMatches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = rebuildRoutePathFromAllRoutes(allRoutes, location); + expect(result).toBe('/users'); + }); + + it('should return empty string when no routes match', () => { + const allRoutes: RouteObject[] = [{ path: '/users', element: null }]; + + const location: Location = { pathname: '/nonexistent' }; + + mockMatchRoutes.mockReturnValue([]); + + const result = rebuildRoutePathFromAllRoutes(allRoutes, location); + expect(result).toBe(''); + }); + + it('should return empty string when no matches found', () => { + const allRoutes: RouteObject[] = [{ path: '/users', element: null }]; + + const location: Location = { pathname: '/users' }; + + mockMatchRoutes.mockReturnValue(null); + + const result = rebuildRoutePathFromAllRoutes(allRoutes, location); + expect(result).toBe(''); + }); + + it('should handle wildcard routes', () => { + const allRoutes: RouteObject[] = [{ path: '/users/*', element: null }]; + + const location: Location = { pathname: '/users/anything' }; + + const mockMatches: RouteMatch[] = [ + { + route: { path: '*', element: null }, + params: { '*': 'anything' }, + pathname: '/users/anything', + pathnameBase: '/users', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = rebuildRoutePathFromAllRoutes(allRoutes, location); + expect(result).toBe(''); + }); + }); + + describe('locationIsInsideDescendantRoute', () => { + it('should return true when location is inside descendant route', () => { + const routes: RouteObject[] = [ + { + path: '/users/*', + element: 'div', + children: undefined, + }, + ]; + + const location: Location = { pathname: '/users/123/profile' }; + + const mockMatches: RouteMatch[] = [ + { + route: { + path: '/users/*', + element: 'div', + children: undefined, + }, + params: { '*': '123/profile' }, + pathname: '/users/123/profile', + pathnameBase: '/users', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = locationIsInsideDescendantRoute(location, routes); + expect(result).toBe(true); + }); + + it('should return false when route has children (not descendant)', () => { + const routes: RouteObject[] = [ + { + path: '/users/*', + element: 'div', + children: [{ path: 'profile' }], + }, + ]; + + const location: Location = { pathname: '/users/123/profile' }; + + const mockMatches: RouteMatch[] = [ + { + route: { + path: '/users/*', + element: 'div', + children: [{ path: 'profile' }], + }, + params: { '*': '123/profile' }, + pathname: '/users/123/profile', + pathnameBase: '/users', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = locationIsInsideDescendantRoute(location, routes); + expect(result).toBe(false); + }); + + it('should return false when route has no element', () => { + const routes: RouteObject[] = [ + { + path: '/users/*', + element: null, + children: undefined, + }, + ]; + + const location: Location = { pathname: '/users/123/profile' }; + + const mockMatches: RouteMatch[] = [ + { + route: { + path: '/users/*', + element: null, + children: undefined, + }, + params: { '*': '123/profile' }, + pathname: '/users/123/profile', + pathnameBase: '/users', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = locationIsInsideDescendantRoute(location, routes); + expect(result).toBe(false); + }); + + it('should return false when path does not end with /*', () => { + const routes: RouteObject[] = [ + { + path: '/users', + element: 'div', + children: undefined, + }, + ]; + + const location: Location = { pathname: '/users' }; + + const mockMatches: RouteMatch[] = [ + { + route: { + path: '/users', + element: 'div', + children: undefined, + }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = locationIsInsideDescendantRoute(location, routes); + expect(result).toBe(false); + }); + + it('should return false when no splat parameter', () => { + const routes: RouteObject[] = [ + { + path: '/users/*', + element: 'div', + children: undefined, + }, + ]; + + const location: Location = { pathname: '/users' }; + + const mockMatches: RouteMatch[] = [ + { + route: { + path: '/users/*', + element: 'div', + children: undefined, + }, + params: {}, + pathname: '/users', + pathnameBase: '/users', + }, + ]; + + mockMatchRoutes.mockReturnValue(mockMatches); + + const result = locationIsInsideDescendantRoute(location, routes); + expect(result).toBe(false); + }); + + it('should return false when no matches found', () => { + const routes: RouteObject[] = [{ path: '/users', element: null }]; + + const location: Location = { pathname: '/posts' }; + + mockMatchRoutes.mockReturnValue(null); + + const result = locationIsInsideDescendantRoute(location, routes); + expect(result).toBe(false); + }); + }); + + describe('getNormalizedName', () => { + it('should return pathname with url source when no routes provided', () => { + const routes: RouteObject[] = []; + const location: Location = { pathname: '/test' }; + const branches: RouteMatch[] = []; + + const result = getNormalizedName(routes, location, branches); + expect(result).toEqual(['/test', 'url']); + }); + + it('should handle index route', () => { + const routes: RouteObject[] = [{ path: '/', index: true, element: null }]; + const location: Location = { pathname: '/' }; + const branches: RouteMatch[] = [ + { + route: { path: '/', index: true, element: null }, + params: {}, + pathname: '/', + pathnameBase: '', + }, + ]; + + const result = getNormalizedName(routes, location, branches, ''); + expect(result).toEqual(['', 'route']); + }); + + it('should handle simple route path', () => { + const routes: RouteObject[] = [{ path: '/users', element: null }]; + const location: Location = { pathname: '/users' }; + const branches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + ]; + + const result = getNormalizedName(routes, location, branches, ''); + expect(result).toEqual(['/users', 'route']); + }); + + it('should handle nested routes', () => { + const routes: RouteObject[] = [ + { path: '/users', element: null }, + { path: ':userId', element: null }, + ]; + const location: Location = { pathname: '/users/123' }; + const branches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + { + route: { path: ':userId', element: null }, + params: { userId: '123' }, + pathname: '/users/123', + pathnameBase: '/users', + }, + ]; + + const result = getNormalizedName(routes, location, branches, ''); + expect(result).toEqual(['/users/:userId', 'route']); + }); + + it('should handle wildcard routes with children', () => { + const routes: RouteObject[] = [ + { + path: '/users/*', + element: null, + children: [{ path: 'profile', element: null }], + }, + ]; + const location: Location = { pathname: '/users/profile' }; + const branches: RouteMatch[] = [ + { + route: { + path: '/users/*', + element: null, + children: [{ path: 'profile', element: null }], + }, + params: { '*': 'profile' }, + pathname: '/users/profile', + pathnameBase: '/users', + }, + ]; + + const result = getNormalizedName(routes, location, branches, ''); + // Function falls back to url when wildcard path with children doesn't match exact logic + expect(result).toEqual(['/users/profile', 'url']); + }); + + it('should handle basename stripping', () => { + // Initialize with stripBasename = true + initializeRouterUtils(mockMatchRoutes as MatchRoutes, true); + + const routes: RouteObject[] = [{ path: '/users', element: null }]; + const location: Location = { pathname: '/app/users' }; + const branches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/app/users', + pathnameBase: '/app', + }, + ]; + + const result = getNormalizedName(routes, location, branches, '/app'); + // Function falls back to url when basename stripping doesn't match exact logic + expect(result).toEqual(['/users', 'url']); + }); + + it('should fallback to pathname when no matches', () => { + const routes: RouteObject[] = [{ path: '/users', element: null }]; + const location: Location = { pathname: '/posts' }; + const branches: RouteMatch[] = []; + + const result = getNormalizedName(routes, location, branches, ''); + expect(result).toEqual(['/posts', 'url']); + }); + }); + + describe('resolveRouteNameAndSource', () => { + beforeEach(() => { + // Reset to default stripBasename = false + initializeRouterUtils(mockMatchRoutes as MatchRoutes, false); + }); + + it('should use descendant route when location is inside one', () => { + const location: Location = { pathname: '/users/123/profile' }; + const routes: RouteObject[] = [{ path: '/users', element: null }]; + const allRoutes: RouteObject[] = [ + { path: '/users/*', element: 'div' }, // element must be truthy for descendant route + { path: '/users', element: null }, + ]; + const branches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + ]; + + // Mock for descendant route check + const descendantMatches: RouteMatch[] = [ + { + route: { path: '/users/*', element: 'div' }, + params: { '*': '123/profile' }, + pathname: '/users/123/profile', + pathnameBase: '/users', + }, + ]; + + // Mock for rebuild route path - should return '/users' + const rebuildMatches: RouteMatch[] = [ + { + route: { path: '/users/*', element: 'div' }, + params: { '*': '123/profile' }, + pathname: '/users', + pathnameBase: '', + }, + ]; + + mockMatchRoutes + .mockReturnValueOnce(descendantMatches) // First call for descendant check + .mockReturnValueOnce(rebuildMatches); // Second call for path rebuild + + const result = resolveRouteNameAndSource(location, routes, allRoutes, branches, ''); + // Since locationIsInsideDescendantRoute returns true, it uses route source + expect(result).toEqual(['/users/123/profile', 'route']); + }); + + it('should use normalized name when not in descendant route', () => { + const location: Location = { pathname: '/users' }; + const routes: RouteObject[] = [{ path: '/users', element: null }]; + const allRoutes: RouteObject[] = [{ path: '/users', element: null }]; + const branches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + ]; + + // Mock for descendant route check (no descendant) + const normalMatches: RouteMatch[] = [ + { + route: { path: '/users', element: null }, + params: {}, + pathname: '/users', + pathnameBase: '', + }, + ]; + + mockMatchRoutes.mockReturnValue(normalMatches); + + const result = resolveRouteNameAndSource(location, routes, allRoutes, branches, ''); + expect(result).toEqual(['/users', 'route']); + }); + + it('should fallback to pathname when no name resolved', () => { + const location: Location = { pathname: '/unknown' }; + const routes: RouteObject[] = []; + const allRoutes: RouteObject[] = []; + const branches: RouteMatch[] = []; + + mockMatchRoutes.mockReturnValue(null); + + const result = resolveRouteNameAndSource(location, routes, allRoutes, branches, ''); + expect(result).toEqual(['/unknown', 'url']); + }); + }); +}); diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/packages/react/test/reactrouter-cross-usage.test.tsx index 57f9b4ce00cf..76cfe59f3df5 100644 --- a/packages/react/test/reactrouter-cross-usage.test.tsx +++ b/packages/react/test/reactrouter-cross-usage.test.tsx @@ -288,11 +288,6 @@ describe('React Router cross usage of wrappers', () => { // It's called 1 time from the wrapped `MemoryRouter` expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); - - // It's called 3 times from the 3 `useRoutes` components - expect(mockNavigationSpan.updateName).toHaveBeenCalledTimes(3); - expect(mockNavigationSpan.updateName).toHaveBeenLastCalledWith('/second-level/:id/third-level/:id'); - expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { name: '/second-level/:id/third-level/:id', attributes: { @@ -459,11 +454,6 @@ describe('React Router cross usage of wrappers', () => { // It's called 1 time from the wrapped `MemoryRouter` expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); - - // It's called 3 times from the 3 `useRoutes` components - expect(mockNavigationSpan.updateName).toHaveBeenCalledTimes(3); - expect(mockNavigationSpan.updateName).toHaveBeenLastCalledWith('/second-level/:id/third-level/:id'); - expect(mockNavigationSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); }); }); @@ -611,10 +601,6 @@ describe('React Router cross usage of wrappers', () => { // It's called 1 time from the wrapped `createMemoryRouter` expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); - // It's called 3 times from the 3 `SentryRoutes` components - expect(mockNavigationSpan.updateName).toHaveBeenCalledTimes(3); - expect(mockNavigationSpan.updateName).toHaveBeenLastCalledWith('/second-level/:id/third-level/:id'); - expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { name: '/second-level/:id/third-level/:id', attributes: { @@ -790,11 +776,6 @@ describe('React Router cross usage of wrappers', () => { // It's called 1 time from the wrapped `MemoryRouter` expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); - - // It's called 3 times from the 2 `useRoutes` components and 1 component - expect(mockNavigationSpan.updateName).toHaveBeenCalledTimes(3); - - expect(mockNavigationSpan.updateName).toHaveBeenLastCalledWith('/second-level/:id/third-level/:id'); expect(mockNavigationSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); }); }); diff --git a/packages/react/test/reactrouterv6-compat-utils.test.tsx b/packages/react/test/reactrouterv6-compat-utils.test.tsx deleted file mode 100644 index 1f10d89c8558..000000000000 --- a/packages/react/test/reactrouterv6-compat-utils.test.tsx +++ /dev/null @@ -1,315 +0,0 @@ -import { describe, expect, it, test } from 'vitest'; -import { checkRouteForAsyncHandler, getNumberOfUrlSegments } from '../src/reactrouterv6-compat-utils'; -import type { RouteObject } from '../src/types'; - -describe('getNumberOfUrlSegments', () => { - test.each([ - ['regular path', '/projects/123/views/234', 4], - ['single param parameterized path', '/users/:id/details', 3], - ['multi param parameterized path', '/stores/:storeId/products/:productId', 4], - ['regex path', String(/\/api\/post[0-9]/), 2], - ])('%s', (_: string, input, output) => { - expect(getNumberOfUrlSegments(input)).toEqual(output); - }); -}); - -describe('checkRouteForAsyncHandler', () => { - it('should not create nested proxies when called multiple times on the same route', () => { - const mockHandler = () => Promise.resolve([]); - const route: RouteObject = { - path: '/test', - handle: { - lazyChildren: mockHandler, - }, - }; - - checkRouteForAsyncHandler(route); - checkRouteForAsyncHandler(route); - - const proxiedHandler = route.handle?.lazyChildren; - expect(typeof proxiedHandler).toBe('function'); - expect(proxiedHandler).not.toBe(mockHandler); - - expect((proxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - - const proxyHandler = (proxiedHandler as any)?.__sentry_proxied__; - expect(proxyHandler).toBe(true); - }); - - it('should handle routes without handle property', () => { - const route: RouteObject = { - path: '/test', - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with non-function handle properties', () => { - const route: RouteObject = { - path: '/test', - handle: { - someData: 'not a function', - }, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with null/undefined handle properties', () => { - const route: RouteObject = { - path: '/test', - handle: null as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with mixed function and non-function handle properties', () => { - const mockHandler = () => Promise.resolve([]); - const route: RouteObject = { - path: '/test', - handle: { - lazyChildren: mockHandler, - someData: 'not a function', - anotherData: 123, - }, - }; - - checkRouteForAsyncHandler(route); - - const proxiedHandler = route.handle?.lazyChildren; - expect(typeof proxiedHandler).toBe('function'); - expect(proxiedHandler).not.toBe(mockHandler); - expect((proxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - - // Non-function properties should remain unchanged - expect(route.handle?.someData).toBe('not a function'); - expect(route.handle?.anotherData).toBe(123); - }); - - it('should handle nested routes with async handlers', () => { - const parentHandler = () => Promise.resolve([]); - const childHandler = () => Promise.resolve([]); - - const route: RouteObject = { - path: '/parent', - handle: { - lazyChildren: parentHandler, - }, - children: [ - { - path: '/child', - handle: { - lazyChildren: childHandler, - }, - }, - ], - }; - - checkRouteForAsyncHandler(route); - - // Check parent handler is proxied - const proxiedParentHandler = route.handle?.lazyChildren; - expect(typeof proxiedParentHandler).toBe('function'); - expect(proxiedParentHandler).not.toBe(parentHandler); - expect((proxiedParentHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - - // Check child handler is proxied - const proxiedChildHandler = route.children?.[0]?.handle?.lazyChildren; - expect(typeof proxiedChildHandler).toBe('function'); - expect(proxiedChildHandler).not.toBe(childHandler); - expect((proxiedChildHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - }); - - it('should handle deeply nested routes', () => { - const level1Handler = () => Promise.resolve([]); - const level2Handler = () => Promise.resolve([]); - const level3Handler = () => Promise.resolve([]); - - const route: RouteObject = { - path: '/level1', - handle: { - lazyChildren: level1Handler, - }, - children: [ - { - path: '/level2', - handle: { - lazyChildren: level2Handler, - }, - children: [ - { - path: '/level3', - handle: { - lazyChildren: level3Handler, - }, - }, - ], - }, - ], - }; - - checkRouteForAsyncHandler(route); - - // Check all handlers are proxied - expect((route.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - expect((route.children?.[0]?.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe( - true, - ); - expect( - (route.children?.[0]?.children?.[0]?.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__, - ).toBe(true); - }); - - it('should handle routes with multiple async handlers', () => { - const handler1 = () => Promise.resolve([]); - const handler2 = () => Promise.resolve([]); - const handler3 = () => Promise.resolve([]); - - const route: RouteObject = { - path: '/test', - handle: { - lazyChildren: handler1, - asyncLoader: handler2, - dataLoader: handler3, - }, - }; - - checkRouteForAsyncHandler(route); - - // Check all handlers are proxied - expect((route.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - expect((route.handle?.asyncLoader as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - expect((route.handle?.dataLoader as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - }); - - it('should not re-proxy already proxied functions', () => { - const mockHandler = () => Promise.resolve([]); - const route: RouteObject = { - path: '/test', - handle: { - lazyChildren: mockHandler, - }, - }; - - // First call should proxy the function - checkRouteForAsyncHandler(route); - const firstProxiedHandler = route.handle?.lazyChildren; - expect(firstProxiedHandler).not.toBe(mockHandler); - expect((firstProxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - - // Second call should not create a new proxy - checkRouteForAsyncHandler(route); - const secondProxiedHandler = route.handle?.lazyChildren; - expect(secondProxiedHandler).toBe(firstProxiedHandler); // Should be the same proxy - expect((secondProxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); - }); - - it('should handle routes with empty children array', () => { - const route: RouteObject = { - path: '/test', - children: [], - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with undefined children', () => { - const route: RouteObject = { - path: '/test', - children: undefined, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with null children', () => { - const route: RouteObject = { - path: '/test', - children: null as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with non-array children', () => { - const route: RouteObject = { - path: '/test', - children: 'not an array' as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is not an object', () => { - const route: RouteObject = { - path: '/test', - handle: 'not an object' as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is null', () => { - const route: RouteObject = { - path: '/test', - handle: null as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is undefined', () => { - const route: RouteObject = { - path: '/test', - handle: undefined as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is a function', () => { - const route: RouteObject = { - path: '/test', - handle: (() => {}) as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is a string', () => { - const route: RouteObject = { - path: '/test', - handle: 'string handle' as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is a number', () => { - const route: RouteObject = { - path: '/test', - handle: 42 as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is a boolean', () => { - const route: RouteObject = { - path: '/test', - handle: true as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); - - it('should handle routes with handle that is an array', () => { - const route: RouteObject = { - path: '/test', - handle: [] as any, - }; - - expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); - }); -}); diff --git a/packages/sveltekit/src/server-common/handle.ts b/packages/sveltekit/src/server-common/handle.ts index 696c3d765c5b..3f5797efd211 100644 --- a/packages/sveltekit/src/server-common/handle.ts +++ b/packages/sveltekit/src/server-common/handle.ts @@ -7,10 +7,13 @@ import { getDefaultIsolationScope, getIsolationScope, getTraceMetaTags, + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, setHttpStatus, + spanToJSON, startSpan, + updateSpanName, winterCGRequestToRequestData, withIsolationScope, } from '@sentry/core'; @@ -88,11 +91,33 @@ export function isFetchProxyRequired(version: string): boolean { return true; } +interface BackwardsForwardsCompatibleEvent { + /** + * For now taken from: https://github.com/sveltejs/kit/pull/13899 + * Access to spans for tracing. If tracing is not enabled or the function is being run in the browser, these spans will do nothing. + * @since 2.31.0 + */ + tracing?: { + /** Whether tracing is enabled. */ + enabled: boolean; + current: Span; + root: Span; + }; +} + async function instrumentHandle( - { event, resolve }: Parameters[0], + { + event, + resolve, + }: { + event: Parameters[0]['event'] & BackwardsForwardsCompatibleEvent; + resolve: Parameters[0]['resolve']; + }, options: SentryHandleOptions, ): Promise { - if (!event.route?.id && !options.handleUnknownRoutes) { + const routeId = event.route?.id; + + if (!routeId && !options.handleUnknownRoutes) { return resolve(event); } @@ -108,7 +133,7 @@ async function instrumentHandle( } } - const routeName = `${event.request.method} ${event.route?.id || event.url.pathname}`; + const routeName = `${event.request.method} ${routeId || event.url.pathname}`; if (getIsolationScope() !== getDefaultIsolationScope()) { getIsolationScope().setTransactionName(routeName); @@ -116,34 +141,72 @@ async function instrumentHandle( DEBUG_BUILD && debug.warn('Isolation scope is default isolation scope - skipping setting transactionName'); } + // We only start a span if SvelteKit's native tracing is not enabled. Two reasons: + // - Used Kit version doesn't yet support tracing + // - Users didn't enable tracing + const kitTracingEnabled = event.tracing?.enabled; + try { - const resolveResult = await startSpan( - { - op: 'http.server', - attributes: { + const resolveWithSentry: (sentrySpan?: Span) => Promise = async (sentrySpan?: Span) => { + getCurrentScope().setSDKProcessingMetadata({ + // We specifically avoid cloning the request here to avoid double read errors. + // We only read request headers so we're not consuming the body anyway. + // Note to future readers: This sounds counter-intuitive but please read + // https://github.com/getsentry/sentry-javascript/issues/14583 + normalizedRequest: winterCGRequestToRequestData(event.request), + }); + const kitRootSpan = event.tracing?.enabled ? event.tracing?.root : undefined; + + if (kitRootSpan) { + // Update the root span emitted from SvelteKit to resemble a `http.server` span + // We're doing this here instead of an event processor to ensure we update the + // span name as early as possible (for dynamic sampling, et al.) + // Other spans are enhanced in the `processKitSpans` integration. + const spanJson = spanToJSON(kitRootSpan); + const kitRootSpanAttributes = spanJson.data; + const originalName = spanJson.description; + + const routeName = kitRootSpanAttributes['http.route']; + if (routeName && typeof routeName === 'string') { + updateSpanName(kitRootSpan, `${event.request.method ?? 'GET'} ${routeName}`); + } + + kitRootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: event.route?.id ? 'route' : 'url', - 'http.method': event.request.method, - }, - name: routeName, - }, - async (span?: Span) => { - getCurrentScope().setSDKProcessingMetadata({ - // We specifically avoid cloning the request here to avoid double read errors. - // We only read request headers so we're not consuming the body anyway. - // Note to future readers: This sounds counter-intuitive but please read - // https://github.com/getsentry/sentry-javascript/issues/14583 - normalizedRequest: winterCGRequestToRequestData(event.request), + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeName ? 'route' : 'url', + 'sveltekit.tracing.original_name': originalName, }); - const res = await resolve(event, { - transformPageChunk: addSentryCodeToPage({ injectFetchProxyScript: options.injectFetchProxyScript ?? true }), - }); - if (span) { - setHttpStatus(span, res.status); - } - return res; - }, - ); + } + + const res = await resolve(event, { + transformPageChunk: addSentryCodeToPage({ + injectFetchProxyScript: options.injectFetchProxyScript ?? true, + }), + }); + + if (sentrySpan) { + setHttpStatus(sentrySpan, res.status); + } + + return res; + }; + + const resolveResult = kitTracingEnabled + ? await resolveWithSentry() + : await startSpan( + { + op: 'http.server', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeId ? 'route' : 'url', + 'http.method': event.request.method, + }, + name: routeName, + }, + resolveWithSentry, + ); + return resolveResult; } catch (e: unknown) { sendErrorToSentry(e, 'handle'); @@ -176,9 +239,12 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle { }; const sentryRequestHandler: Handle = input => { + const backwardsForwardsCompatibleEvent = input.event as typeof input.event & BackwardsForwardsCompatibleEvent; + // Escape hatch to suppress request isolation and trace continuation (see initCloudflareSentryHandle) const skipIsolation = - '_sentrySkipRequestIsolation' in input.event.locals && input.event.locals._sentrySkipRequestIsolation; + '_sentrySkipRequestIsolation' in backwardsForwardsCompatibleEvent.locals && + backwardsForwardsCompatibleEvent.locals._sentrySkipRequestIsolation; // In case of a same-origin `fetch` call within a server`load` function, // SvelteKit will actually just re-enter the `handle` function and set `isSubRequest` @@ -187,7 +253,9 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle { // currently active span instead of a new root span to correctly reflect this // behavior. if (skipIsolation || input.event.isSubRequest) { - return instrumentHandle(input, options); + return instrumentHandle(input, { + ...options, + }); } return withIsolationScope(isolationScope => { @@ -200,6 +268,13 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle { // https://github.com/getsentry/sentry-javascript/issues/14583 normalizedRequest: winterCGRequestToRequestData(input.event.request), }); + + if (backwardsForwardsCompatibleEvent.tracing?.enabled) { + // if sveltekit tracing is enabled (since 2.31.0), trace continuation is handled by + // kit before our hook is executed. No noeed to call `continueTrace` from our end + return instrumentHandle(input, options); + } + return continueTrace(getTracePropagationData(input.event), () => instrumentHandle(input, options)); }); }; diff --git a/packages/sveltekit/src/server-common/rewriteFramesIntegration.ts b/packages/sveltekit/src/server-common/integrations/rewriteFramesIntegration.ts similarity index 95% rename from packages/sveltekit/src/server-common/rewriteFramesIntegration.ts rename to packages/sveltekit/src/server-common/integrations/rewriteFramesIntegration.ts index 412dd6f98815..9f6f0add6944 100644 --- a/packages/sveltekit/src/server-common/rewriteFramesIntegration.ts +++ b/packages/sveltekit/src/server-common/integrations/rewriteFramesIntegration.ts @@ -7,8 +7,8 @@ import { join, rewriteFramesIntegration as originalRewriteFramesIntegration, } from '@sentry/core'; -import { WRAPPED_MODULE_SUFFIX } from '../common/utils'; -import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; +import { WRAPPED_MODULE_SUFFIX } from '../../common/utils'; +import type { GlobalWithSentryValues } from '../../vite/injectGlobalValues'; type StackFrameIteratee = (frame: StackFrame) => StackFrame; interface RewriteFramesOptions { diff --git a/packages/sveltekit/src/server-common/integrations/svelteKitSpans.ts b/packages/sveltekit/src/server-common/integrations/svelteKitSpans.ts new file mode 100644 index 000000000000..5ab24a731279 --- /dev/null +++ b/packages/sveltekit/src/server-common/integrations/svelteKitSpans.ts @@ -0,0 +1,78 @@ +import type { Integration, SpanOrigin } from '@sentry/core'; +import { type SpanJSON, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; + +/** + * A small integration that preprocesses spans so that SvelteKit-generated spans + * (via Kit's tracing feature since 2.31.0) get the correct Sentry attributes + * and data. + */ +export function svelteKitSpansIntegration(): Integration { + return { + name: 'SvelteKitSpansEnhancement', + // Using preprocessEvent to ensure the processing happens before user-configured + // event processors are executed + preprocessEvent(event) { + // only iterate over the spans if the root span was emitted by SvelteKit + // TODO: Right now, we can't optimize this to only check traces with a kit-emitted root span + // this is because in Cloudflare, the kit-emitted root span is missing but our cloudflare + // SDK emits the http.server span. + if (event.type === 'transaction') { + event.spans?.forEach(_enhanceKitSpan); + } + }, + }; +} + +/** + * Adds sentry-specific attributes and data to a span emitted by SvelteKit's native tracing (since 2.31.0) + * @exported for testing + */ +export function _enhanceKitSpan(span: SpanJSON): void { + let op: string | undefined = undefined; + let origin: SpanOrigin | undefined = undefined; + + const spanName = span.description; + + const previousOp = span.op || span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]; + const previousOrigin = span.origin || span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]; + + switch (spanName) { + case 'sveltekit.resolve': + op = 'function.sveltekit.resolve'; + origin = 'auto.http.sveltekit'; + break; + case 'sveltekit.load': + op = 'function.sveltekit.load'; + origin = 'auto.function.sveltekit.load'; + break; + case 'sveltekit.form_action': + op = 'function.sveltekit.form_action'; + origin = 'auto.function.sveltekit.action'; + break; + case 'sveltekit.remote.call': + op = 'function.sveltekit.remote'; + origin = 'auto.rpc.sveltekit.remote'; + break; + case 'sveltekit.handle.root': + // We don't want to overwrite the root handle span at this point since + // we already enhance the root span in our `sentryHandle` hook. + break; + default: { + if (spanName?.startsWith('sveltekit.handle.sequenced.')) { + op = 'function.sveltekit.handle'; + origin = 'auto.function.sveltekit.handle'; + } + break; + } + } + + if (!previousOp && op) { + span.op = op; + span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP] = op; + } + + if ((!previousOrigin || previousOrigin === 'manual') && origin) { + span.origin = origin; + span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] = origin; + } +} diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 56400dcc5423..d287331df14d 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -53,7 +53,6 @@ export { getTraceMetaTags, graphqlIntegration, hapiIntegration, - httpIntegration, // eslint-disable-next-line deprecation/deprecation inboundFiltersIntegration, eventFiltersIntegration, @@ -141,6 +140,7 @@ export { wrapLoadWithSentry, wrapServerLoadWithSentry } from '../server-common/l export { sentryHandle } from '../server-common/handle'; export { initCloudflareSentryHandle } from './handle'; export { wrapServerRouteWithSentry } from '../server-common/serverRoute'; +export { httpIntegration } from './integrations/http'; /** * Tracks the Svelte component's initialization and mounting operation as well as diff --git a/packages/sveltekit/src/server/integrations/http.ts b/packages/sveltekit/src/server/integrations/http.ts new file mode 100644 index 000000000000..4d6844017d1d --- /dev/null +++ b/packages/sveltekit/src/server/integrations/http.ts @@ -0,0 +1,39 @@ +import type { IntegrationFn } from '@sentry/core'; +import { httpIntegration as originalHttpIntegration } from '@sentry/node'; + +type HttpOptions = Parameters[0]; + +/** + * The http integration instruments Node's internal http and https modules. + * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span. + * + * For SvelteKit, does not create spans for incoming requests but instead we use SvelteKit's own spans. + * If you need to create incoming spans, set the `disableIncomingRequestSpans` option to `false`. + * (You likely don't need this!) + * + */ +export const httpIntegration = ((options: HttpOptions = {}) => { + /* + * This is a slightly modified version of the original httpIntegration: We avoid creating + * incoming request spans because: + * + * - If Kit-tracing is available and enabled, we take the `sveltekit.handle.root` span + * as the root span and make it the `http.server` span. This gives us a single root + * span across all deployment plaftorms (while httpIntegration doesn't apply on e.g. + * AWS Lambda or edge) + * - If Kit-tracing is N/A or disabled and users follow the current/old docs, httpIntegration + * does nothing anyway, so this isn't a concern. + * - Which leaves the undocumented case that users --import an instrument.mjs file + * in which they initialize the SDK. IMHO it's fine to ignore this for now since it was + * well ... undocumented. Given in the future there won't be be an easy way for us + * to detect where the SDK is initialized, we should simply redirect users to use + * instrumentation.server.ts instead. If users want to, they can simply import and + * register `httpIntegration` and explicitly enable incoming request spans. + */ + + return originalHttpIntegration({ + // We disable incoming request spans here, because otherwise we'd end up with duplicate spans. + disableIncomingRequestSpans: true, + ...options, + }); +}) satisfies IntegrationFn; diff --git a/packages/sveltekit/src/server/sdk.ts b/packages/sveltekit/src/server/sdk.ts index 19a0a8f9f5ad..fb7a5dbbb471 100644 --- a/packages/sveltekit/src/server/sdk.ts +++ b/packages/sveltekit/src/server/sdk.ts @@ -1,15 +1,24 @@ import { applySdkMetadata } from '@sentry/core'; import type { NodeClient, NodeOptions } from '@sentry/node'; import { getDefaultIntegrations as getDefaultNodeIntegrations, init as initNodeSdk } from '@sentry/node'; -import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegration'; +import { rewriteFramesIntegration } from '../server-common/integrations/rewriteFramesIntegration'; +import { svelteKitSpansIntegration } from '../server-common/integrations/svelteKitSpans'; +import { httpIntegration } from './integrations/http'; /** * Initialize the Server-side Sentry SDK * @param options */ export function init(options: NodeOptions): NodeClient | undefined { + const defaultIntegrations = [ + ...getDefaultNodeIntegrations(options).filter(integration => integration.name !== 'Http'), + rewriteFramesIntegration(), + httpIntegration(), + svelteKitSpansIntegration(), + ]; + const opts = { - defaultIntegrations: [...getDefaultNodeIntegrations(options), rewriteFramesIntegration()], + defaultIntegrations, ...options, }; diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts index 63f4888257de..58862e452ddc 100644 --- a/packages/sveltekit/src/vite/autoInstrument.ts +++ b/packages/sveltekit/src/vite/autoInstrument.ts @@ -27,6 +27,7 @@ export type AutoInstrumentSelection = { type AutoInstrumentPluginOptions = AutoInstrumentSelection & { debug: boolean; + onlyInstrumentClient: boolean; }; /** @@ -41,12 +42,26 @@ type AutoInstrumentPluginOptions = AutoInstrumentSelection & { export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptions): Plugin { const { load: wrapLoadEnabled, serverLoad: wrapServerLoadEnabled, debug } = options; + let isServerBuild: boolean | undefined = undefined; + return { name: 'sentry-auto-instrumentation', // This plugin needs to run as early as possible, before the SvelteKit plugin virtualizes all paths and ids enforce: 'pre', + configResolved: config => { + // The SvelteKit plugins trigger additional builds within the main (SSR) build. + // We just need a mechanism to upload source maps only once. + // `config.build.ssr` is `true` for that first build and `false` in the other ones. + // Hence we can use it as a switch to upload source maps only once in main build. + isServerBuild = !!config.build.ssr; + }, + async load(id) { + if (options.onlyInstrumentClient && isServerBuild) { + return null; + } + const applyUniversalLoadWrapper = wrapLoadEnabled && /^\+(page|layout)\.(js|ts|mjs|mts)$/.test(path.basename(id)) && @@ -58,6 +73,12 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio return getWrapperCode('wrapLoadWithSentry', `${id}${WRAPPED_MODULE_SUFFIX}`); } + if (options.onlyInstrumentClient) { + // Now that we've checked universal files, we can early return and avoid further + // regexp checks below for server-only files, in case `onlyInstrumentClient` is `true`. + return null; + } + const applyServerLoadWrapper = wrapServerLoadEnabled && /^\+(page|layout)\.server\.(js|ts|mjs|mts)$/.test(path.basename(id)) && diff --git a/packages/sveltekit/src/vite/injectGlobalValues.ts b/packages/sveltekit/src/vite/injectGlobalValues.ts index 96ad05123ce6..20f446b6b46f 100644 --- a/packages/sveltekit/src/vite/injectGlobalValues.ts +++ b/packages/sveltekit/src/vite/injectGlobalValues.ts @@ -1,4 +1,8 @@ -import type { InternalGlobal } from '@sentry/core'; +import { type InternalGlobal, escapeStringForRegex } from '@sentry/core'; +import MagicString from 'magic-string'; +import type { Plugin } from 'vite'; +import { type BackwardsForwardsCompatibleSvelteConfig, getAdapterOutputDir, getHooksFileName } from './svelteConfig'; +import type { SentrySvelteKitPluginOptions } from './types'; export type GlobalSentryValues = { __sentry_sveltekit_output_dir?: string; @@ -27,3 +31,71 @@ export function getGlobalValueInjectionCode(globalSentryValues: GlobalSentryValu return `${injectedValuesCode}\n`; } + +/** + * Injects SvelteKit app configuration values the svelte.config.js into the + * server's global object so that the SDK can pick up the information at runtime + */ +export async function makeGlobalValuesInjectionPlugin( + svelteConfig: BackwardsForwardsCompatibleSvelteConfig, + options: Pick, +): Promise { + const { adapter = 'other', debug = false } = options; + + const serverHooksFile = getHooksFileName(svelteConfig, 'server'); + const adapterOutputDir = await getAdapterOutputDir(svelteConfig, adapter); + + const globalSentryValues: GlobalSentryValues = { + __sentry_sveltekit_output_dir: adapterOutputDir, + }; + + if (debug) { + // eslint-disable-next-line no-console + console.log('[Sentry SvelteKit] Global values:', globalSentryValues); + } + + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway + const hooksFileRegexp = new RegExp(`/${escapeStringForRegex(serverHooksFile)}(.(js|ts|mjs|mts))?`); + + return { + name: 'sentry-sveltekit-global-values-injection-plugin', + resolveId: (id, _importer, _ref) => { + if (id === VIRTUAL_GLOBAL_VALUES_FILE) { + return { + id: VIRTUAL_GLOBAL_VALUES_FILE, + external: false, + moduleSideEffects: true, + }; + } + return null; + }, + + load: id => { + if (id === VIRTUAL_GLOBAL_VALUES_FILE) { + return { + code: getGlobalValueInjectionCode(globalSentryValues), + }; + } + return null; + }, + + transform: async (code, id) => { + const isServerEntryFile = /instrumentation\.server\./.test(id) || hooksFileRegexp.test(id); + + if (isServerEntryFile) { + if (debug) { + // eslint-disable-next-line no-console + console.log('[Global Values Plugin] Injecting global values into', id); + } + const ms = new MagicString(code); + ms.append(`\n; import "${VIRTUAL_GLOBAL_VALUES_FILE}";\n`); + return { + code: ms.toString(), + map: ms.generateMap({ hires: true }), + }; + } + + return null; + }, + }; +} diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 4444ba9a6ab7..61b388a94cf2 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -2,7 +2,9 @@ import type { Plugin } from 'vite'; import type { AutoInstrumentSelection } from './autoInstrument'; import { makeAutoInstrumentationPlugin } from './autoInstrument'; import { detectAdapter } from './detectAdapter'; +import { makeGlobalValuesInjectionPlugin } from './injectGlobalValues'; import { makeCustomSentryVitePlugins } from './sourceMaps'; +import { loadSvelteConfig } from './svelteConfig'; import type { CustomSentryVitePluginOptions, SentrySvelteKitPluginOptions } from './types'; const DEFAULT_PLUGIN_OPTIONS: SentrySvelteKitPluginOptions = { @@ -25,9 +27,14 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} adapter: options.adapter || (await detectAdapter(options.debug)), }; + const svelteConfig = await loadSvelteConfig(); + const sentryPlugins: Plugin[] = []; if (mergedOptions.autoInstrument) { + // TODO: Once tracing is promoted stable, we need to adjust this check! + const kitTracingEnabled = !!svelteConfig.kit?.experimental?.tracing?.server; + const pluginOptions: AutoInstrumentSelection = { load: true, serverLoad: true, @@ -38,15 +45,26 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} makeAutoInstrumentationPlugin({ ...pluginOptions, debug: options.debug || false, + // if kit-internal tracing is enabled, we only want to wrap and instrument client-side code. + onlyInstrumentClient: kitTracingEnabled, }), ); } const sentryVitePluginsOptions = generateVitePluginOptions(mergedOptions); - if (sentryVitePluginsOptions) { - const sentryVitePlugins = await makeCustomSentryVitePlugins(sentryVitePluginsOptions); + if (mergedOptions.autoUploadSourceMaps) { + // When source maps are enabled, we need to inject the output directory to get a correct + // stack trace, by using this SDK's `rewriteFrames` integration. + // This integration picks up the value. + // TODO: I don't think this is technically correct. Either we always or never inject the output directory. + // Stack traces shouldn't be different, depending on source maps config. With debugIds, we might not even + // need to rewrite frames anymore. + sentryPlugins.push(await makeGlobalValuesInjectionPlugin(svelteConfig, mergedOptions)); + } + if (sentryVitePluginsOptions) { + const sentryVitePlugins = await makeCustomSentryVitePlugins(sentryVitePluginsOptions, svelteConfig); sentryPlugins.push(...sentryVitePlugins); } diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index eb3b449144f8..57bf21055bf1 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -1,17 +1,14 @@ -/* eslint-disable max-lines */ import { escapeStringForRegex, uuid4 } from '@sentry/core'; import { getSentryRelease } from '@sentry/node'; import type { SentryVitePluginOptions } from '@sentry/vite-plugin'; import { sentryVitePlugin } from '@sentry/vite-plugin'; import * as child_process from 'child_process'; import * as fs from 'fs'; -import MagicString from 'magic-string'; import * as path from 'path'; import type { Plugin, UserConfig } from 'vite'; import { WRAPPED_MODULE_SUFFIX } from '../common/utils'; -import type { GlobalSentryValues } from './injectGlobalValues'; -import { getGlobalValueInjectionCode, VIRTUAL_GLOBAL_VALUES_FILE } from './injectGlobalValues'; -import { getAdapterOutputDir, getHooksFileName, loadSvelteConfig } from './svelteConfig'; +import type { BackwardsForwardsCompatibleSvelteConfig } from './svelteConfig'; +import { getAdapterOutputDir } from './svelteConfig'; import type { CustomSentryVitePluginOptions } from './types'; // sorcery has no types, so these are some basic type definitions: @@ -45,9 +42,10 @@ type FilesToDeleteAfterUpload = string | string[] | undefined; * * @returns the custom Sentry Vite plugin */ -export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePluginOptions): Promise { - const svelteConfig = await loadSvelteConfig(); - +export async function makeCustomSentryVitePlugins( + options: CustomSentryVitePluginOptions, + svelteConfig: BackwardsForwardsCompatibleSvelteConfig, +): Promise { const usedAdapter = options?.adapter || 'other'; const adapterOutputDir = await getAdapterOutputDir(svelteConfig, usedAdapter); @@ -149,12 +147,6 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug let isSSRBuild = true; - const serverHooksFile = getHooksFileName(svelteConfig, 'server'); - - const globalSentryValues: GlobalSentryValues = { - __sentry_sveltekit_output_dir: adapterOutputDir, - }; - const sourceMapSettingsPlugin: Plugin = { name: 'sentry-sveltekit-update-source-map-setting-plugin', apply: 'build', // only apply this plugin at build time @@ -202,26 +194,6 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug name: 'sentry-sveltekit-debug-id-upload-plugin', apply: 'build', // only apply this plugin at build time enforce: 'post', // this needs to be set to post, otherwise we don't pick up the output from the SvelteKit adapter - resolveId: (id, _importer, _ref) => { - if (id === VIRTUAL_GLOBAL_VALUES_FILE) { - return { - id: VIRTUAL_GLOBAL_VALUES_FILE, - external: false, - moduleSideEffects: true, - }; - } - return null; - }, - - load: id => { - if (id === VIRTUAL_GLOBAL_VALUES_FILE) { - return { - code: getGlobalValueInjectionCode(globalSentryValues), - }; - } - return null; - }, - configResolved: config => { // The SvelteKit plugins trigger additional builds within the main (SSR) build. // We just need a mechanism to upload source maps only once. @@ -232,22 +204,6 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug } }, - transform: async (code, id) => { - // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway - const isServerHooksFile = new RegExp(`/${escapeStringForRegex(serverHooksFile)}(.(js|ts|mjs|mts))?`).test(id); - - if (isServerHooksFile) { - const ms = new MagicString(code); - ms.append(`\n; import "${VIRTUAL_GLOBAL_VALUES_FILE}";\n`); - return { - code: ms.toString(), - map: ms.generateMap({ hires: true }), - }; - } - - return null; - }, - // We need to start uploading source maps later than in the original plugin // because SvelteKit is invoking the adapter at closeBundle. // This means that we need to wait until the adapter is done before we start uploading. diff --git a/packages/sveltekit/src/vite/svelteConfig.ts b/packages/sveltekit/src/vite/svelteConfig.ts index 34874bfd2f97..f1e908af9c93 100644 --- a/packages/sveltekit/src/vite/svelteConfig.ts +++ b/packages/sveltekit/src/vite/svelteConfig.ts @@ -4,12 +4,32 @@ import * as path from 'path'; import * as url from 'url'; import type { SupportedSvelteKitAdapters } from './detectAdapter'; +export type SvelteKitTracingConfig = { + tracing?: { + server: boolean; + }; + // TODO: Once instrumentation is promoted stable, this will be removed! + instrumentation?: { + server: boolean; + }; +}; + +/** + * Experimental tracing and instrumentation config is available @since 2.31.0 + * // TODO: Once instrumentation and tracing is promoted stable, adjust this type!s + */ +type BackwardsForwardsCompatibleKitConfig = Config['kit'] & { experimental?: SvelteKitTracingConfig }; + +export interface BackwardsForwardsCompatibleSvelteConfig extends Config { + kit?: BackwardsForwardsCompatibleKitConfig; +} + /** * Imports the svelte.config.js file and returns the config object. * The sveltekit plugins import the config in the same way. * See: https://github.com/sveltejs/kit/blob/master/packages/kit/src/core/config/index.js#L63 */ -export async function loadSvelteConfig(): Promise { +export async function loadSvelteConfig(): Promise { // This can only be .js (see https://github.com/sveltejs/kit/pull/4031#issuecomment-1049475388) const SVELTE_CONFIG_FILE = 'svelte.config.js'; @@ -23,7 +43,7 @@ export async function loadSvelteConfig(): Promise { const svelteConfigModule = await import(`${url.pathToFileURL(configFile).href}`); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return (svelteConfigModule?.default as Config) || {}; + return (svelteConfigModule?.default as BackwardsForwardsCompatibleSvelteConfig) || {}; } catch (e) { // eslint-disable-next-line no-console console.warn("[Source Maps Plugin] Couldn't load svelte.config.js:"); diff --git a/packages/sveltekit/src/vite/types.ts b/packages/sveltekit/src/vite/types.ts index 0f6717a2c7e9..4267ce378bb1 100644 --- a/packages/sveltekit/src/vite/types.ts +++ b/packages/sveltekit/src/vite/types.ts @@ -219,6 +219,7 @@ export type SentrySvelteKitPluginOptions = { * @default true`. */ autoUploadSourceMaps?: boolean; + /** * Options related to source maps upload to Sentry */ diff --git a/packages/sveltekit/src/worker/cloudflare.ts b/packages/sveltekit/src/worker/cloudflare.ts index b27ceba87780..612b174f6c69 100644 --- a/packages/sveltekit/src/worker/cloudflare.ts +++ b/packages/sveltekit/src/worker/cloudflare.ts @@ -6,7 +6,8 @@ import { } from '@sentry/cloudflare'; import { addNonEnumerableProperty } from '@sentry/core'; import type { Handle } from '@sveltejs/kit'; -import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegration'; +import { rewriteFramesIntegration } from '../server-common/integrations/rewriteFramesIntegration'; +import { svelteKitSpansIntegration } from '../server-common/integrations/svelteKitSpans'; /** * Initializes Sentry SvelteKit Cloudflare SDK @@ -16,7 +17,11 @@ import { rewriteFramesIntegration } from '../server-common/rewriteFramesIntegrat */ export function initCloudflareSentryHandle(options: CloudflareOptions): Handle { const opts: CloudflareOptions = { - defaultIntegrations: [...getDefaultCloudflareIntegrations(options), rewriteFramesIntegration()], + defaultIntegrations: [ + ...getDefaultCloudflareIntegrations(options), + rewriteFramesIntegration(), + svelteKitSpansIntegration(), + ], ...options, }; diff --git a/packages/sveltekit/test/server-common/handle.test.ts b/packages/sveltekit/test/server-common/handle.test.ts index 79c0f88e0b5d..db1e1fe4811f 100644 --- a/packages/sveltekit/test/server-common/handle.test.ts +++ b/packages/sveltekit/test/server-common/handle.test.ts @@ -111,7 +111,7 @@ describe('sentryHandle', () => { [Type.Async, true, undefined], [Type.Async, false, mockResponse], ])('%s resolve with error %s', (type, isError, mockResponse) => { - it('should return a response', async () => { + it('returns a response', async () => { let response: any = undefined; try { response = await sentryHandle()({ event: mockEvent(), resolve: resolve(type, isError) }); @@ -123,7 +123,7 @@ describe('sentryHandle', () => { expect(response).toEqual(mockResponse); }); - it("creates a transaction if there's no active span", async () => { + it("starts a span if there's no active span", async () => { let _span: Span | undefined = undefined; client.on('spanEnd', span => { if (span === getRootSpan(span)) { @@ -150,7 +150,27 @@ describe('sentryHandle', () => { expect(spans).toHaveLength(1); }); - it('creates a child span for nested server calls (i.e. if there is an active span)', async () => { + it("doesn't start a span if sveltekit tracing is enabled", async () => { + let _span: Span | undefined = undefined; + client.on('spanEnd', span => { + if (span === getRootSpan(span)) { + _span = span; + } + }); + + try { + await sentryHandle()({ + event: mockEvent({ tracing: { enabled: true } }), + resolve: resolve(type, isError), + }); + } catch { + // + } + + expect(_span).toBeUndefined(); + }); + + it('starts a child span for nested server calls (i.e. if there is an active span)', async () => { let _span: Span | undefined = undefined; let txnCount = 0; client.on('spanEnd', span => { @@ -197,7 +217,7 @@ describe('sentryHandle', () => { ); }); - it("creates a transaction from sentry-trace header but doesn't populate a new DSC", async () => { + it("starts a span from sentry-trace header but doesn't populate a new DSC", async () => { const event = mockEvent({ request: { headers: { diff --git a/packages/sveltekit/test/server-common/rewriteFramesIntegration.test.ts b/packages/sveltekit/test/server-common/integrations/rewriteFramesIntegration.test.ts similarity index 93% rename from packages/sveltekit/test/server-common/rewriteFramesIntegration.test.ts rename to packages/sveltekit/test/server-common/integrations/rewriteFramesIntegration.test.ts index 20f9c52a8e27..836152a81eb0 100644 --- a/packages/sveltekit/test/server-common/rewriteFramesIntegration.test.ts +++ b/packages/sveltekit/test/server-common/integrations/rewriteFramesIntegration.test.ts @@ -2,8 +2,8 @@ import { rewriteFramesIntegration } from '@sentry/browser'; import type { Event, StackFrame } from '@sentry/core'; import { basename } from '@sentry/core'; import { describe, expect, it } from 'vitest'; -import { rewriteFramesIteratee } from '../../src/server-common/rewriteFramesIntegration'; -import type { GlobalWithSentryValues } from '../../src/vite/injectGlobalValues'; +import { rewriteFramesIteratee } from '../../../src/server-common/integrations/rewriteFramesIntegration'; +import type { GlobalWithSentryValues } from '../../../src/vite/injectGlobalValues'; describe('rewriteFramesIteratee', () => { it('removes the module property from the frame', () => { diff --git a/packages/sveltekit/test/server-common/integrations/svelteKitSpans.test.ts b/packages/sveltekit/test/server-common/integrations/svelteKitSpans.test.ts new file mode 100644 index 000000000000..0d95cb3d6fb6 --- /dev/null +++ b/packages/sveltekit/test/server-common/integrations/svelteKitSpans.test.ts @@ -0,0 +1,172 @@ +import type { SpanJSON, TransactionEvent } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; +import { describe, expect, it } from 'vitest'; +import { _enhanceKitSpan, svelteKitSpansIntegration } from '../../../src/server-common/integrations/svelteKitSpans'; + +describe('svelteKitSpansIntegration', () => { + it('has a name and a preprocessEventHook', () => { + const integration = svelteKitSpansIntegration(); + + expect(integration.name).toBe('SvelteKitSpansEnhancement'); + expect(typeof integration.preprocessEvent).toBe('function'); + }); + + it('enhances spans from SvelteKit', () => { + const event: TransactionEvent = { + type: 'transaction', + contexts: { + trace: { + span_id: '123', + trace_id: 'abc', + data: { + 'sveltekit.tracing.original_name': 'sveltekit.handle.root', + }, + }, + }, + spans: [ + { + description: 'sveltekit.resolve', + data: { + someAttribute: 'someValue', + }, + span_id: '123', + trace_id: 'abc', + start_timestamp: 0, + }, + ], + }; + + // @ts-expect-error -- passing in an empty option for client but it is unused in the integration + svelteKitSpansIntegration().preprocessEvent?.(event, {}, {}); + + expect(event.spans).toHaveLength(1); + expect(event.spans?.[0]?.op).toBe('function.sveltekit.resolve'); + expect(event.spans?.[0]?.origin).toBe('auto.http.sveltekit'); + expect(event.spans?.[0]?.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('function.sveltekit.resolve'); + expect(event.spans?.[0]?.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toBe('auto.http.sveltekit'); + }); + + describe('_enhanceKitSpan', () => { + it.each([ + ['sveltekit.resolve', 'function.sveltekit.resolve', 'auto.http.sveltekit'], + ['sveltekit.load', 'function.sveltekit.load', 'auto.function.sveltekit.load'], + ['sveltekit.form_action', 'function.sveltekit.form_action', 'auto.function.sveltekit.action'], + ['sveltekit.remote.call', 'function.sveltekit.remote', 'auto.rpc.sveltekit.remote'], + ['sveltekit.handle.sequenced.0', 'function.sveltekit.handle', 'auto.function.sveltekit.handle'], + ['sveltekit.handle.sequenced.myHandler', 'function.sveltekit.handle', 'auto.function.sveltekit.handle'], + ])('enhances %s span with the correct op and origin', (spanName, op, origin) => { + const span = { + description: spanName, + data: { + someAttribute: 'someValue', + }, + span_id: '123', + trace_id: 'abc', + start_timestamp: 0, + } as SpanJSON; + + _enhanceKitSpan(span); + + expect(span.op).toBe(op); + expect(span.origin).toBe(origin); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe(op); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toBe(origin); + }); + + it("doesn't change spans from other origins", () => { + const span = { + description: 'someOtherSpan', + data: {}, + } as SpanJSON; + + _enhanceKitSpan(span); + + expect(span.op).toBeUndefined(); + expect(span.origin).toBeUndefined(); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBeUndefined(); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toBeUndefined(); + }); + + it("doesn't overwrite the sveltekit.handle.root span", () => { + const rootHandleSpan = { + description: 'sveltekit.handle.root', + op: 'http.server', + origin: 'auto.http.sveltekit', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', + }, + span_id: '123', + trace_id: 'abc', + start_timestamp: 0, + } as SpanJSON; + + _enhanceKitSpan(rootHandleSpan); + + expect(rootHandleSpan.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('http.server'); + expect(rootHandleSpan.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toBe('auto.http.sveltekit'); + expect(rootHandleSpan.op).toBe('http.server'); + expect(rootHandleSpan.origin).toBe('auto.http.sveltekit'); + }); + + it("doesn't enhance unrelated spans", () => { + const span = { + description: 'someOtherSpan', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.pg', + }, + op: 'db', + origin: 'auto.db.pg', + span_id: '123', + trace_id: 'abc', + start_timestamp: 0, + } as SpanJSON; + + _enhanceKitSpan(span); + + expect(span.op).toBe('db'); + expect(span.origin).toBe('auto.db.pg'); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('db'); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toBe('auto.db.pg'); + }); + + it("doesn't overwrite already set ops or origins on sveltekit spans", () => { + // for example, if users manually set this (for whatever reason) + const span = { + description: 'sveltekit.resolve', + origin: 'auto.custom.origin', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'custom.op', + }, + span_id: '123', + trace_id: 'abc', + start_timestamp: 0, + } as SpanJSON; + + _enhanceKitSpan(span); + + expect(span.origin).toBe('auto.custom.origin'); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('custom.op'); + }); + + it('overwrites previously set "manual" origins on sveltekit spans', () => { + // for example, if users manually set this (for whatever reason) + const span = { + description: 'sveltekit.resolve', + origin: 'manual', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'custom.op', + }, + span_id: '123', + trace_id: 'abc', + start_timestamp: 0, + } as SpanJSON; + + _enhanceKitSpan(span); + + expect(span.origin).toBe('auto.http.sveltekit'); + expect(span.data[SEMANTIC_ATTRIBUTE_SENTRY_OP]).toBe('custom.op'); + }); + }); +}); diff --git a/packages/sveltekit/test/server/integrations/http.test.ts b/packages/sveltekit/test/server/integrations/http.test.ts new file mode 100644 index 000000000000..08a6fffcd06f --- /dev/null +++ b/packages/sveltekit/test/server/integrations/http.test.ts @@ -0,0 +1,27 @@ +import * as SentryNode from '@sentry/node'; +import { describe, expect, it, vi } from 'vitest'; +import { httpIntegration as svelteKitHttpIntegration } from '../../../src/server/integrations/http'; + +describe('httpIntegration', () => { + it('calls the original httpIntegration with incoming request span recording disabled', () => { + const sentryNodeHttpIntegration = vi.spyOn(SentryNode, 'httpIntegration'); + svelteKitHttpIntegration({ breadcrumbs: false }); + + expect(sentryNodeHttpIntegration).toHaveBeenCalledTimes(1); + expect(sentryNodeHttpIntegration).toHaveBeenCalledWith({ + breadcrumbs: false, // leaves other options untouched + disableIncomingRequestSpans: true, + }); + }); + + it('allows users to override incoming request span recording', () => { + const sentryNodeHttpIntegration = vi.spyOn(SentryNode, 'httpIntegration'); + svelteKitHttpIntegration({ breadcrumbs: false, disableIncomingRequestSpans: false }); + + expect(sentryNodeHttpIntegration).toHaveBeenCalledTimes(1); + expect(sentryNodeHttpIntegration).toHaveBeenCalledWith({ + breadcrumbs: false, + disableIncomingRequestSpans: false, + }); + }); +}); diff --git a/packages/sveltekit/test/vite/autoInstrument.test.ts b/packages/sveltekit/test/vite/autoInstrument.test.ts index 364680e31bf3..b58f0280cdea 100644 --- a/packages/sveltekit/test/vite/autoInstrument.test.ts +++ b/packages/sveltekit/test/vite/autoInstrument.test.ts @@ -41,7 +41,12 @@ describe('makeAutoInstrumentationPlugin()', () => { }); it('returns the auto instrumentation plugin', async () => { - const plugin = makeAutoInstrumentationPlugin({ debug: true, load: true, serverLoad: true }); + const plugin = makeAutoInstrumentationPlugin({ + debug: true, + load: true, + serverLoad: true, + onlyInstrumentClient: false, + }); expect(plugin.name).toEqual('sentry-auto-instrumentation'); expect(plugin.enforce).toEqual('pre'); expect(plugin.load).toEqual(expect.any(Function)); @@ -58,7 +63,12 @@ describe('makeAutoInstrumentationPlugin()', () => { 'path/to/+layout.mjs', ])('transform %s files', (path: string) => { it('wraps universal load if `load` option is `true`', async () => { - const plugin = makeAutoInstrumentationPlugin({ debug: false, load: true, serverLoad: true }); + const plugin = makeAutoInstrumentationPlugin({ + debug: false, + load: true, + serverLoad: true, + onlyInstrumentClient: false, + }); // @ts-expect-error this exists const loadResult = await plugin.load(path); expect(loadResult).toEqual( @@ -74,6 +84,7 @@ describe('makeAutoInstrumentationPlugin()', () => { debug: false, load: false, serverLoad: false, + onlyInstrumentClient: false, }); // @ts-expect-error this exists const loadResult = await plugin.load(path); @@ -92,7 +103,12 @@ describe('makeAutoInstrumentationPlugin()', () => { 'path/to/+layout.server.mjs', ])('transform %s files', (path: string) => { it('wraps universal load if `load` option is `true`', async () => { - const plugin = makeAutoInstrumentationPlugin({ debug: false, load: false, serverLoad: true }); + const plugin = makeAutoInstrumentationPlugin({ + debug: false, + load: false, + serverLoad: true, + onlyInstrumentClient: false, + }); // @ts-expect-error this exists const loadResult = await plugin.load(path); expect(loadResult).toEqual( @@ -108,12 +124,101 @@ describe('makeAutoInstrumentationPlugin()', () => { debug: false, load: false, serverLoad: false, + onlyInstrumentClient: false, }); // @ts-expect-error this exists const loadResult = await plugin.load(path); expect(loadResult).toEqual(null); }); }); + + describe('when `onlyInstrumentClient` is `true`', () => { + it.each([ + // server-only files + 'path/to/+page.server.ts', + 'path/to/+layout.server.js', + // universal files + 'path/to/+page.mts', + 'path/to/+layout.mjs', + ])("doesn't wrap code in SSR build in %s", async (path: string) => { + const plugin = makeAutoInstrumentationPlugin({ + debug: false, + load: true, + serverLoad: true, + onlyInstrumentClient: true, + }); + + // @ts-expect-error this exists and is callable + plugin.configResolved({ + build: { + ssr: true, + }, + }); + + // @ts-expect-error this exists + const loadResult = await plugin.load(path); + + expect(loadResult).toEqual(null); + }); + + it.each(['path/to/+page.ts', 'path/to/+layout.js'])( + 'wraps client-side code in universal files in %s', + async (path: string) => { + const plugin = makeAutoInstrumentationPlugin({ + debug: false, + load: true, + serverLoad: true, + onlyInstrumentClient: true, + }); + + // @ts-expect-error this exists and is callable + plugin.configResolved({ + build: { + ssr: false, + }, + }); + + // @ts-expect-error this exists and is callable + const loadResult = await plugin.load(path); + + expect(loadResult).toBe( + 'import { wrapLoadWithSentry } from "@sentry/sveltekit";' + + `import * as userModule from "${path}?sentry-auto-wrap";` + + 'export const load = userModule.load ? wrapLoadWithSentry(userModule.load) : undefined;' + + `export * from "${path}?sentry-auto-wrap";`, + ); + }, + ); + + /** + * This is a bit of a constructed case because in a client build, server-only files + * shouldn't even be passed into the load hook. But just to be extra careful, let's + * make sure we don't wrap server-only files in a client build. + */ + it.each(['path/to/+page.server.ts', 'path/to/+layout.server.js'])( + "doesn't wrap client-side code in server-only files in %s", + async (path: string) => { + const plugin = makeAutoInstrumentationPlugin({ + debug: false, + load: true, + serverLoad: true, + onlyInstrumentClient: true, + }); + + // @ts-expect-error this exists and is callable + plugin.configResolved({ + build: { + ssr: false, + }, + }); + + // @ts-expect-error this exists and is callable + const loadResult = await plugin.load(path); + + expect(loadResult).toBe(null); + }, + ); + }); }); describe('canWrapLoad', () => { diff --git a/packages/sveltekit/test/vite/injectGlobalValues.test.ts b/packages/sveltekit/test/vite/injectGlobalValues.test.ts index 3e07bf7e26a7..50f41c84880f 100644 --- a/packages/sveltekit/test/vite/injectGlobalValues.test.ts +++ b/packages/sveltekit/test/vite/injectGlobalValues.test.ts @@ -4,17 +4,18 @@ import { getGlobalValueInjectionCode } from '../../src/vite/injectGlobalValues'; describe('getGlobalValueInjectionCode', () => { it('returns code that injects values into the global object', () => { const injectionCode = getGlobalValueInjectionCode({ - // @ts-expect-error - just want to test this with multiple values - something: 'else', __sentry_sveltekit_output_dir: '.svelte-kit/output', }); - expect(injectionCode).toEqual(`globalThis["something"] = "else"; -globalThis["__sentry_sveltekit_output_dir"] = ".svelte-kit/output"; -`); + + expect(injectionCode).toMatchInlineSnapshot(` + "globalThis["__sentry_sveltekit_output_dir"] = ".svelte-kit/output"; + " + `); // Check that the code above is in fact valid and works as expected // The return value of eval here is the value of the last expression in the code - expect(eval(`${injectionCode}`)).toEqual('.svelte-kit/output'); + eval(injectionCode); + expect(globalThis.__sentry_sveltekit_output_dir).toEqual('.svelte-kit/output'); delete globalThis.__sentry_sveltekit_output_dir; }); diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index d21bcf0d8d04..fa8df96f03a6 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -41,8 +41,8 @@ describe('sentrySvelteKit()', () => { const plugins = await getSentrySvelteKitPlugins(); expect(plugins).toBeInstanceOf(Array); - // 1 auto instrument plugin + 5 source maps plugins - expect(plugins).toHaveLength(9); + // 1 auto instrument plugin + 1 global values injection plugin + 5 source maps plugins + expect(plugins).toHaveLength(10); }); it('returns the custom sentry source maps upload plugin, unmodified sourcemaps plugins and the auto-instrument plugin by default', async () => { @@ -51,6 +51,8 @@ describe('sentrySvelteKit()', () => { expect(pluginNames).toEqual([ // auto instrument plugin: 'sentry-auto-instrumentation', + // global values injection plugin: + 'sentry-sveltekit-global-values-injection-plugin', // default source maps plugins: 'sentry-telemetry-plugin', 'sentry-vite-release-injection-plugin', @@ -68,7 +70,7 @@ describe('sentrySvelteKit()', () => { it("doesn't return the sentry source maps plugins if autoUploadSourcemaps is `false`", async () => { const plugins = await getSentrySvelteKitPlugins({ autoUploadSourceMaps: false }); - expect(plugins).toHaveLength(1); + expect(plugins).toHaveLength(1); // auto instrument }); it("doesn't return the sentry source maps plugins if `NODE_ENV` is development", async () => { @@ -78,7 +80,7 @@ describe('sentrySvelteKit()', () => { const plugins = await getSentrySvelteKitPlugins({ autoUploadSourceMaps: true, autoInstrument: true }); const instrumentPlugin = plugins[0]; - expect(plugins).toHaveLength(1); + expect(plugins).toHaveLength(2); // auto instrument + global values injection expect(instrumentPlugin?.name).toEqual('sentry-auto-instrumentation'); process.env.NODE_ENV = previousEnv; @@ -87,8 +89,8 @@ describe('sentrySvelteKit()', () => { it("doesn't return the auto instrument plugin if autoInstrument is `false`", async () => { const plugins = await getSentrySvelteKitPlugins({ autoInstrument: false }); const pluginNames = plugins.map(plugin => plugin.name); - expect(plugins).toHaveLength(8); - expect(pluginNames).not.toContain('sentry-upload-source-maps'); + expect(plugins).toHaveLength(9); // global values injection + 5 source maps plugins + 3 default plugins + expect(pluginNames).not.toContain('sentry-auto-instrumentation'); }); it('passes user-specified vite plugin options to the custom sentry source maps plugin', async () => { @@ -106,15 +108,18 @@ describe('sentrySvelteKit()', () => { adapter: 'vercel', }); - expect(makePluginSpy).toHaveBeenCalledWith({ - debug: true, - sourcemaps: { - assets: ['foo/*.js'], - ignore: ['bar/*.js'], - filesToDeleteAfterUpload: ['baz/*.js'], + expect(makePluginSpy).toHaveBeenCalledWith( + { + debug: true, + sourcemaps: { + assets: ['foo/*.js'], + ignore: ['bar/*.js'], + filesToDeleteAfterUpload: ['baz/*.js'], + }, + adapter: 'vercel', }, - adapter: 'vercel', - }); + {}, + ); }); it('passes user-specified vite plugin options to the custom sentry source maps plugin', async () => { @@ -152,26 +157,29 @@ describe('sentrySvelteKit()', () => { adapter: 'vercel', }); - expect(makePluginSpy).toHaveBeenCalledWith({ - debug: true, - org: 'other-org', - sourcemaps: { - assets: ['foo/*.js'], - ignore: ['bar/*.js'], - filesToDeleteAfterUpload: ['baz/*.js'], - }, - release: { - inject: false, - name: '3.0.0', - setCommits: { - auto: true, + expect(makePluginSpy).toHaveBeenCalledWith( + { + debug: true, + org: 'other-org', + sourcemaps: { + assets: ['foo/*.js'], + ignore: ['bar/*.js'], + filesToDeleteAfterUpload: ['baz/*.js'], }, + release: { + inject: false, + name: '3.0.0', + setCommits: { + auto: true, + }, + }, + headers: { + 'X-My-Header': 'foo', + }, + adapter: 'vercel', }, - headers: { - 'X-My-Header': 'foo', - }, - adapter: 'vercel', - }); + {}, + ); }); it('passes user-specified options to the auto instrument plugin', async () => { @@ -192,27 +200,36 @@ describe('sentrySvelteKit()', () => { debug: true, load: true, serverLoad: false, + onlyInstrumentClient: false, }); }); }); describe('generateVitePluginOptions', () => { - it('should return null if no relevant options are provided', () => { + it('returns null if no relevant options are provided', () => { const options: SentrySvelteKitPluginOptions = {}; const result = generateVitePluginOptions(options); expect(result).toBeNull(); }); - it('should use default `debug` value if only default options are provided', () => { + it('uses default `debug` value if only default options are provided', () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; // Ensure we're not in development mode + const options: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, autoInstrument: true, debug: false }; const expected: CustomSentryVitePluginOptions = { debug: false, }; const result = generateVitePluginOptions(options); expect(result).toEqual(expected); + + process.env.NODE_ENV = originalEnv; }); - it('should apply user-defined sourceMapsUploadOptions', () => { + it('applies user-defined sourceMapsUploadOptions', () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; // Ensure we're not in development mode + const options: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, sourceMapsUploadOptions: { @@ -234,9 +251,14 @@ describe('generateVitePluginOptions', () => { }; const result = generateVitePluginOptions(options); expect(result).toEqual(expected); + + process.env.NODE_ENV = originalEnv; }); - it('should override options with unstable_sentryVitePluginOptions', () => { + it('overrides options with unstable_sentryVitePluginOptions', () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; // Ensure we're not in development mode + const options: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, sourceMapsUploadOptions: { @@ -264,9 +286,14 @@ describe('generateVitePluginOptions', () => { }; const result = generateVitePluginOptions(options); expect(result).toEqual(expected); + + process.env.NODE_ENV = originalEnv; }); - it('should merge release options correctly', () => { + it('merges release options correctly', () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; // Ensure we're not in development mode + const options: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, sourceMapsUploadOptions: { @@ -293,9 +320,14 @@ describe('generateVitePluginOptions', () => { }; const result = generateVitePluginOptions(options); expect(result).toEqual(expected); + + process.env.NODE_ENV = originalEnv; }); - it('should handle adapter and debug options correctly', () => { + it('handles adapter and debug options correctly', () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; // Ensure we're not in development mode + const options: SentrySvelteKitPluginOptions = { autoUploadSourceMaps: true, adapter: 'vercel', @@ -315,9 +347,14 @@ describe('generateVitePluginOptions', () => { }; const result = generateVitePluginOptions(options); expect(result).toEqual(expected); + + process.env.NODE_ENV = originalEnv; }); - it('should apply bundleSizeOptimizations AND sourceMapsUploadOptions when both are set', () => { + it('applies bundleSizeOptimizations AND sourceMapsUploadOptions when both are set', () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; // Ensure we're not in development mode + const options: SentrySvelteKitPluginOptions = { bundleSizeOptimizations: { excludeTracing: true, @@ -349,5 +386,7 @@ describe('generateVitePluginOptions', () => { }; const result = generateVitePluginOptions(options); expect(result).toEqual(expected); + + process.env.NODE_ENV = originalEnv; }); }); diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts index 97d223dc309b..45d0b74ad6ff 100644 --- a/packages/sveltekit/test/vite/sourceMaps.test.ts +++ b/packages/sveltekit/test/vite/sourceMaps.test.ts @@ -52,12 +52,15 @@ beforeEach(() => { }); async function getSentryViteSubPlugin(name: string): Promise { - const plugins = await makeCustomSentryVitePlugins({ - authToken: 'token', - org: 'org', - project: 'project', - adapter: 'other', - }); + const plugins = await makeCustomSentryVitePlugins( + { + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + }, + { kit: {} }, + ); return plugins.find(plugin => plugin.name === name); } @@ -79,8 +82,8 @@ describe('makeCustomSentryVitePlugins()', () => { expect(plugin?.apply).toEqual('build'); expect(plugin?.enforce).toEqual('post'); - expect(plugin?.resolveId).toBeInstanceOf(Function); - expect(plugin?.transform).toBeInstanceOf(Function); + expect(plugin?.resolveId).toBeUndefined(); + expect(plugin?.transform).toBeUndefined(); expect(plugin?.configResolved).toBeInstanceOf(Function); @@ -178,18 +181,10 @@ describe('makeCustomSentryVitePlugins()', () => { }); }); - describe('Custom debug id source maps plugin plugin', () => { - it('injects the output dir into the server hooks file', async () => { - const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); - // @ts-expect-error this function exists! - const transformOutput = await plugin.transform('foo', '/src/hooks.server.ts'); - const transformedCode = transformOutput.code; - const transformedSourcemap = transformOutput.map; - const expectedTransformedCode = 'foo\n; import "\0sentry-inject-global-values-file";\n'; - expect(transformedCode).toEqual(expectedTransformedCode); - expect(transformedSourcemap).toBeDefined(); - }); + // Note: The global values injection plugin tests are now in a separate test file + // since the plugin was moved to injectGlobalValues.ts + describe('Custom debug id source maps plugin plugin', () => { it('uploads source maps during the SSR build', async () => { const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); // @ts-expect-error this function exists! @@ -423,12 +418,15 @@ describe('deleteFilesAfterUpload', () => { }; }); - const plugins = await makeCustomSentryVitePlugins({ - authToken: 'token', - org: 'org', - project: 'project', - adapter: 'other', - }); + const plugins = await makeCustomSentryVitePlugins( + { + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + }, + { kit: {} }, + ); // @ts-expect-error this function exists! const mergedOptions = sentryVitePlugin.mock.calls[0][0]; @@ -498,15 +496,18 @@ describe('deleteFilesAfterUpload', () => { }; }); - const plugins = await makeCustomSentryVitePlugins({ - authToken: 'token', - org: 'org', - project: 'project', - adapter: 'other', - sourcemaps: { - filesToDeleteAfterUpload, + const plugins = await makeCustomSentryVitePlugins( + { + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + sourcemaps: { + filesToDeleteAfterUpload, + }, }, - }); + { kit: {} }, + ); // @ts-expect-error this function exists! const mergedOptions = sentryVitePlugin.mock.calls[0][0];