diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3670646ec8a7..990361d6ae7c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -475,7 +475,7 @@ jobs: needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 - timeout-minutes: 18 + timeout-minutes: 25 strategy: fail-fast: false matrix: diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 04e7b444a173..3bd402dce4cf 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -53,13 +53,14 @@ jobs: cd packages/e2e-tests yarn test:e2e - name: Create Issue - if: failure() + if: failure() && github.event_name == 'schedule' uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} RUN_LINK: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} with: filename: .github/CANARY_FAILURE_TEMPLATE.md + update_existing: true job_ember_canary_test: name: Ember Canary Tests @@ -92,10 +93,12 @@ jobs: yarn ember try:one ${{ matrix.scenario }} --skip-cleanup=true - name: Create Issue - if: failure() + if: failure() && github.event_name == 'schedule' uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} RUN_LINK: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} with: filename: .github/CANARY_FAILURE_TEMPLATE.md + update_existing: true + title: 'Ember Canary tests failed' diff --git a/CHANGELOG.md b/CHANGELOG.md index c759af697841..090ec65be827 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,69 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.54.0 + +### Important Changes + +- **feat(core): Add default entries to `ignoreTransactions` for Healthchecks #8191** + + All SDKs now filter out health check transactions by default. + These are transactions where the transaction name matches typical API health check calls, such as `/^.*healthy.*$/` or `/^. *heartbeat.*$/`. Take a look at [this list](https://github.com/getsentry/sentry-javascript/blob/8c6ad156829f7c4eec34e4a67e6dd866ba482d5d/packages/core/src/integrations/inboundfilters.ts#L8C2-L16) to learn which regexes we currently use to match transaction names. + We believe that these transactions do not provide value in most cases and we want to save you some of your quota by filtering them out by default. + These filters are implemented as default values for the top level `ignoreTransactions` option. + + You can disable this filtering by manually specifiying the `InboundFilters` integration and setting the `disableTransactionDefaults` option: + ```js + Sentry.init({ + //... + integrations: [new InboundFilters({ disableTransactionDefaults: true })], + }) + ``` + +- **feat(replay): Add `mutationBreadcrumbLimit` and `mutationLimit` to Replay Options (#8228)** + + The previously experimental options `mutationBreadcumbLimit` and `mutationLimit` have been promoted to regular Replay integration options. + + A high number of DOM mutations (in a single event loop) can cause performance regressions in end-users' browsers. + Use `mutationBreadcrumbLimit` to send a breadcrumb along with your recording if the mutation limit was reached. + Use `mutationLimit` to stop recording if the mutation limit was reached. + +- **feat(sveltekit): Add source maps support for Vercel (lambda) (#8256)** + - feat(sveltekit): Auto-detect SvelteKit adapters (#8193) + + The SvelteKit SDK can now be used if you deploy your SvelteKit app to Vercel. + By default, the SDK's Vite plugin will detect the used adapter and adjust the source map uploading config as necessary. + If you want to override the default adapter detection, you can specify the `adapter` option in the `sentrySvelteKit` options: + + ```js + // vite.config.js + export default defineConfig({ + plugins: [ + sentrySvelteKit({ + adapter: 'vercel', + }), + sveltekit(), + ], + }); + ``` + + Currently, the Vite plugin will configure itself correctly for `@sveltejs/adapter-auto`, `@sveltejs/adapter-vercel` and `@sveltejs/adapter-node`. + + **Important:** The SvelteKit SDK is not yet compatible with Vercel's edge runtime. + It will only work for lambda functions. + +### Other Changes + +- feat(replay): Throttle breadcrumbs to max 300/5s (#8086) +- feat(sveltekit): Add option to control handling of unknown server routes (#8201) +- fix(node): Strip query and fragment from request URLs without route parameters (#8213) +- fix(remix): Don't log missing parameters warning on server-side. (#8269) +- fix(remix): Pass `loadContext` through wrapped document request function (#8268) +- fix(replay): Guard against missing key (#8246) +- fix(sveltekit): Avoid capturing redirects and 4xx Http errors in request Handlers (#8215) +- fix(sveltekit): Bump `magicast` to support `satisfied` keyword (#8254) +- fix(wasm): Avoid throwing an error when WASM modules are loaded from blobs (#8263) + ## 7.53.1 - chore(deps): bump socket.io-parser from 4.2.1 to 4.2.3 (#8196) diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js index 5c30f352959c..3aa2548f1522 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js @@ -4,9 +4,7 @@ window.Sentry = Sentry; window.Replay = new Sentry.Replay({ flushMinDelay: 500, flushMaxDelay: 500, - _experiments: { - mutationLimit: 250, - }, + mutationLimit: 250, }); Sentry.init({ diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts index 5d5dbb9d3f93..84f0113263d7 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts @@ -1,10 +1,15 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; +import { + getReplayRecordingContent, + getReplaySnapshot, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../../utils/replayHelpers'; sentryTest( - 'handles large mutations with _experiments.mutationLimit configured', + 'handles large mutations by stopping replay when `mutationLimit` configured', async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => { if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) { sentryTest.skip(); @@ -34,36 +39,29 @@ sentryTest( await forceFlushReplay(); const res1 = await reqPromise1; - const reqPromise2 = waitForReplayRequest(page); + // replay should be stopped due to mutation limit + let replay = await getReplaySnapshot(page); + expect(replay.session).toBe(undefined); + expect(replay._isEnabled).toBe(false); void page.click('#button-modify'); await forceFlushReplay(); - const res2 = await reqPromise2; - const reqPromise3 = waitForReplayRequest(page); - - void page.click('#button-remove'); + await page.click('#button-remove'); await forceFlushReplay(); - const res3 = await reqPromise3; const replayData0 = getReplayRecordingContent(res0); - const replayData1 = getReplayRecordingContent(res1); - const replayData2 = getReplayRecordingContent(res2); - const replayData3 = getReplayRecordingContent(res3); - expect(replayData0.fullSnapshots.length).toBe(1); expect(replayData0.incrementalSnapshots.length).toBe(0); - // This includes both a full snapshot as well as some incremental snapshots - expect(replayData1.fullSnapshots.length).toBe(1); + // Breadcrumbs (click and mutation); + const replayData1 = getReplayRecordingContent(res1); + expect(replayData1.fullSnapshots.length).toBe(0); expect(replayData1.incrementalSnapshots.length).toBeGreaterThan(0); + expect(replayData1.breadcrumbs.map(({ category }) => category).sort()).toEqual(['replay.mutations', 'ui.click']); - // This does not trigger mutations, for whatever reason - so no full snapshot either! - expect(replayData2.fullSnapshots.length).toBe(0); - expect(replayData2.incrementalSnapshots.length).toBeGreaterThan(0); - - // This includes both a full snapshot as well as some incremental snapshots - expect(replayData3.fullSnapshots.length).toBe(1); - expect(replayData3.incrementalSnapshots.length).toBeGreaterThan(0); + replay = await getReplaySnapshot(page); + expect(replay.session).toBe(undefined); + expect(replay._isEnabled).toBe(false); }, ); diff --git a/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts b/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts index f1765b2a3c22..bd303c9e68c3 100644 --- a/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts +++ b/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts @@ -14,8 +14,9 @@ import { // Session should expire after 2s - keep in sync with init.js const SESSION_TIMEOUT = 2000; -sentryTest('handles an expired session', async ({ getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { +sentryTest('handles an expired session', async ({ browserName, getLocalTestPath, page }) => { + // This test seems to only be flakey on firefox + if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) { sentryTest.skip(); } diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts index 2ed458213955..d7b5800f9eea 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -63,8 +63,9 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(2000); }); -sentryTest('immediate mutation does not trigger slow click', async ({ getLocalTestUrl, page }) => { - if (shouldSkipReplayTest()) { +sentryTest('immediate mutation does not trigger slow click', async ({ browserName, getLocalTestUrl, page }) => { + // This test seems to only be flakey on firefox + if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) { sentryTest.skip(); } diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js new file mode 100644 index 000000000000..11207b23752d --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js @@ -0,0 +1,17 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 5000, + flushMaxDelay: 5000, + useCompression: false, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 0, + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + integrations: [window.Replay], +}); diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js new file mode 100644 index 000000000000..30ba4b350700 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js @@ -0,0 +1,8 @@ +const COUNT = 400; + +document.querySelector('[data-console]').addEventListener('click', () => { + // Call console.log() many times + for (let i = 0; i < COUNT; i++) { + console.log(`testing ${i}`); + } +}); diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html new file mode 100644 index 000000000000..07807a41f8c8 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts new file mode 100644 index 000000000000..17f4210624a0 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; + +const THROTTLE_LIMIT = 300; + +sentryTest( + 'throttles breadcrumbs when many `console.log` are made at the same time', + async ({ getLocalTestUrl, page, forceFlushReplay, browserName }) => { + if (shouldSkipReplayTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + await page.click('[data-console]'); + await forceFlushReplay(); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + // 1 click breadcrumb + 1 throttled breadcrumb is why console logs are less + // than throttle limit + expect(breadcrumbs.length).toBe(THROTTLE_LIMIT); + expect(breadcrumbs.filter(breadcrumb => breadcrumb.category === 'replay.throttled').length).toBe(1); + }, +); diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index 415dcd667414..ec332edea74d 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -34,6 +34,25 @@ export type IncrementalRecordingSnapshot = eventWithTime & { export type RecordingSnapshot = FullRecordingSnapshot | IncrementalRecordingSnapshot; +/** Returns the replay event from the given request, or undefined if this is not a replay request. */ +export function getReplayEventFromRequest(req: Request): ReplayEvent | undefined { + const postData = req.postData(); + if (!postData) { + return undefined; + } + + try { + const event = envelopeRequestParser(req); + + if (!isReplayEvent(event)) { + return undefined; + } + + return event; + } catch { + return undefined; + } +} /** * Waits for a replay request to be sent by the page and returns it. * @@ -58,18 +77,13 @@ export function waitForReplayRequest( res => { const req = res.request(); - const postData = req.postData(); - if (!postData) { + const event = getReplayEventFromRequest(req); + + if (!event) { return false; } try { - const event = envelopeRequestParser(req); - - if (!isReplayEvent(event)) { - return false; - } - if (callback) { return callback(event, res); } diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 998c7ab8203d..3b6b2e8933de 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -5,6 +5,16 @@ import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/u // this is the result of a script being pulled in from an external domain and CORS. const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/]; +const DEFAULT_IGNORE_TRANSACTIONS = [ + /^.*healthcheck.*$/, + /^.*healthy.*$/, + /^.*live.*$/, + /^.*ready.*$/, + /^.*heartbeat.*$/, + /^.*\/health$/, + /^.*\/healthz$/, +]; + /** Options for the InboundFilters integration */ export interface InboundFiltersOptions { allowUrls: Array; @@ -12,6 +22,8 @@ export interface InboundFiltersOptions { ignoreErrors: Array; ignoreTransactions: Array; ignoreInternal: boolean; + disableErrorDefaults: boolean; + disableTransactionDefaults: boolean; } /** Inbound filters configurable by the user */ @@ -62,9 +74,13 @@ export function _mergeOptions( ignoreErrors: [ ...(internalOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), - ...DEFAULT_IGNORE_ERRORS, + ...(internalOptions.disableErrorDefaults ? [] : DEFAULT_IGNORE_ERRORS), + ], + ignoreTransactions: [ + ...(internalOptions.ignoreTransactions || []), + ...(clientOptions.ignoreTransactions || []), + ...(internalOptions.disableTransactionDefaults ? [] : DEFAULT_IGNORE_TRANSACTIONS), ], - ignoreTransactions: [...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || [])], ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, }; } diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index a528d765bfeb..88ab23f65909 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -208,6 +208,21 @@ const TRANSACTION_EVENT_3: Event = { type: 'transaction', }; +const TRANSACTION_EVENT_HEALTH: Event = { + transaction: 'GET /health', + type: 'transaction', +}; + +const TRANSACTION_EVENT_HEALTH_2: Event = { + transaction: 'GET /healthy', + type: 'transaction', +}; + +const TRANSACTION_EVENT_HEALTH_3: Event = { + transaction: 'GET /live', + type: 'transaction', +}; + describe('InboundFilters', () => { describe('_isSentryError', () => { it('should work as expected', () => { @@ -372,7 +387,28 @@ describe('InboundFilters', () => { it('uses default filters', () => { const eventProcessor = createInboundFiltersEventProcessor(); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); + }); + + it('disable default error filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableErrorDefaults: true }); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(SCRIPT_ERROR_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); + }); + + it('disable default transaction filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableTransactionDefaults: true }); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(TRANSACTION_EVENT_HEALTH); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(TRANSACTION_EVENT_HEALTH_2); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(TRANSACTION_EVENT_HEALTH_3); }); }); diff --git a/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.server.ts b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.server.ts new file mode 100644 index 000000000000..b07376ba97c9 --- /dev/null +++ b/packages/e2e-tests/test-applications/sveltekit/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/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte new file mode 100644 index 000000000000..fde274c60705 --- /dev/null +++ b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte @@ -0,0 +1,6 @@ +

Check Build

+ +

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

diff --git a/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.ts b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.ts new file mode 100644 index 000000000000..049acdc1fafa --- /dev/null +++ b/packages/e2e-tests/test-applications/sveltekit/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/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts b/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts index 028c8b2d7998..dc0f76d67644 100644 --- a/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts @@ -4,7 +4,7 @@ import { Transaction } from '@sentry/types'; test('should report a `pageload` transaction', async ({ page }) => { const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { - url: '/healthy', + url: '/testy', envelopeType: 'transaction', }); @@ -16,5 +16,5 @@ test('should report a `pageload` transaction', async ({ page }) => { }, }); - expect(await countEnvelopes(page, { url: '/healthy', envelopeType: 'transaction', timeout: 4000 })).toBe(1); + expect(await countEnvelopes(page, { url: '/testy', envelopeType: 'transaction', timeout: 4000 })).toBe(1); }); diff --git a/packages/remix/src/performance/client.tsx b/packages/remix/src/performance/client.tsx index 0f88a67c2592..879c93e51f42 100644 --- a/packages/remix/src/performance/client.tsx +++ b/packages/remix/src/performance/client.tsx @@ -1,7 +1,7 @@ -import type { ErrorBoundaryProps} from '@sentry/react'; +import type { ErrorBoundaryProps } from '@sentry/react'; import { WINDOW, withErrorBoundary } from '@sentry/react'; import type { Transaction, TransactionContext } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { isNodeEnv, logger } from '@sentry/utils'; import * as React from 'react'; const DEFAULT_TAGS = { @@ -101,6 +101,7 @@ export function withSentry

, R extends React.FC // Early return when any of the required functions is not available. if (!_useEffect || !_useLocation || !_useMatches || !_customStartTransaction) { __DEBUG_BUILD__ && + !isNodeEnv() && logger.warn('Remix SDK was unable to wrap your root because of one or more missing parameters.'); // @ts-ignore Setting more specific React Component typing for `R` generic above diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 96acde1db2aa..7fbff4bb6bd8 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -113,6 +113,7 @@ function makeWrappedDocumentRequestFunction( responseStatusCode: number, responseHeaders: Headers, context: Record, + loadContext?: Record, ): Promise { let res: Response; @@ -120,7 +121,7 @@ function makeWrappedDocumentRequestFunction( const currentScope = getCurrentHub().getScope(); if (!currentScope) { - return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context); + return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context, loadContext); } try { @@ -133,7 +134,14 @@ function makeWrappedDocumentRequestFunction( }, }); - res = await origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context); + res = await origDocumentRequestFunction.call( + this, + request, + responseStatusCode, + responseHeaders, + context, + loadContext, + ); span?.finish(); } catch (err) { diff --git a/packages/remix/src/utils/types.ts b/packages/remix/src/utils/types.ts index e57b8ad15e34..642f6eef76cb 100644 --- a/packages/remix/src/utils/types.ts +++ b/packages/remix/src/utils/types.ts @@ -127,9 +127,13 @@ export interface ServerBuild { } export interface HandleDocumentRequestFunction { - (request: RemixRequest, responseStatusCode: number, responseHeaders: Headers, context: EntryContext): - | Promise - | Response; + ( + request: RemixRequest, + responseStatusCode: number, + responseHeaders: Headers, + context: EntryContext, + loadContext?: AppLoadContext, + ): Promise | Response; } export interface HandleDataRequestFunction { diff --git a/packages/replay/.eslintrc.js b/packages/replay/.eslintrc.js index da006cf432a2..6db928fcb1b9 100644 --- a/packages/replay/.eslintrc.js +++ b/packages/replay/.eslintrc.js @@ -8,7 +8,9 @@ module.exports = { overrides: [ { files: ['src/**/*.ts'], - rules: {}, + rules: { + '@sentry-internal/sdk/no-unsupported-es6-methods': 'off', + }, }, { files: ['jest.setup.ts', 'jest.config.ts'], diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 26fb5f4a633a..84bd1afd7abc 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -1,4 +1,3 @@ -import { addBreadcrumb } from '@sentry/core'; import type { Event, EventHint } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -50,13 +49,6 @@ export function handleGlobalEventListener( event.tags = { ...event.tags, replayId: replay.getSessionId() }; } - if (__DEBUG_BUILD__ && replay.getOptions()._experiments.traceInternals && isErrorEvent(event)) { - const exc = getEventExceptionValues(event); - addInternalBreadcrumb({ - message: `Tagging event (${event.event_id}) - ${event.message} - ${exc.type}: ${exc.value}`, - }); - } - // In cases where a custom client is used that does not support the new hooks (yet), // we manually call this hook method here if (afterSendHandler) { @@ -67,22 +59,3 @@ export function handleGlobalEventListener( return event; }; } - -function addInternalBreadcrumb(arg: Parameters[0]): void { - const { category, level, message, ...rest } = arg; - - addBreadcrumb({ - category: category || 'console', - level: level || 'debug', - message: `[debug]: ${message}`, - ...rest, - }); -} - -function getEventExceptionValues(event: Event): { type: string; value: string } { - return { - type: 'Unknown', - value: 'n/a', - ...(event.exception && event.exception.values && event.exception.values[0]), - }; -} diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index e50f5e6e3ab5..f7943d34fa4f 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -28,7 +28,7 @@ export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { const { metaKey, shiftKey, ctrlKey, altKey, key, target } = event; // never capture for input fields - if (!target || isInputElement(target as HTMLElement)) { + if (!target || isInputElement(target as HTMLElement) || !key) { return null; } diff --git a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts index cbb998d499d4..947fb12f1ae4 100644 --- a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts +++ b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts @@ -3,7 +3,6 @@ import type { Breadcrumb } from '@sentry/types'; import { normalize } from '@sentry/utils'; import type { ReplayContainer } from '../../types'; -import { addEvent } from '../../util/addEvent'; /** * Add a breadcrumb event to replay. @@ -20,7 +19,7 @@ export function addBreadcrumbEvent(replay: ReplayContainer, breadcrumb: Breadcru } replay.addUpdate(() => { - void addEvent(replay, { + void replay.throttledAddEvent({ type: EventType.Custom, // TODO: We were converting from ms to seconds for breadcrumbs, spans, // but maybe we should just keep them as milliseconds diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 114d015f2702..0a8813c14d38 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -60,6 +60,9 @@ export class Replay implements Integration { maskAllInputs = true, blockAllMedia = true, + mutationBreadcrumbLimit = 750, + mutationLimit = 10_000, + networkDetailAllowUrls = [], networkCaptureBodies = true, networkRequestHeaders = [], @@ -127,6 +130,8 @@ export class Replay implements Integration { blockAllMedia, maskAllInputs, maskAllText, + mutationBreadcrumbLimit, + mutationLimit, networkDetailAllowUrls, networkCaptureBodies, networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders), diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 52c772f57c21..718b658ad82d 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { EventType, record } from '@sentry-internal/rrweb'; import { captureException, getCurrentHub } from '@sentry/core'; -import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types'; +import type { Breadcrumb, ReplayRecordingMode, Transaction } from '@sentry/types'; import { logger } from '@sentry/utils'; import { @@ -24,6 +24,7 @@ import type { EventBuffer, InternalEventContext, PopEventContext, + RecordingEvent, RecordingOptions, ReplayContainer as ReplayContainerInterface, ReplayPluginOptions, @@ -42,6 +43,8 @@ import { getHandleRecordingEmit } from './util/handleRecordingEmit'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { sendReplay } from './util/sendReplay'; +import type { SKIPPED } from './util/throttle'; +import { throttle, THROTTLED } from './util/throttle'; /** * The main replay container class, which holds all the state and methods for recording and sending replays. @@ -65,6 +68,12 @@ export class ReplayContainer implements ReplayContainerInterface { */ public recordingMode: ReplayRecordingMode = 'session'; + /** + * The current or last active transcation. + * This is only available when performance is enabled. + */ + public lastTransaction?: Transaction; + /** * These are here so we can overwrite them in tests etc. * @hidden @@ -75,6 +84,11 @@ export class ReplayContainer implements ReplayContainerInterface { maxSessionLife: MAX_SESSION_LIFE, } as const; + private _throttledAddEvent: ( + event: RecordingEvent, + isCheckout?: boolean, + ) => typeof THROTTLED | typeof SKIPPED | Promise; + /** * Options to pass to `rrweb.record()` */ @@ -136,6 +150,14 @@ export class ReplayContainer implements ReplayContainerInterface { this._debouncedFlush = debounce(() => this._flush(), this._options.flushMinDelay, { maxWait: this._options.flushMaxDelay, }); + + this._throttledAddEvent = throttle( + (event: RecordingEvent, isCheckout?: boolean) => addEvent(this, event, isCheckout), + // Max 300 events... + 300, + // ... per 5s + 5, + ); } /** Get the event context. */ @@ -565,6 +587,52 @@ export class ReplayContainer implements ReplayContainerInterface { this._context.urls.push(url); } + /** + * Add a breadcrumb event, that may be throttled. + * If it was throttled, we add a custom breadcrumb to indicate that. + */ + public throttledAddEvent( + event: RecordingEvent, + isCheckout?: boolean, + ): typeof THROTTLED | typeof SKIPPED | Promise { + const res = this._throttledAddEvent(event, isCheckout); + + // If this is THROTTLED, it means we have throttled the event for the first time + // In this case, we want to add a breadcrumb indicating that something was skipped + if (res === THROTTLED) { + const breadcrumb = createBreadcrumb({ + category: 'replay.throttled', + }); + + this.addUpdate(() => { + void addEvent(this, { + type: EventType.Custom, + timestamp: breadcrumb.timestamp || 0, + data: { + tag: 'breadcrumb', + payload: breadcrumb, + metric: true, + }, + }); + }); + } + + return res; + } + + /** + * This will get the parametrized route name of the current page. + * This is only available if performance is enabled, and if an instrumented router is used. + */ + public getCurrentRoute(): string | undefined { + const lastTransaction = this.lastTransaction || getCurrentHub().getScope().getTransaction(); + if (!lastTransaction || !['route', 'custom'].includes(lastTransaction.metadata.source)) { + return undefined; + } + + return lastTransaction.name; + } + /** * Initialize and start all listeners to varying events (DOM, * Performance Observer, Recording, Sentry SDK, etc) @@ -803,7 +871,7 @@ export class ReplayContainer implements ReplayContainerInterface { */ private _createCustomBreadcrumb(breadcrumb: Breadcrumb): void { this.addUpdate(() => { - void addEvent(this, { + void this.throttledAddEvent({ type: EventType.Custom, timestamp: breadcrumb.timestamp || 0, data: { @@ -1007,8 +1075,8 @@ export class ReplayContainer implements ReplayContainerInterface { private _onMutationHandler = (mutations: unknown[]): boolean => { const count = mutations.length; - const mutationLimit = this._options._experiments.mutationLimit || 0; - const mutationBreadcrumbLimit = this._options._experiments.mutationBreadcrumbLimit || 1000; + const mutationLimit = this._options.mutationLimit; + const mutationBreadcrumbLimit = this._options.mutationBreadcrumbLimit; const overMutationLimit = mutationLimit && count > mutationLimit; // Create a breadcrumb if a lot of mutations happen at the same time @@ -1018,15 +1086,15 @@ export class ReplayContainer implements ReplayContainerInterface { category: 'replay.mutations', data: { count, + limit: overMutationLimit, }, }); this._createCustomBreadcrumb(breadcrumb); } + // Stop replay if over the mutation limit if (overMutationLimit) { - // We want to skip doing an incremental snapshot if there are too many mutations - // Instead, we do a full snapshot - this._triggerFullSnapshot(false); + void this.stop('mutationLimit'); return false; } diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index ebea87d0f363..42758c1b06d9 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -4,10 +4,12 @@ import type { ReplayRecordingData, ReplayRecordingMode, SentryWrappedXMLHttpRequest, + Transaction, XhrBreadcrumbHint, } from '@sentry/types'; import type { eventWithTime, recordOptions } from './types/rrweb'; +import type { SKIPPED, THROTTLED } from './util/throttle'; export type RecordingEvent = eventWithTime; export type RecordingOptions = recordOptions; @@ -271,6 +273,20 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ maskAllText: boolean; + /** + * A high number of DOM mutations (in a single event loop) can cause + * performance regressions in end-users' browsers. This setting will create + * a breadcrumb in the recording when the limit has been reached. + */ + mutationBreadcrumbLimit: number; + + /** + * A high number of DOM mutations (in a single event loop) can cause + * performance regressions in end-users' browsers. This setting will cause + * recording to stop when the limit has been reached. + */ + mutationLimit: number; + /** * Callback before adding a custom recording event * @@ -294,8 +310,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { _experiments: Partial<{ captureExceptions: boolean; traceInternals: boolean; - mutationLimit: number; - mutationBreadcrumbLimit: number; slowClicks: { threshold: number; timeout: number; @@ -522,6 +536,11 @@ export interface ReplayContainer { session: Session | undefined; recordingMode: ReplayRecordingMode; timeouts: Timeouts; + lastTransaction?: Transaction; + throttledAddEvent: ( + event: RecordingEvent, + isCheckout?: boolean, + ) => typeof THROTTLED | typeof SKIPPED | Promise; isEnabled(): boolean; isPaused(): boolean; getContext(): InternalEventContext; @@ -542,6 +561,7 @@ export interface ReplayContainer { getSessionId(): string | undefined; checkAndHandleExpiredSession(): boolean | void; setInitialState(): void; + getCurrentRoute(): string | undefined; } export interface ReplayPerformanceEntry { diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index e5096f397fae..c1adbc72e787 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -40,6 +40,16 @@ export function addGlobalListeners(replay: ReplayContainer): void { dsc.replay_id = replayId; } }); + + client.on('startTransaction', transaction => { + replay.lastTransaction = transaction; + }); + + // We may be missing the initial startTransaction due to timing issues, + // so we capture it on finish again. + client.on('finishTransaction', transaction => { + replay.lastTransaction = transaction; + }); } } diff --git a/packages/replay/src/util/createPerformanceSpans.ts b/packages/replay/src/util/createPerformanceSpans.ts index cd7a20ddf74a..86de4eb9f958 100644 --- a/packages/replay/src/util/createPerformanceSpans.ts +++ b/packages/replay/src/util/createPerformanceSpans.ts @@ -1,17 +1,16 @@ import { EventType } from '@sentry-internal/rrweb'; import type { AddEventResult, AllEntryData, ReplayContainer, ReplayPerformanceEntry } from '../types'; -import { addEvent } from './addEvent'; /** - * Create a "span" for each performance entry. The parent transaction is `this.replayEvent`. + * Create a "span" for each performance entry. */ export function createPerformanceSpans( replay: ReplayContainer, entries: ReplayPerformanceEntry[], ): Promise[] { - return entries.map(({ type, start, end, name, data }) => - addEvent(replay, { + return entries.map(({ type, start, end, name, data }) => { + const response = replay.throttledAddEvent({ type: EventType.Custom, timestamp: start, data: { @@ -24,6 +23,9 @@ export function createPerformanceSpans( data, }, }, - }), - ); + }); + + // If response is a string, it means its either THROTTLED or SKIPPED + return typeof response === 'string' ? Promise.resolve(null) : response; + }); } diff --git a/packages/replay/src/util/throttle.ts b/packages/replay/src/util/throttle.ts new file mode 100644 index 000000000000..66ce6ed0cdfb --- /dev/null +++ b/packages/replay/src/util/throttle.ts @@ -0,0 +1,55 @@ +export const THROTTLED = '__THROTTLED'; +export const SKIPPED = '__SKIPPED'; + +/** + * Create a throttled function off a given function. + * When calling the throttled function, it will call the original function only + * if it hasn't been called more than `maxCount` times in the last `durationSeconds`. + * + * Returns `THROTTLED` if throttled for the first time, after that `SKIPPED`, + * or else the return value of the original function. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function throttle any>( + fn: T, + maxCount: number, + durationSeconds: number, +): (...rest: Parameters) => ReturnType | typeof THROTTLED | typeof SKIPPED { + const counter = new Map(); + + const _cleanup = (now: number): void => { + const threshold = now - durationSeconds; + counter.forEach((_value, key) => { + if (key < threshold) { + counter.delete(key); + } + }); + }; + + const _getTotalCount = (): number => { + return [...counter.values()].reduce((a, b) => a + b, 0); + }; + + let isThrottled = false; + + return (...rest: Parameters): ReturnType | typeof THROTTLED | typeof SKIPPED => { + // Date in second-precision, which we use as basis for the throttling + const now = Math.floor(Date.now() / 1000); + + // First, make sure to delete any old entries + _cleanup(now); + + // If already over limit, do nothing + if (_getTotalCount() >= maxCount) { + const wasThrottled = isThrottled; + isThrottled = true; + return wasThrottled ? SKIPPED : THROTTLED; + } + + isThrottled = false; + const count = counter.get(now) || 0; + counter.set(now, count + 1); + + return fn(...rest); + }; +} diff --git a/packages/replay/test/unit/util/throttle.test.ts b/packages/replay/test/unit/util/throttle.test.ts new file mode 100644 index 000000000000..4f1c0bc9f6a2 --- /dev/null +++ b/packages/replay/test/unit/util/throttle.test.ts @@ -0,0 +1,125 @@ +import { BASE_TIMESTAMP } from '../..'; +import { SKIPPED, throttle, THROTTLED } from '../../../src/util/throttle'; + +jest.useFakeTimers(); + +describe('Unit | util | throttle', () => { + it('executes when not hitting the limit', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + + const callback = jest.fn(); + const fn = throttle(callback, 100, 60); + + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(3); + + jest.advanceTimersByTime(59_000); + + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(5); + + jest.advanceTimersByTime(1_000); + + fn(); + + expect(callback).toHaveBeenCalledTimes(6); + + jest.advanceTimersByTime(1_000); + + fn(); + + expect(callback).toHaveBeenCalledTimes(7); + }); + + it('stops executing when hitting the limit', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + const callback = jest.fn(); + const fn = throttle(callback, 5, 60); + + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(3); + + jest.advanceTimersByTime(59_000); + + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(5); + + jest.advanceTimersByTime(1_000); + + fn(); + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(5); + + // Now, the first three will "expire", so we can add three more + jest.advanceTimersByTime(1_000); + + fn(); + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(8); + }); + + it('has correct return value', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + + const callback = jest.fn(() => 'foo'); + const fn = throttle(callback, 5, 60); + + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe(THROTTLED); + expect(fn()).toBe(SKIPPED); + expect(fn()).toBe(SKIPPED); + + // After 61s, should be reset + jest.advanceTimersByTime(61_000); + + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe(THROTTLED); + expect(fn()).toBe(SKIPPED); + expect(fn()).toBe(SKIPPED); + }); + + it('passes args correctly', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + + const originalFunction = (a: number, b: number) => a + b; + const callback = jest.fn(originalFunction); + const fn = throttle(callback, 5, 60); + + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(THROTTLED); + expect(fn(1, 2)).toBe(SKIPPED); + expect(fn(1, 2)).toBe(SKIPPED); + }); +}); diff --git a/packages/replay/test/utils/setupReplayContainer.ts b/packages/replay/test/utils/setupReplayContainer.ts index 83ced117c464..c7c302cce72b 100644 --- a/packages/replay/test/utils/setupReplayContainer.ts +++ b/packages/replay/test/utils/setupReplayContainer.ts @@ -15,6 +15,8 @@ const DEFAULT_OPTIONS = { networkCaptureBodies: true, networkRequestHeaders: [], networkResponseHeaders: [], + mutationLimit: 1500, + mutationBreadcrumbLimit: 500, _experiments: {}, }; @@ -54,6 +56,8 @@ export const DEFAULT_OPTIONS_EVENT_PAYLOAD = { maskAllText: false, maskAllInputs: false, useCompression: DEFAULT_OPTIONS.useCompression, + mutationLimit: DEFAULT_OPTIONS.mutationLimit, + mutationBreadcrumbLimit: DEFAULT_OPTIONS.mutationBreadcrumbLimit, networkDetailHasUrls: DEFAULT_OPTIONS.networkDetailAllowUrls.length > 0, networkCaptureBodies: DEFAULT_OPTIONS.networkCaptureBodies, networkRequestHeaders: DEFAULT_OPTIONS.networkRequestHeaders.length > 0, diff --git a/packages/sveltekit/.eslintrc.js b/packages/sveltekit/.eslintrc.js index c3ca0faa363b..4a8474637698 100644 --- a/packages/sveltekit/.eslintrc.js +++ b/packages/sveltekit/.eslintrc.js @@ -17,6 +17,12 @@ module.exports = { project: ['tsconfig.test.json'], }, }, + { + files: ['src/vite/**', 'src/server/**'], + rules: { + '@sentry-internal/sdk/no-optional-chaining': 'off', + }, + }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index dd3133ad6b88..f96ee58811a8 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -28,7 +28,7 @@ "@sentry/types": "7.53.1", "@sentry/utils": "7.53.1", "@sentry/vite-plugin": "^0.6.0", - "magicast": "0.2.6", + "magicast": "0.2.8", "sorcery": "0.11.0" }, "devDependencies": { diff --git a/packages/sveltekit/src/common/utils.ts b/packages/sveltekit/src/common/utils.ts index ecf762cee8c4..84b384861dff 100644 --- a/packages/sveltekit/src/common/utils.ts +++ b/packages/sveltekit/src/common/utils.ts @@ -1,4 +1,4 @@ -import type { Redirect } from '@sveltejs/kit'; +import type { HttpError, Redirect } from '@sveltejs/kit'; export type SentryWrappedFlag = { /** @@ -22,3 +22,10 @@ export function isRedirect(error: unknown): error is Redirect { 'status' in error && typeof error.status === 'number' && error.status >= 300 && error.status <= 308; return hasValidLocation && hasValidStatus; } + +/** + * Determines if a thrown "error" is a HttpError + */ +export function isHttpError(err: unknown): err is HttpError { + return typeof err === 'object' && err !== null && 'status' in err && 'body' in err; +} diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 88e0615d13bb..6d41317cc044 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -5,13 +5,40 @@ import { captureException } from '@sentry/node'; import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; +import { isHttpError, isRedirect } from '../common/utils'; import { getTracePropagationData } from './utils'; +export type SentryHandleOptions = { + /** + * Controls whether the SDK should capture errors and traces in requests that don't belong to a + * route defined in your SvelteKit application. + * + * By default, this option is set to `false` to reduce noise (e.g. bots sending random requests to your server). + * + * Set this option to `true` if you want to monitor requests events without a route. This might be useful in certain + * scenarios, for instance if you registered other handlers that handle these requests. + * If you set this option, you might want adjust the the transaction name in the `beforeSendTransaction` + * callback of your server-side `Sentry.init` options. You can also use `beforeSendTransaction` to filter out + * transactions that you still don't want to be sent to Sentry. + * + * @default false + */ + handleUnknownRoutes?: boolean; +}; + function sendErrorToSentry(e: unknown): unknown { // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can // store a seen flag on it. const objectifiedErr = objectify(e); + // similarly to the `load` function, we don't want to capture 4xx errors or redirects + if ( + isRedirect(objectifiedErr) || + (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) + ) { + return objectifiedErr; + } + captureException(objectifiedErr, scope => { scope.addEventProcessor(event => { addExceptionMechanism(event, { @@ -59,33 +86,42 @@ export const transformPageChunk: NonNullable { + // if there is an active transaction, we know that this handle call is nested and hence + // we don't create a new domain for it. If we created one, nested server calls would + // create new transactions instead of adding a child span to the currently active span. + if (getCurrentHub().getScope().getSpan()) { + return instrumentHandle(input, options); + } + return runWithAsyncContext(() => { + return instrumentHandle(input, options); + }); + }; + return sentryRequestHandler; } -const sentryRequestHandler: Handle = input => { - // if there is an active transaction, we know that this handle call is nested and hence - // we don't create a new domain for it. If we created one, nested server calls would - // create new transactions instead of adding a child span to the currently active span. - if (getCurrentHub().getScope().getSpan()) { - return instrumentHandle(input); +function instrumentHandle({ event, resolve }: Parameters[0], options: SentryHandleOptions): ReturnType { + if (!event.route?.id && !options.handleUnknownRoutes) { + return resolve(event); } - return runWithAsyncContext(() => { - return instrumentHandle(input); - }); -}; -function instrumentHandle({ event, resolve }: Parameters[0]): ReturnType { const { traceparentData, dynamicSamplingContext } = getTracePropagationData(event); return trace( { op: 'http.server', - name: `${event.request.method} ${event.route.id}`, + name: `${event.request.method} ${event.route?.id || event.url.pathname}`, status: 'ok', ...traceparentData, metadata: { - source: 'route', + source: event.route?.id ? 'route' : 'url', dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }, diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index ab9f9ebdafb3..c776dc639d3a 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -3,19 +3,15 @@ import { trace } from '@sentry/core'; import { captureException } from '@sentry/node'; import type { TransactionContext } from '@sentry/types'; import { addExceptionMechanism, addNonEnumerableProperty, objectify } from '@sentry/utils'; -import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; +import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { isRedirect } from '../common/utils'; +import { isHttpError, isRedirect } from '../common/utils'; import { getTracePropagationData } from './utils'; type PatchedLoadEvent = LoadEvent & SentryWrappedFlag; type PatchedServerLoadEvent = ServerLoadEvent & SentryWrappedFlag; -function isHttpError(err: unknown): err is HttpError { - return typeof err === 'object' && err !== null && 'status' in err && 'body' in err; -} - function sendErrorToSentry(e: unknown): unknown { // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can // store a seen flag on it. diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index 17fc855ebc16..8ef6acced314 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -1,8 +1,16 @@ import type { DynamicSamplingContext, StackFrame, TraceparentData } from '@sentry/types'; -import { baggageHeaderToDynamicSamplingContext, basename, extractTraceparentData } from '@sentry/utils'; +import { + baggageHeaderToDynamicSamplingContext, + basename, + escapeStringForRegex, + extractTraceparentData, + GLOBAL_OBJ, + join, +} from '@sentry/utils'; import type { RequestEvent } from '@sveltejs/kit'; import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument'; +import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; /** * Takes a request event and extracts traceparent and DSC data @@ -35,7 +43,8 @@ export function rewriteFramesIteratee(frame: StackFrame): StackFrame { if (!frame.filename) { return frame; } - + const globalWithSentryValues: GlobalWithSentryValues = GLOBAL_OBJ; + const svelteKitBuildOutDir = globalWithSentryValues.__sentry_sveltekit_output_dir; const prefix = 'app:///'; // Check if the frame filename begins with `/` or a Windows-style prefix such as `C:\` @@ -48,8 +57,16 @@ export function rewriteFramesIteratee(frame: StackFrame): StackFrame { .replace(/\\/g, '/') // replace all `\\` instances with `/` : frame.filename; - const base = basename(filename); - frame.filename = `${prefix}${base}`; + let strippedFilename; + if (svelteKitBuildOutDir) { + strippedFilename = filename.replace( + new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`), + '', + ); + } else { + strippedFilename = basename(filename); + } + frame.filename = `${prefix}${strippedFilename}`; } delete frame.module; diff --git a/packages/sveltekit/src/vite/detectAdapter.ts b/packages/sveltekit/src/vite/detectAdapter.ts new file mode 100644 index 000000000000..95e26dfc9f6d --- /dev/null +++ b/packages/sveltekit/src/vite/detectAdapter.ts @@ -0,0 +1,62 @@ +import type { Package } from '@sentry/types'; +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Supported @sveltejs/adapters-[adapter] SvelteKit adapters + */ +export type SupportedSvelteKitAdapters = 'node' | 'auto' | 'vercel' | 'other'; + +/** + * Tries to detect the used adapter for SvelteKit by looking at the dependencies. + * returns the name of the adapter or 'other' if no supported adapter was found. + */ +export async function detectAdapter(debug?: boolean): Promise { + const pkgJson = await loadPackageJson(); + + const allDependencies = pkgJson ? { ...pkgJson.dependencies, ...pkgJson.devDependencies } : {}; + + let adapter: SupportedSvelteKitAdapters = 'other'; + if (allDependencies['@sveltejs/adapter-vercel']) { + adapter = 'vercel'; + } else if (allDependencies['@sveltejs/adapter-node']) { + adapter = 'node'; + } else if (allDependencies['@sveltejs/adapter-auto']) { + adapter = 'auto'; + } + + if (debug) { + if (adapter === 'other') { + // eslint-disable-next-line no-console + console.warn( + "[Sentry SvelteKit Plugin] Couldn't detect SvelteKit adapter. Please set the 'adapter' option manually.", + ); + } else { + // eslint-disable-next-line no-console + console.log(`[Sentry SvelteKit Plugin] Detected SvelteKit ${adapter} adapter`); + } + } + + return adapter; +} + +/** + * Imports the pacakge.json file and returns the parsed JSON object. + */ +async function loadPackageJson(): Promise { + const pkgFile = path.join(process.cwd(), 'package.json'); + + try { + if (!fs.existsSync(pkgFile)) { + throw new Error(`File ${pkgFile} doesn't exist}`); + } + + const pkgJsonContent = (await fs.promises.readFile(pkgFile, 'utf-8')).toString(); + + return JSON.parse(pkgJsonContent); + } catch (e) { + // eslint-disable-next-line no-console + console.warn("[Sentry SvelteKit Plugin] Couldn't load package.json:", e); + return undefined; + } +} diff --git a/packages/sveltekit/src/vite/injectGlobalValues.ts b/packages/sveltekit/src/vite/injectGlobalValues.ts new file mode 100644 index 000000000000..d0f6424a338d --- /dev/null +++ b/packages/sveltekit/src/vite/injectGlobalValues.ts @@ -0,0 +1,41 @@ +import type { InternalGlobal } from '@sentry/utils'; + +export type GlobalSentryValues = { + __sentry_sveltekit_output_dir?: string; +}; + +/** + * Extend the `global` type with custom properties that are + * injected by the SvelteKit SDK at build time. + * @see packages/sveltekit/src/vite/sourcemaps.ts + */ +export type GlobalWithSentryValues = InternalGlobal & GlobalSentryValues; + +export const VIRTUAL_GLOBAL_VALUES_FILE = '\0sentry-inject-global-values-file'; + +/** + * @returns code that injects @param globalSentryValues into the global object. + */ +export function getGlobalValueInjectionCode(globalSentryValues: GlobalSentryValues): string { + if (Object.keys(globalSentryValues).length === 0) { + return ''; + } + + const sentryGlobal = '_global'; + + const globalCode = `var ${sentryGlobal} = + typeof window !== 'undefined' ? + window : + typeof globalThis !== 'undefined' ? + globalThis : + typeof global !== 'undefined' ? + global : + typeof self !== 'undefined' ? + self : + {};`; + const injectedValuesCode = Object.entries(globalSentryValues) + .map(([key, value]) => `${sentryGlobal}["${key}"] = ${JSON.stringify(value)};`) + .join('\n'); + + return `${globalCode}\n${injectedValuesCode}\n`; +} diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 1f983d7a9653..60852424ded2 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -3,6 +3,8 @@ import type { Plugin } from 'vite'; import type { AutoInstrumentSelection } from './autoInstrument'; import { makeAutoInstrumentationPlugin } from './autoInstrument'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; +import { detectAdapter } from './detectAdapter'; import { makeCustomSentryVitePlugin } from './sourceMaps'; type SourceMapsUploadOptions = { @@ -39,6 +41,24 @@ export type SentrySvelteKitPluginOptions = { * @default false. */ debug?: boolean; + + /** + * Specify which SvelteKit adapter you're using. + * By default, the SDK will attempt auto-detect the used adapter at build time and apply the + * correct config for source maps upload or auto-instrumentation. + * + * Currently, the SDK supports the following adapters: + * - node (@sveltejs/adapter-node) + * - auto (@sveltejs/adapter-auto) only Vercel + * - vercel (@sveltejs/adapter-auto) only Serverless functions, no edge runtime + * + * Set this option, if the SDK detects the wrong adapter or you want to use an adapter + * that is not in this list. If you specify 'other', you'll most likely need to configure + * source maps upload yourself. + * + * @default {} the SDK attempts to auto-detect the used adapter at build time + */ + adapter?: SupportedSvelteKitAdapters; } & SourceMapsUploadOptions & AutoInstrumentOptions; @@ -59,6 +79,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} const mergedOptions = { ...DEFAULT_PLUGIN_OPTIONS, ...options, + adapter: options.adapter || (await detectAdapter(options.debug || false)), }; const sentryPlugins: Plugin[] = []; @@ -82,6 +103,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} const pluginOptions = { ...mergedOptions.sourceMapsUploadOptions, debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options + adapter: mergedOptions.adapter, }; sentryPlugins.push(await makeCustomSentryVitePlugin(pluginOptions)); } diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index 8c7662a93813..6f2b7086786a 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -11,7 +11,10 @@ import * as sorcery from 'sorcery'; import type { Plugin } from 'vite'; import { WRAPPED_MODULE_SUFFIX } from './autoInstrument'; -import { getAdapterOutputDir, loadSvelteConfig } from './svelteConfig'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; +import type { GlobalSentryValues } from './injectGlobalValues'; +import { getGlobalValueInjectionCode, VIRTUAL_GLOBAL_VALUES_FILE } from './injectGlobalValues'; +import { getAdapterOutputDir, getHooksFileName, loadSvelteConfig } from './svelteConfig'; // sorcery has no types, so these are some basic type definitions: type Chain = { @@ -26,6 +29,10 @@ type SentryVitePluginOptionsOptionalInclude = Omit { +export async function makeCustomSentryVitePlugin(options?: CustomSentryVitePluginOptions): Promise { const svelteConfig = await loadSvelteConfig(); - const outputDir = await getAdapterOutputDir(svelteConfig); + const usedAdapter = options?.adapter || 'other'; + const outputDir = await getAdapterOutputDir(svelteConfig, usedAdapter); const hasSentryProperties = fs.existsSync(path.resolve(process.cwd(), 'sentry.properties')); const defaultPluginOptions: SentryVitePluginOptions = { - include: [ - { paths: [`${outputDir}/client`] }, - { paths: [`${outputDir}/server/chunks`] }, - { paths: [`${outputDir}/server`], ignore: ['chunks/**'] }, - ], + include: [`${outputDir}/client`, `${outputDir}/server`], configFile: hasSentryProperties ? 'sentry.properties' : undefined, release, }; @@ -70,10 +74,16 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio const sentryPlugin: Plugin = sentryVitePlugin(mergedOptions); const { debug } = mergedOptions; - const { buildStart, resolveId, transform, renderChunk } = sentryPlugin; + const { buildStart, renderChunk } = sentryPlugin; let isSSRBuild = true; + const serverHooksFile = getHooksFileName(svelteConfig, 'server'); + + const globalSentryValues: GlobalSentryValues = { + __sentry_sveltekit_output_dir: outputDir, + }; + const customPlugin: Plugin = { name: 'sentry-upload-source-maps', apply: 'build', // only apply this plugin at build time @@ -82,9 +92,7 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio // These hooks are copied from the original Sentry Vite plugin. // They're mostly responsible for options parsing and release injection. buildStart, - resolveId, renderChunk, - transform, // Modify the config to generate source maps config: config => { @@ -99,6 +107,27 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio }; }, + resolveId: (id, _importer, _ref) => { + if (id === VIRTUAL_GLOBAL_VALUES_FILE) { + return { + id: VIRTUAL_GLOBAL_VALUES_FILE, + external: false, + moduleSideEffects: true, + }; + } + // @ts-ignore - this hook exists on the plugin! + return sentryPlugin.resolveId(id, _importer, _ref); + }, + + 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. @@ -109,6 +138,18 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio } }, + transform: async (code, id) => { + let modifiedCode = code; + const isServerHooksFile = new RegExp(`/${escapeStringForRegex(serverHooksFile)}(.(js|ts|mjs|mts))?`).test(id); + + if (isServerHooksFile) { + const globalValuesImport = `; import "${VIRTUAL_GLOBAL_VALUES_FILE}";`; + modifiedCode = `${code}\n${globalValuesImport}\n`; + } + // @ts-ignore - this hook exists on the plugin! + return sentryPlugin.transform(modifiedCode, id); + }, + // 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 702e29cb9c3f..07c701e912f1 100644 --- a/packages/sveltekit/src/vite/svelteConfig.ts +++ b/packages/sveltekit/src/vite/svelteConfig.ts @@ -5,6 +5,8 @@ import * as fs from 'fs'; import * as path from 'path'; import * as url from 'url'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; + /** * Imports the svelte.config.js file and returns the config object. * The sveltekit plugins import the config in the same way. @@ -35,20 +37,38 @@ export async function loadSvelteConfig(): Promise { } } +/** + * Reads a custom hooks directory from the SvelteKit config. In case no custom hooks + * directory is specified, the default directory is returned. + */ +export function getHooksFileName(svelteConfig: Config, hookType: 'client' | 'server'): string { + return svelteConfig.kit?.files?.hooks?.[hookType] || `src/hooks.${hookType}`; +} + /** * Attempts to read a custom output directory that can be specidied in the options * of a SvelteKit adapter. If no custom output directory is specified, the default * directory is returned. - * - * To get the directory, we have to apply a hack and call the adapter's adapt method + */ +export async function getAdapterOutputDir(svelteConfig: Config, adapter: SupportedSvelteKitAdapters): Promise { + if (adapter === 'node') { + return await getNodeAdapterOutputDir(svelteConfig); + } + + // Auto and Vercel adapters simply use config.kit.outDir + // Let's also use this directory for the 'other' case + return path.join(svelteConfig.kit?.outDir || '.svelte-kit', 'output'); +} + +/** + * To get the Node adapter output directory, we have to apply a hack and call the adapter's adapt method * with a custom adapter `Builder` that only calls the `writeClient` method. * This method is the first method that is called with the output directory. * Once we obtained the output directory, we throw an error to exit the adapter. * * see: https://github.com/sveltejs/kit/blob/master/packages/adapter-node/index.js#L17 - * */ -export async function getAdapterOutputDir(svelteConfig: Config): Promise { +async function getNodeAdapterOutputDir(svelteConfig: Config): Promise { // 'build' is the default output dir for the node adapter let outputDir = 'build'; @@ -56,7 +76,7 @@ export async function getAdapterOutputDir(svelteConfig: Config): Promise return outputDir; } - const adapter = svelteConfig.kit.adapter; + const nodeAdapter = svelteConfig.kit.adapter; const adapterBuilder: Builder = { writeClient(dest: string) { @@ -85,7 +105,7 @@ export async function getAdapterOutputDir(svelteConfig: Config): Promise }; try { - await adapter.adapt(adapterBuilder); + await nodeAdapter.adapt(adapterBuilder); } catch (_) { // We expect the adapter to throw in writeClient! } diff --git a/packages/sveltekit/test/common/utils.test.ts b/packages/sveltekit/test/common/utils.test.ts index 51af645cc961..5581fe60c5e4 100644 --- a/packages/sveltekit/test/common/utils.test.ts +++ b/packages/sveltekit/test/common/utils.test.ts @@ -1,4 +1,6 @@ -import { isRedirect } from '../../src/common/utils'; +import { redirect } from '@sveltejs/kit'; + +import { isHttpError, isRedirect } from '../../src/common/utils'; describe('isRedirect', () => { it.each([ @@ -23,3 +25,19 @@ describe('isRedirect', () => { expect(isRedirect(redirectObject)).toBe(false); }); }); + +describe('isHttpError', () => { + it.each([ + { status: 404, body: 'Not found' }, + { status: 500, body: 'Internal server error' }, + ])('returns `true` for valid HttpError objects (%s)', httpErrorObject => { + expect(isHttpError(httpErrorObject)).toBe(true); + }); + + it.each([new Error(), redirect(301, '/users/id'), 'string error', { status: 404 }, { body: 'Not found' }])( + 'returns `false` for other thrown objects (%s)', + httpErrorObject => { + expect(isHttpError(httpErrorObject)).toBe(false); + }, + ); +}); diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index 94d471f20689..ffe1db0e9e75 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -2,6 +2,7 @@ import { addTracingExtensions, Hub, makeMain, Scope } from '@sentry/core'; import { NodeClient } from '@sentry/node'; import type { Transaction } from '@sentry/types'; import type { Handle } from '@sveltejs/kit'; +import { redirect } from '@sveltejs/kit'; import { vi } from 'vitest'; import { sentryHandle, transformPageChunk } from '../../src/server/handle'; @@ -69,7 +70,18 @@ const enum Type { Async = 'async', } -function resolve(type: Type, isError: boolean): Parameters[0]['resolve'] { +function resolve( + type: Type, + isError: boolean, + throwSpecialError?: 'redirect' | 'http', +): Parameters[0]['resolve'] { + if (throwSpecialError === 'redirect') { + throw redirect(302, '/redirect'); + } + if (throwSpecialError === 'http') { + throw { status: 404, body: 'Not found' }; + } + if (type === Type.Sync) { return (..._args: unknown[]) => { if (isError) { @@ -292,6 +304,22 @@ describe('handleSentry', () => { } }); + it("doesn't send redirects in a request handler to Sentry", async () => { + try { + await sentryHandle()({ event: mockEvent(), resolve: resolve(type, false, 'redirect') }); + } catch (e) { + expect(mockCaptureException).toBeCalledTimes(0); + } + }); + + it("doesn't send Http 4xx errors in a request handler to Sentry", async () => { + try { + await sentryHandle()({ event: mockEvent(), resolve: resolve(type, false, 'http') }); + } catch (e) { + expect(mockCaptureException).toBeCalledTimes(0); + } + }); + it('calls `transformPageChunk`', async () => { const mockResolve = vi.fn().mockImplementation(resolve(type, isError)); const event = mockEvent(); @@ -305,6 +333,39 @@ describe('handleSentry', () => { expect(mockResolve).toHaveBeenCalledTimes(1); expect(mockResolve).toHaveBeenCalledWith(event, { transformPageChunk: expect.any(Function) }); }); + + it("doesn't create a transaction if there's no route", async () => { + let ref: any = undefined; + client.on('finishTransaction', (transaction: Transaction) => { + ref = transaction; + }); + + try { + await sentryHandle()({ event: mockEvent({ route: undefined }), resolve: resolve(type, isError) }); + } catch { + // + } + + expect(ref).toBeUndefined(); + }); + + it("Creates a transaction if there's no route but `handleUnknownRequests` is true", async () => { + let ref: any = undefined; + client.on('finishTransaction', (transaction: Transaction) => { + ref = transaction; + }); + + try { + await sentryHandle({ handleUnknownRoutes: true })({ + event: mockEvent({ route: undefined }), + resolve: resolve(type, isError), + }); + } catch { + // + } + + expect(ref).toBeDefined(); + }); }); }); diff --git a/packages/sveltekit/test/server/utils.test.ts b/packages/sveltekit/test/server/utils.test.ts index 179cc6682d85..2fd9b0492013 100644 --- a/packages/sveltekit/test/server/utils.test.ts +++ b/packages/sveltekit/test/server/utils.test.ts @@ -1,6 +1,8 @@ import { RewriteFrames } from '@sentry/integrations'; import type { StackFrame } from '@sentry/types'; +import { basename } from '@sentry/utils'; +import type { GlobalWithSentryValues } from '../../src/server/utils'; import { getTracePropagationData, rewriteFramesIteratee } from '../../src/server/utils'; const MOCK_REQUEST_EVENT: any = { @@ -69,7 +71,7 @@ describe('rewriteFramesIteratee', () => { expect(result).not.toHaveProperty('module'); }); - it('does the same filename modification as the default RewriteFrames iteratee', () => { + it('does the same filename modification as the default RewriteFrames iteratee if no output dir is available', () => { const frame: StackFrame = { filename: '/some/path/to/server/chunks/3-ab34d22f.js', lineno: 1, @@ -94,4 +96,36 @@ describe('rewriteFramesIteratee', () => { expect(result).toStrictEqual(defaultResult); }); + + it.each([ + ['adapter-node', 'build', '/absolute/path/to/build/server/chunks/3-ab34d22f.js', 'app:///chunks/3-ab34d22f.js'], + [ + 'adapter-auto', + '.svelte-kit/output', + '/absolute/path/to/.svelte-kit/output/server/entries/pages/page.ts.js', + 'app:///entries/pages/page.ts.js', + ], + ])( + 'removes the absolut path to the server output dir, if the output dir is available (%s)', + (_, outputDir, frameFilename, modifiedFilename) => { + (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir = outputDir; + + const frame: StackFrame = { + filename: frameFilename, + lineno: 1, + colno: 1, + module: basename(frameFilename), + }; + + const result = rewriteFramesIteratee({ ...frame }); + + expect(result).toStrictEqual({ + filename: modifiedFilename, + lineno: 1, + colno: 1, + }); + + delete (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir; + }, + ); }); diff --git a/packages/sveltekit/test/vite/detectAdapter.test.ts b/packages/sveltekit/test/vite/detectAdapter.test.ts new file mode 100644 index 000000000000..7f4e05a1a44b --- /dev/null +++ b/packages/sveltekit/test/vite/detectAdapter.test.ts @@ -0,0 +1,79 @@ +import { vi } from 'vitest'; + +import { detectAdapter } from '../../src/vite/detectAdapter'; + +let existsFile = true; +const pkgJson = { + dependencies: {}, +}; +describe('detectAdapter', () => { + beforeEach(() => { + existsFile = true; + vi.clearAllMocks(); + pkgJson.dependencies = {}; + }); + + vi.mock('fs', () => { + return { + existsSync: () => existsFile, + promises: { + readFile: () => { + return Promise.resolve(JSON.stringify(pkgJson)); + }, + }, + }; + }); + + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + it.each(['auto', 'vercel', 'node'])( + 'returns the adapter name (adapter %s) and logs it to the console', + async adapter => { + pkgJson.dependencies[`@sveltejs/adapter-${adapter}`] = '1.0.0'; + const detectedAdapter = await detectAdapter(true); + expect(detectedAdapter).toEqual(adapter); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining(`Detected SvelteKit ${adapter} adapter`)); + }, + ); + + it('returns "other" if no supported adapter was found', async () => { + pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('other'); + }); + + it('logs a warning if in debug mode and no supported adapter was found', async () => { + pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; + const detectedAdapter = await detectAdapter(true); + expect(detectedAdapter).toEqual('other'); + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Couldn't detect SvelteKit adapter")); + }); + + it('returns "other" if package.json isnt available and emits a warning log', async () => { + existsFile = false; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('other'); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Couldn't load package.json"), + expect.any(Error), + ); + }); + + it('prefers all other adapters over adapter auto', async () => { + pkgJson.dependencies['@sveltejs/adapter-auto'] = '1.0.0'; + pkgJson.dependencies['@sveltejs/adapter-vercel'] = '1.0.0'; + pkgJson.dependencies['@sveltejs/adapter-node'] = '1.0.0'; + + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('vercel'); + + delete pkgJson.dependencies['@sveltejs/adapter-vercel']; + const detectedAdapter2 = await detectAdapter(); + expect(detectedAdapter2).toEqual('node'); + }); +}); diff --git a/packages/sveltekit/test/vite/injectGlobalValues.test.ts b/packages/sveltekit/test/vite/injectGlobalValues.test.ts new file mode 100644 index 000000000000..0fe6b07dc989 --- /dev/null +++ b/packages/sveltekit/test/vite/injectGlobalValues.test.ts @@ -0,0 +1,34 @@ +import { getGlobalValueInjectionCode } from '../../src/vite/injectGlobalValues'; + +describe('getGlobalValueInjectionCode', () => { + it('returns code that injects values into the global object', () => { + const injectionCode = getGlobalValueInjectionCode({ + // @ts-ignore - just want to test this with multiple values + something: 'else', + __sentry_sveltekit_output_dir: '.svelte-kit/output', + }); + expect(injectionCode).toEqual(`var _global = + typeof window !== 'undefined' ? + window : + typeof globalThis !== 'undefined' ? + globalThis : + typeof global !== 'undefined' ? + global : + typeof self !== 'undefined' ? + self : + {}; +_global["something"] = "else"; +_global["__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'); + + delete globalThis.__sentry_sveltekit_output_dir; + }); + + it('returns empty string if no values are passed', () => { + expect(getGlobalValueInjectionCode({})).toEqual(''); + }); +}); diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 963844bdef70..923005b2e9f9 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -17,7 +17,14 @@ vi.mock('fs', async () => { }; }); -describe('sentryVite()', () => { +vi.spyOn(console, 'log').mockImplementation(() => { + /* noop */ +}); +vi.spyOn(console, 'warn').mockImplementation(() => { + /* noop */ +}); + +describe('sentrySvelteKit()', () => { it('returns an array of Vite plugins', async () => { const plugins = await sentrySvelteKit(); @@ -65,6 +72,7 @@ describe('sentryVite()', () => { ignore: ['bar.js'], }, autoInstrument: false, + adapter: 'vercel', }); const plugin = plugins[0]; @@ -73,6 +81,7 @@ describe('sentryVite()', () => { debug: true, ignore: ['bar.js'], include: ['foo.js'], + adapter: 'vercel', }); }); diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts index 7d412811b92e..9d565aceab58 100644 --- a/packages/sveltekit/test/vite/sourceMaps.test.ts +++ b/packages/sveltekit/test/vite/sourceMaps.test.ts @@ -6,7 +6,7 @@ const mockedSentryVitePlugin = { buildStart: vi.fn(), resolveId: vi.fn(), renderChunk: vi.fn(), - transform: vi.fn(), + transform: vi.fn().mockImplementation((code: string, _id: string) => code), writeBundle: vi.fn(), }; @@ -54,6 +54,15 @@ describe('makeCustomSentryVitePlugin()', () => { }); }); + it('injects the output dir into the server hooks file', async () => { + const plugin = await makeCustomSentryVitePlugin(); + // @ts-ignore this function exists! + const transformedCode = await plugin.transform('foo', '/src/hooks.server.ts'); + const expectedtransformedCode = 'foo\n; import "\0sentry-inject-global-values-file";\n'; + expect(mockedSentryVitePlugin.transform).toHaveBeenCalledWith(expectedtransformedCode, '/src/hooks.server.ts'); + expect(transformedCode).toEqual(expectedtransformedCode); + }); + it('uploads source maps during the SSR build', async () => { const plugin = await makeCustomSentryVitePlugin(); // @ts-ignore this function exists! diff --git a/packages/sveltekit/test/vite/svelteConfig.test.ts b/packages/sveltekit/test/vite/svelteConfig.test.ts index 73f624c8b1be..5f079deb7c1a 100644 --- a/packages/sveltekit/test/vite/svelteConfig.test.ts +++ b/packages/sveltekit/test/vite/svelteConfig.test.ts @@ -1,6 +1,7 @@ import { vi } from 'vitest'; -import { getAdapterOutputDir, loadSvelteConfig } from '../../src/vite/svelteConfig'; +import type { SupportedSvelteKitAdapters } from '../../src/vite/detectAdapter'; +import { getAdapterOutputDir, getHooksFileName, loadSvelteConfig } from '../../src/vite/svelteConfig'; let existsFile; @@ -62,8 +63,33 @@ describe('getAdapterOutputDir', () => { }, }; - it('returns the output directory of the adapter', async () => { - const outputDir = await getAdapterOutputDir({ kit: { adapter: mockedAdapter } }); + it('returns the output directory of the Node adapter', async () => { + const outputDir = await getAdapterOutputDir({ kit: { adapter: mockedAdapter } }, 'node'); expect(outputDir).toEqual('customBuildDir'); }); + + it.each(['vercel', 'auto', 'other'] as SupportedSvelteKitAdapters[])( + 'returns the config.kit.outdir directory for adapter-%s', + async adapter => { + const outputDir = await getAdapterOutputDir({ kit: { outDir: 'customOutDir' } }, adapter); + expect(outputDir).toEqual('customOutDir/output'); + }, + ); + + it('falls back to the default out dir for all other adapters if outdir is not specified in the config', async () => { + const outputDir = await getAdapterOutputDir({ kit: {} }, 'vercel'); + expect(outputDir).toEqual('.svelte-kit/output'); + }); +}); + +describe('getHooksFileName', () => { + it('returns the default hooks file name if no custom hooks file is specified', () => { + const hooksFileName = getHooksFileName({}, 'server'); + expect(hooksFileName).toEqual('src/hooks.server'); + }); + + it('returns the custom hooks file name if specified in the config', () => { + const hooksFileName = getHooksFileName({ kit: { files: { hooks: { server: 'serverhooks' } } } }, 'server'); + expect(hooksFileName).toEqual('serverhooks'); + }); }); diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts index 8b3bcb52fcf4..165494563eb7 100644 --- a/packages/tracing-internal/src/node/integrations/express.ts +++ b/packages/tracing-internal/src/node/integrations/express.ts @@ -1,6 +1,12 @@ /* eslint-disable max-lines */ import type { Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/types'; -import { extractPathForTransaction, getNumberOfUrlSegments, isRegExp, logger } from '@sentry/utils'; +import { + extractPathForTransaction, + getNumberOfUrlSegments, + isRegExp, + logger, + stripUrlQueryAndFragment, +} from '@sentry/utils'; import { shouldDisableAutoInstrumentation } from './utils/node-utils'; @@ -335,7 +341,7 @@ function instrumentRouter(appOrRouter: ExpressRouter): void { if (urlLength === routeLength) { if (!req._hasParameters) { if (req._reconstructedRoute !== req.originalUrl) { - req._reconstructedRoute = req.originalUrl; + req._reconstructedRoute = req.originalUrl ? stripUrlQueryAndFragment(req.originalUrl) : req.originalUrl; } } diff --git a/packages/types/src/package.ts b/packages/types/src/package.ts index c44d66e62950..15a61f3668d3 100644 --- a/packages/types/src/package.ts +++ b/packages/types/src/package.ts @@ -2,4 +2,6 @@ export interface Package { name: string; version: string; + dependencies?: Record; + devDependencies?: Record; } diff --git a/packages/wasm/src/registry.ts b/packages/wasm/src/registry.ts index 765b47026f00..2005c840b630 100644 --- a/packages/wasm/src/registry.ts +++ b/packages/wasm/src/registry.ts @@ -45,11 +45,22 @@ export function registerModule(module: WebAssembly.Module, url: string): void { if (oldIdx >= 0) { IMAGES.splice(oldIdx, 1); } + + let debugFileUrl = null; + if (debugFile) { + try { + debugFileUrl = new URL(debugFile, url).href; + } catch { + // debugFile could be a blob URL which causes the URL constructor to throw + // for now we just ignore this case + } + } + IMAGES.push({ type: 'wasm', code_id: buildId, code_file: url, - debug_file: debugFile ? new URL(debugFile, url).href : null, + debug_file: debugFileUrl, debug_id: `${buildId.padEnd(32, '0').slice(0, 32)}0`, }); } diff --git a/yarn.lock b/yarn.lock index 4c8836ca0838..d118d336daf6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1032,10 +1032,10 @@ resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.20.15.tgz#eec9f36d8eaf0948bb88c87a46784b5ee9fd0c89" integrity sha512-DI4a1oZuf8wC+oAJA9RW6ga3Zbe8RZFt7kD9i4qAspz3I/yHet1VvC3DiSy/fsUvv5pvJuNPh0LPOdCcqinDPg== -"@babel/parser@^7.21.8": - version "7.21.8" - resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.8.tgz#642af7d0333eab9c0ad70b14ac5e76dbde7bfdf8" - integrity sha512-6zavDGdzG3gUqAdWvlLFfk+36RilI+Pwyuuh7HItyeScCWP3k6i8vKclAQ0bM/0y/Kz/xiwvxhMv9MgTJP5gmA== +"@babel/parser@^7.21.9": + version "7.22.4" + resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.22.4.tgz#a770e98fd785c231af9d93f6459d36770993fb32" + integrity sha512-VLLsx06XkEYqBtE5YGPwfSGwfrjnyPP5oiGty3S8pQLFDFLaS8VwWSIxkTXpcvr5zeYLE6+MBNl2npl/YnfofA== "@babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression@^7.18.6": version "7.18.6" @@ -6584,10 +6584,10 @@ ast-types@0.14.2: dependencies: tslib "^2.0.1" -ast-types@0.15.2: - version "0.15.2" - resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.15.2.tgz#39ae4809393c4b16df751ee563411423e85fb49d" - integrity sha512-c27loCv9QkZinsa5ProX751khO9DJl/AcB5c2KNtA6NRvHKS0PgLfcftz72KVq504vB0Gku5s2kUZzDBvQWvHg== +ast-types@^0.16.1: + version "0.16.1" + resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.16.1.tgz#7a9da1617c9081bc121faafe91711b4c8bb81da2" + integrity sha512-6t10qk83GOG8p0vKmaCr8eiilZwO171AvbROMtvvNiwrTly62t+7XkA8RdIIVbpMhCASAsxgAzdRSwh6nw/5Dg== dependencies: tslib "^2.0.1" @@ -17998,14 +17998,14 @@ magic-string@^0.30.0: dependencies: "@jridgewell/sourcemap-codec" "^1.4.13" -magicast@0.2.6: - version "0.2.6" - resolved "https://registry.yarnpkg.com/magicast/-/magicast-0.2.6.tgz#08c9f1900177ca1896e9c07981912171d4ed8ec1" - integrity sha512-6bX0nVjGrA41o+qHSv9Duiv3VuF7jUyjT7dIb3E61YW/5mucvCBMgyZssUznRc+xlUMPYyXZZluZjE1k5z+2yQ== +magicast@0.2.8: + version "0.2.8" + resolved "https://registry.yarnpkg.com/magicast/-/magicast-0.2.8.tgz#02b298c65fbc5b7d1fce52ef779c59caf68cc9cf" + integrity sha512-zEnqeb3E6TfMKYXGyHv3utbuHNixr04o3/gVGviSzVQkbFiU46VZUd+Ea/1npKfvEsEWxBYuIksKzoztTDPg0A== dependencies: - "@babel/parser" "^7.21.8" + "@babel/parser" "^7.21.9" "@babel/types" "^7.21.5" - recast "^0.22.0" + recast "^0.23.2" make-dir@3.1.0, make-dir@^3.0.0, make-dir@^3.0.2, make-dir@^3.1.0, make-dir@~3.1.0: version "3.1.0" @@ -22937,13 +22937,13 @@ recast@^0.20.5: source-map "~0.6.1" tslib "^2.0.1" -recast@^0.22.0: - version "0.22.0" - resolved "https://registry.yarnpkg.com/recast/-/recast-0.22.0.tgz#1dd3bf1b86e5eb810b044221a1a734234ed3e9c0" - integrity sha512-5AAx+mujtXijsEavc5lWXBPQqrM4+Dl5qNH96N2aNeuJFUzpiiToKPsxQD/zAIJHspz7zz0maX0PCtCTFVlixQ== +recast@^0.23.2: + version "0.23.2" + resolved "https://registry.yarnpkg.com/recast/-/recast-0.23.2.tgz#d3dda3e8f0a3366860d7508c00e34a338ac52b41" + integrity sha512-Qv6cPfVZyMOtPszK6PgW70pUgm7gPlFitAPf0Q69rlOA0zLw2XdDcNmPbVGYicFGT9O8I7TZ/0ryJD+6COvIPw== dependencies: assert "^2.0.0" - ast-types "0.15.2" + ast-types "^0.16.1" esprima "~4.0.0" source-map "~0.6.1" tslib "^2.0.1"