diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cac9a0da588a..3670646ec8a7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -253,43 +253,6 @@ jobs: # `job_build` can't see `job_install_deps` and what it returned) dependency_cache_key: ${{ needs.job_install_deps.outputs.dependency_cache_key }} - job_pack_aws_lambda_layer: - name: Pack and Upload AWS Lambda Layer - needs: [job_get_metadata, job_build] - # only upload the zipped layer file if we're about to release - if: startsWith(github.ref, 'refs/heads/release/') - runs-on: ubuntu-20.04 - steps: - - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) - uses: actions/checkout@v3 - with: - ref: ${{ env.HEAD_COMMIT }} - - name: Set up Node - uses: actions/setup-node@v3 - with: - node-version-file: 'package.json' - - name: Restore caches - uses: ./.github/actions/restore-cache - env: - DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }} - - - name: Get SDK version - # `jq` reads JSON files, and `tee` pipes its input to the given location and to stdout. (Adding `-a` is the - # equivalent of using >> rather than >.) - run: | - export SDK_VERSION=$(cat packages/core/package.json | jq --raw-output '.version') - echo "SDK_VERSION=$SDK_VERSION" | tee -a $GITHUB_ENV - - name: Move dist-serverless to root directory (requirement for zipping action) - run: | - mv ./packages/serverless/build/aws/dist-serverless . - - name: Create and upload final zip file - uses: getsentry/action-build-aws-lambda-extension@v1 - with: - artifact_name: ${{ env.HEAD_COMMIT }} - zip_file_name: sentry-node-serverless-${{ env.SDK_VERSION }}.zip - build_cache_paths: ${{ env.CACHED_BUILD_PATHS }} - build_cache_key: ${{ env.BUILD_CACHE_KEY }} - job_size_check: name: Size Check needs: [job_get_metadata, job_build] @@ -399,6 +362,7 @@ jobs: ${{ github.workspace }}/packages/integrations/build/bundles/** ${{ github.workspace }}/packages/replay/build/bundles/** ${{ github.workspace }}/packages/**/*.tgz + ${{ github.workspace }}/packages/serverless/build/aws/dist-serverless/*.zip job_browser_unit_tests: name: Browser Unit Tests diff --git a/CHANGELOG.md b/CHANGELOG.md index db0fc8c538b4..5a373bbae480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,27 @@ ## Unreleased + - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.53.0 + +- feat(replay): Add `beforeAddRecordingEvent` Replay option (#8124) +- feat(replay): Do not capture replays < 5 seconds (#7949) +- fix(nextjs): Guard for non-absolute paths when injecting sentry config (#8151) +- fix(nextjs): Import path issue on Windows (#8142) +- fix(nextjs): Make `withSentryConfig` isomorphic (#8166) +- fix(node): Add debug logging for node checkin (#8131) +- fix(node): Add LRU map for tracePropagationTargets calculation (#8130) +- fix(node): Remove new URL usage in Undici integration (#8147) +- fix(replay): Show the correct Replay config option name `maskFn` +- fix(sveltekit): Avoid double-wrapping load functions (#8094) +- fix(tracing): Change where content-length gets added (#8139) +- fix(tracing): Use integer for content length (#8152) +- fix(utils): Fail silently if the provided Dsn is invalid (#8121) +- ref(node): Cache undici trace propagation decisions (#8136) +- ref(serverless): Remove relay extension from AWS Layer (#8080) + ## 7.52.1 - feat(replay): Capture slow clicks (experimental) (#8052) diff --git a/packages/core/src/api.ts b/packages/core/src/api.ts index 7191871d9ec4..d6c04ea7c350 100644 --- a/packages/core/src/api.ts +++ b/packages/core/src/api.ts @@ -58,6 +58,10 @@ export function getReportDialogEndpoint( }, ): string { const dsn = makeDsn(dsnLike); + if (!dsn) { + return ''; + } + const endpoint = `${getBaseApiEndpoint(dsn)}embed/error-page/`; let encodedOptions = `dsn=${dsnToString(dsn)}`; diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 80be9dc037bd..9a1312a131ac 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -112,16 +112,20 @@ export abstract class BaseClient implements Client { */ protected constructor(options: O) { this._options = options; + if (options.dsn) { this._dsn = makeDsn(options.dsn); + } else { + __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.'); + } + + if (this._dsn) { const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options); this._transport = options.transport({ recordDroppedEvent: this.recordDroppedEvent.bind(this), ...options.transportOptions, url, }); - } else { - __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.'); } } diff --git a/packages/core/src/transports/multiplexed.ts b/packages/core/src/transports/multiplexed.ts index 9f47b5b76542..c7045f4b2686 100644 --- a/packages/core/src/transports/multiplexed.ts +++ b/packages/core/src/transports/multiplexed.ts @@ -51,9 +51,13 @@ export function makeMultiplexedTransport( const fallbackTransport = createTransport(options); const otherTransports: Record = {}; - function getTransport(dsn: string): Transport { + function getTransport(dsn: string): Transport | undefined { if (!otherTransports[dsn]) { - const url = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(dsn)); + const validatedDsn = dsnFromString(dsn); + if (!validatedDsn) { + return undefined; + } + const url = getEnvelopeEndpointWithUrlEncodedAuth(validatedDsn); otherTransports[dsn] = createTransport({ ...options, url }); } @@ -66,7 +70,9 @@ export function makeMultiplexedTransport( return eventFromEnvelope(envelope, eventTypes); } - const transports = matcher({ envelope, getEvent }).map(dsn => getTransport(dsn)); + const transports = matcher({ envelope, getEvent }) + .map(dsn => getTransport(dsn)) + .filter((t): t is Transport => !!t); // If we have no transports to send to, use the fallback transport if (transports.length === 0) { diff --git a/packages/core/test/lib/api.test.ts b/packages/core/test/lib/api.test.ts index 141301878a5d..ecb92d22a064 100644 --- a/packages/core/test/lib/api.test.ts +++ b/packages/core/test/lib/api.test.ts @@ -9,7 +9,7 @@ const dsnPublic = 'https://abc@sentry.io:1234/subpath/123'; const tunnel = 'https://hello.com/world'; const _metadata = { sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } } as ClientOptions['_metadata']; -const dsnPublicComponents = makeDsn(dsnPublic); +const dsnPublicComponents = makeDsn(dsnPublic)!; describe('API', () => { describe('getEnvelopeEndpointWithUrlEncodedAuth', () => { diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 9705f7a6622f..aca8784ad511 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -70,19 +70,19 @@ describe('BaseClient', () => { }); test('allows missing Dsn', () => { - expect.assertions(1); - const options = getDefaultTestClientOptions(); const client = new TestClient(options); expect(client.getDsn()).toBeUndefined(); + expect(client.getTransport()).toBeUndefined(); }); - test('throws with invalid Dsn', () => { - expect.assertions(1); - + test('handles being passed an invalid Dsn', () => { const options = getDefaultTestClientOptions({ dsn: 'abc' }); - expect(() => new TestClient(options)).toThrow(SentryError); + const client = new TestClient(options); + + expect(client.getDsn()).toBeUndefined(); + expect(client.getTransport()).toBeUndefined(); }); }); diff --git a/packages/core/test/lib/transports/multiplexed.test.ts b/packages/core/test/lib/transports/multiplexed.test.ts index f4f0144e045e..2d2dcb5ce46d 100644 --- a/packages/core/test/lib/transports/multiplexed.test.ts +++ b/packages/core/test/lib/transports/multiplexed.test.ts @@ -12,10 +12,10 @@ import { TextEncoder } from 'util'; import { createTransport, getEnvelopeEndpointWithUrlEncodedAuth, makeMultiplexedTransport } from '../../../src'; const DSN1 = 'https://1234@5678.ingest.sentry.io/4321'; -const DSN1_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN1)); +const DSN1_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN1)!); const DSN2 = 'https://5678@1234.ingest.sentry.io/8765'; -const DSN2_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN2)); +const DSN2_URL = getEnvelopeEndpointWithUrlEncodedAuth(dsnFromString(DSN2)!); const ERROR_EVENT = { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ @@ -83,6 +83,20 @@ describe('makeMultiplexedTransport', () => { await transport.send(ERROR_ENVELOPE); }); + it('Falls back to options DSN when a matched DSN is invalid', async () => { + expect.assertions(1); + + const makeTransport = makeMultiplexedTransport( + createTestTransport(url => { + expect(url).toBe(DSN1_URL); + }), + () => ['invalidDsn'], + ); + + const transport = makeTransport({ url: DSN1_URL, ...transportOptions }); + await transport.send(ERROR_ENVELOPE); + }); + it('DSN can be overridden via match callback', async () => { expect.assertions(1); diff --git a/packages/core/test/lib/transports/offline.test.ts b/packages/core/test/lib/transports/offline.test.ts index d7d2ee7a90ae..056d11aac90a 100644 --- a/packages/core/test/lib/transports/offline.test.ts +++ b/packages/core/test/lib/transports/offline.test.ts @@ -38,7 +38,7 @@ const REPLAY_EVENT: ReplayEvent = { replay_type: 'buffer', }; -const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337'); +const DSN = dsnFromString('https://public@dsn.ingest.sentry.io/1337')!; const DATA = 'nothing'; diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index d0be07a3c8d0..665b557e4e7b 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -123,6 +123,13 @@ function addClientIntegrations(options: BrowserOptions): void { options.integrations = integrations; } +/** + * Just a passthrough in case this is imported from the client. + */ +export function withSentryConfig(exportedUserNextConfig: T): T { + return exportedUserNextConfig; +} + export { // eslint-disable-next-line deprecation/deprecation withSentryServerSideGetInitialProps, diff --git a/packages/nextjs/src/client/tunnelRoute.ts b/packages/nextjs/src/client/tunnelRoute.ts index 81f9f936cc82..6f10b190727a 100644 --- a/packages/nextjs/src/client/tunnelRoute.ts +++ b/packages/nextjs/src/client/tunnelRoute.ts @@ -12,6 +12,9 @@ export function applyTunnelRouteOption(options: BrowserOptions): void { const tunnelRouteOption = globalWithInjectedValues.__sentryRewritesTunnelPath__; if (tunnelRouteOption && options.dsn) { const dsnComponents = dsnFromString(options.dsn); + if (!dsnComponents) { + return; + } const sentrySaasDsnMatch = dsnComponents.host.match(/^o(\d+)\.ingest\.sentry\.io$/); if (sentrySaasDsnMatch) { const orgId = sentrySaasDsnMatch[1]; diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 28c8f58b2eb9..3157d41df71f 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -202,8 +202,13 @@ export default function wrappingLoader( templateCode = templateCode.replace(/__COMPONENT_TYPE__/g, 'Unknown'); } - if (sentryConfigFilePath) { - templateCode = `import "${sentryConfigFilePath}";`.concat(templateCode); + // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, + // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. + if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + .replace(/\\/g, '/'); + templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); } } else if (wrappingTargetKind === 'middleware') { templateCode = middlewareWrapperTemplateCode; diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index ca56ce3facbe..8e8edfdc3b07 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -126,6 +126,13 @@ export function lastEventId(): string | undefined { return getCurrentHub().lastEventId(); } +/** + * Just a passthrough in case this is imported from the client. + */ +export function withSentryConfig(exportedUserNextConfig: T): T { + return exportedUserNextConfig; +} + export { flush } from './utils/flush'; export * from '@sentry/core'; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index a75b61e02ea3..6fcf7436399b 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -39,6 +39,8 @@ export declare const withErrorBoundary: typeof clientSdk.withErrorBoundary; export declare const Span: typeof edgeSdk.Span; export declare const Transaction: typeof edgeSdk.Transaction; +export { withSentryConfig } from './config'; + /** * @deprecated Use `wrapApiHandlerWithSentry` instead */ diff --git a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts index 7c3256c5e6a7..88634c92012e 100644 --- a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts @@ -31,7 +31,12 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag expect(transaction[0].spans).toEqual( expect.arrayContaining([ expect.objectContaining({ - data: { 'http.method': 'GET', url: 'http://example.com', type: 'fetch' }, + data: { + 'http.method': 'GET', + url: 'http://example.com', + type: 'fetch', + 'http.response_content_length': expect.any(Number), + }, description: 'GET http://example.com', op: 'http.client', parent_span_id: expect.any(String), diff --git a/packages/nextjs/test/integration/test/server/utils/helpers.ts b/packages/nextjs/test/integration/test/server/utils/helpers.ts index e483f08a144c..c7213785493f 100644 --- a/packages/nextjs/test/integration/test/server/utils/helpers.ts +++ b/packages/nextjs/test/integration/test/server/utils/helpers.ts @@ -9,7 +9,7 @@ import next from 'next'; // Type not exported from NextJS // @ts-ignore export const createNextServer = async config => { - const app = next(config); + const app = next({ ...config, customServer: false }); // customServer: false because: https://github.com/vercel/next.js/pull/49805#issuecomment-1557321794 const handle = app.getRequestHandler(); await app.prepare(); diff --git a/packages/nextjs/test/utils/tunnelRoute.test.ts b/packages/nextjs/test/utils/tunnelRoute.test.ts index 0e0a6d48f47c..62b2fb28cf9c 100644 --- a/packages/nextjs/test/utils/tunnelRoute.test.ts +++ b/packages/nextjs/test/utils/tunnelRoute.test.ts @@ -11,7 +11,7 @@ beforeEach(() => { }); describe('applyTunnelRouteOption()', () => { - it('should correctly apply `tunnelRoute` option when conditions are met', () => { + it('Correctly applies `tunnelRoute` option when conditions are met', () => { globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; const options: any = { dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333', @@ -22,7 +22,7 @@ describe('applyTunnelRouteOption()', () => { expect(options.tunnel).toBe('/my-error-monitoring-route?o=2222222&p=3333333'); }); - it('should not apply `tunnelRoute` when DSN is missing', () => { + it("Doesn't apply `tunnelRoute` when DSN is missing", () => { globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; const options: any = { // no dsn @@ -33,7 +33,18 @@ describe('applyTunnelRouteOption()', () => { expect(options.tunnel).toBeUndefined(); }); - it("should not apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => { + it("Doesn't apply `tunnelRoute` when DSN is invalid", () => { + globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; + const options: any = { + dsn: 'invalidDsn', + } as BrowserOptions; + + applyTunnelRouteOption(options); + + expect(options.tunnel).toBeUndefined(); + }); + + it("Doesn't apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => { const options: any = { dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333', } as BrowserOptions; @@ -43,7 +54,7 @@ describe('applyTunnelRouteOption()', () => { expect(options.tunnel).toBeUndefined(); }); - it('should not apply `tunnelRoute` option when DSN is not a SaaS DSN', () => { + it("Doesn't `tunnelRoute` option when DSN is not a SaaS DSN", () => { globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; const options: any = { dsn: 'https://11111111111111111111111111111111@example.com/3333333', diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 7424e4b10a26..ccdbeda279e1 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -186,6 +186,8 @@ export class NodeClient extends BaseClient { } const envelope = createCheckInEnvelope(serializedCheckIn, this.getSdkMetadata(), tunnel, this.getDsn()); + + __DEBUG_BUILD__ && logger.info('Sending checkin:', checkIn.monitorSlug, checkIn.status); void this._sendEnvelope(envelope); return id; } diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 41fe2a68c058..5e2ce9e253a2 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -138,7 +138,7 @@ function _createWrappedRequestMethodFactory( ): WrappedRequestMethodFactory { // We're caching results so we don't have to recompute regexp every time we create a request. const createSpanUrlMap = new LRUMap(100); - const headersUrlMap: Record = {}; + const headersUrlMap = new LRUMap(100); const shouldCreateSpan = (url: string): boolean => { if (tracingOptions?.shouldCreateSpanForRequest === undefined) { @@ -160,13 +160,14 @@ function _createWrappedRequestMethodFactory( return true; } - if (headersUrlMap[url]) { - return headersUrlMap[url]; + const cachedDecision = headersUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; } - headersUrlMap[url] = stringMatchesSomePattern(url, tracingOptions.tracePropagationTargets); - - return headersUrlMap[url]; + const decision = stringMatchesSomePattern(url, tracingOptions.tracePropagationTargets); + headersUrlMap.set(url, decision); + return decision; }; return function wrappedRequestMethodFactory(originalRequestMethod: OriginalRequestMethod): WrappedRequestMethod { diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index b609cba70b50..38f920283b7e 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -3,9 +3,11 @@ import type { EventProcessor, Integration } from '@sentry/types'; import { dynamicRequire, dynamicSamplingContextToSentryBaggageHeader, + getSanitizedUrlString, + parseUrl, stringMatchesSomePattern, - stripUrlQueryAndFragment, } from '@sentry/utils'; +import { LRUMap } from 'lru_map'; import type { NodeClient } from '../../client'; import { NODE_VERSION } from '../../nodeVersion'; @@ -29,12 +31,21 @@ export interface UndiciOptions { * Function determining whether or not to create spans to track outgoing requests to the given URL. * By default, spans will be created for all outgoing requests. */ - shouldCreateSpanForRequest: (url: string) => boolean; + shouldCreateSpanForRequest?: (url: string) => boolean; } // Please note that you cannot use `console.log` to debug the callbacks registered to the `diagnostics_channel` API. // To debug, you can use `writeFileSync` to write to a file: // https://nodejs.org/api/async_hooks.html#printing-in-asynchook-callbacks +// +// import { writeFileSync } from 'fs'; +// import { format } from 'util'; +// +// function debug(...args: any): void { +// // Use a function like this one when debugging inside an AsyncHook callback +// // @ts-ignore any +// writeFileSync('log.out', `${format(...args)}\n`, { flag: 'a' }); +// } /** * Instruments outgoing HTTP requests made with the `undici` package via @@ -57,10 +68,13 @@ export class Undici implements Integration { private readonly _options: UndiciOptions; + private readonly _createSpanUrlMap: LRUMap = new LRUMap(100); + private readonly _headersUrlMap: LRUMap = new LRUMap(100); + public constructor(_options: Partial = {}) { this._options = { breadcrumbs: _options.breadcrumbs === undefined ? true : _options.breadcrumbs, - shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest || (() => true), + shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest, }; } @@ -85,6 +99,21 @@ export class Undici implements Integration { return; } + const shouldCreateSpan = (url: string): boolean => { + if (this._options.shouldCreateSpanForRequest === undefined) { + return true; + } + + const cachedDecision = this._createSpanUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = this._options.shouldCreateSpanForRequest(url); + this._createSpanUrlMap.set(url, decision); + return decision; + }; + // https://github.com/nodejs/undici/blob/e6fc80f809d1217814c044f52ed40ef13f21e43c/docs/api/DiagnosticsChannel.md ds.subscribe(ChannelName.RequestCreate, message => { const hub = getCurrentHub(); @@ -94,8 +123,8 @@ export class Undici implements Integration { const { request } = message as RequestCreateMessage; - const url = new URL(request.path, request.origin); - const stringUrl = url.toString(); + const stringUrl = request.origin ? request.origin.toString() + request.path : request.path; + const url = parseUrl(stringUrl); if (isSentryRequest(stringUrl) || request.__sentry__ !== undefined) { return; @@ -108,32 +137,41 @@ export class Undici implements Integration { if (activeSpan && client) { const clientOptions = client.getOptions(); - const shouldCreateSpan = this._options.shouldCreateSpanForRequest(stringUrl); - if (shouldCreateSpan) { + if (shouldCreateSpan(stringUrl)) { const method = request.method || 'GET'; const data: Record = { 'http.method': method, }; - const params = url.searchParams.toString(); - if (params) { - data['http.query'] = `?${params}`; + if (url.search) { + data['http.query'] = url.search; } if (url.hash) { data['http.fragment'] = url.hash; } const span = activeSpan.startChild({ op: 'http.client', - description: `${method} ${stripUrlQueryAndFragment(stringUrl)}`, + description: `${method} ${getSanitizedUrlString(url)}`, data, }); request.__sentry__ = span; - const shouldPropagate = clientOptions.tracePropagationTargets - ? stringMatchesSomePattern(stringUrl, clientOptions.tracePropagationTargets) - : true; + const shouldAttachTraceData = (url: string): boolean => { + if (clientOptions.tracePropagationTargets === undefined) { + return true; + } + + const cachedDecision = this._headersUrlMap.get(url); + if (cachedDecision !== undefined) { + return cachedDecision; + } + + const decision = stringMatchesSomePattern(url, clientOptions.tracePropagationTargets); + this._headersUrlMap.set(url, decision); + return decision; + }; - if (shouldPropagate) { + if (shouldAttachTraceData(stringUrl)) { request.addHeader('sentry-trace', span.toTraceparent()); if (span.transaction) { const dynamicSamplingContext = span.transaction.getDynamicSamplingContext(); @@ -155,8 +193,7 @@ export class Undici implements Integration { const { request, response } = message as RequestEndMessage; - const url = new URL(request.path, request.origin); - const stringUrl = url.toString(); + const stringUrl = request.origin ? request.origin.toString() + request.path : request.path; if (isSentryRequest(stringUrl)) { return; @@ -196,8 +233,7 @@ export class Undici implements Integration { const { request } = message as RequestErrorMessage; - const url = new URL(request.path, request.origin); - const stringUrl = url.toString(); + const stringUrl = request.origin ? request.origin.toString() + request.path : request.path; if (isSentryRequest(stringUrl)) { return; diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index ab3c6bfe346e..f4925f7046a2 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -132,6 +132,25 @@ conditionalTest({ min: 16 })('Undici integration', () => { expect(span).toEqual(expect.objectContaining({ status: 'internal_error' })); }); + it('creates a span for invalid looking urls', async () => { + const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; + hub.getScope().setSpan(transaction); + + try { + // Intentionally add // to the url + // fetch accepts this URL, but throws an error later on + await fetch('http://a-url-that-no-exists.com//'); + } catch (e) { + // ignore + } + + expect(transaction.spanRecorder?.spans.length).toBe(2); + + const span = transaction.spanRecorder?.spans[1]; + expect(span).toEqual(expect.objectContaining({ description: 'GET http://a-url-that-no-exists.com//' })); + expect(span).toEqual(expect.objectContaining({ status: 'internal_error' })); + }); + it('does not create a span for sentry requests', async () => { const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; hub.getScope().setSpan(transaction); diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index 58b2710f1ac5..4b914f234981 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -83,6 +83,9 @@ const defaultOptions = { textEncoder, }; +// empty function to keep test output clean +const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + describe('makeNewHttpTransport()', () => { afterEach(() => { jest.clearAllMocks(); @@ -403,7 +406,6 @@ describe('makeNewHttpTransport()', () => { }); it('should warn if an invalid url is passed', async () => { - const consoleWarnSpy = jest.spyOn(console, 'warn'); const transport = makeNodeTransport({ ...defaultOptions, url: 'invalid url' }); await transport.send(EVENT_ENVELOPE); expect(consoleWarnSpy).toHaveBeenCalledWith( diff --git a/packages/replay/README.md b/packages/replay/README.md index a790370c2d49..091f51d785bf 100644 --- a/packages/replay/README.md +++ b/packages/replay/README.md @@ -195,17 +195,17 @@ The following options can be configured as options to the integration, in `new R The following options can be configured as options to the integration, in `new Replay({})`: -| key | type | default | description | -| ---------------- | ------------------------ | ----------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| maskAllText | boolean | `true` | Mask _all_ text content. Will pass text content through `maskTextFn` before sending to server. | -| maskAllInputs | boolean | `true` | Mask values of `` elements. Passes input values through `maskInputFn` before sending to server. | -| blockAllMedia | boolean | `true` | Block _all_ media elements (`img, svg, video, object, picture, embed, map, audio`) -| maskTextFn | (text: string) => string | `(text) => '*'.repeat(text.length)` | Function to customize how text content is masked before sending to server. By default, masks text with `*`. | -| block | Array | `.sentry-block, [data-sentry-block]` | Redact any elements that match the DOM selectors. See [privacy](#blocking) section for an example. | -| unblock | Array | `.sentry-unblock, [data-sentry-unblock]`| Do not redact any elements that match the DOM selectors. Useful when using `blockAllMedia`. See [privacy](#blocking) section for an example. | -| mask | Array | `.sentry-mask, [data-sentry-mask]` | Mask all elements that match the given DOM selectors. See [privacy](#masking) section for an example. | -| unmask | Array | `.sentry-unmask, [data-sentry-unmask]` | Unmask all elements that match the given DOM selectors. Useful when using `maskAllText`. See [privacy](#masking) section for an example. | -| ignore | Array | `.sentry-ignore, [data-sentry-ignore]` | Ignores all events on the matching input fields. See [privacy](#ignoring) section for an example. | +| key | type | default | description | +| ---------------- | ------------------------ | --------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| maskAllText | boolean | `true` | Mask _all_ text content. Will pass text content through `maskFn` before sending to server. | +| maskAllInputs | boolean | `true` | Mask values of `` elements. Passes input values through `maskInputFn` before sending to server. | +| blockAllMedia | boolean | `true` | Block _all_ media elements (`img, svg, video, object, picture, embed, map, audio`) | +| maskFn | (text: string) => string | `(text) => '*'.repeat(text.length)` | Function to customize how text content is masked before sending to server. By default, masks text with `*`. | +| block | Array | `.sentry-block, [data-sentry-block]` | Redact any elements that match the DOM selectors. See [privacy](#blocking) section for an example. | +| unblock | Array | `.sentry-unblock, [data-sentry-unblock]`| Do not redact any elements that match the DOM selectors. Useful when using `blockAllMedia`. See [privacy](#blocking) section for an example. | +| mask | Array | `.sentry-mask, [data-sentry-mask]` | Mask all elements that match the given DOM selectors. See [privacy](#masking) section for an example. | +| unmask | Array | `.sentry-unmask, [data-sentry-unmask]` | Unmask all elements that match the given DOM selectors. Useful when using `maskAllText`. See [privacy](#masking) section for an example. | +| ignore | Array | `.sentry-ignore, [data-sentry-ignore]` | Ignores all events on the matching input fields. See [privacy](#ignoring) section for an example. | #### Deprecated options In order to streamline our privacy options, the following have been deprecated in favor for the respective options above. diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index d5d4115fffc7..114d015f2702 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -72,6 +72,8 @@ export class Replay implements Integration { ignore = [], maskFn, + beforeAddRecordingEvent, + // eslint-disable-next-line deprecation/deprecation blockClass, // eslint-disable-next-line deprecation/deprecation @@ -129,6 +131,7 @@ export class Replay implements Integration { networkCaptureBodies, networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders), networkResponseHeaders: _getMergedNetworkHeaders(networkResponseHeaders), + beforeAddRecordingEvent, _experiments, }; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 8d89aa0b2653..e45830e9fdde 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -463,7 +463,17 @@ export class ReplayContainer implements ReplayContainerInterface { } /** - * + * Only flush if `this.recordingMode === 'session'` + */ + public conditionalFlush(): Promise { + if (this.recordingMode === 'buffer') { + return Promise.resolve(); + } + + return this.flushImmediate(); + } + + /** * Always flush via `_debouncedFlush` so that we do not have flushes triggered * from calling both `flush` and `_debouncedFlush`. Otherwise, there could be * cases of mulitple flushes happening closely together. @@ -474,6 +484,13 @@ export class ReplayContainer implements ReplayContainerInterface { return this._debouncedFlush.flush() as Promise; } + /** + * Cancels queued up flushes. + */ + public cancelFlush(): void { + this._debouncedFlush.cancel(); + } + /** Get the current sesion (=replay) ID */ public getSessionId(): string | undefined { return this.session && this.session.id; @@ -723,7 +740,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Send replay when the page/tab becomes hidden. There is no reason to send // replay if it becomes visible, since no actions we care about were done // while it was hidden - this._conditionalFlush(); + void this.conditionalFlush(); } /** @@ -807,17 +824,6 @@ export class ReplayContainer implements ReplayContainerInterface { return Promise.all(createPerformanceSpans(this, createPerformanceEntries(entries))); } - /** - * Only flush if `this.recordingMode === 'session'` - */ - private _conditionalFlush(): void { - if (this.recordingMode === 'buffer') { - return; - } - - void this.flushImmediate(); - } - /** * Clear _context */ diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 12900dc22e74..ebea87d0f363 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -183,6 +183,10 @@ export interface WorkerResponse { export type AddEventResult = void; +export interface BeforeAddRecordingEvent { + (event: RecordingEvent): RecordingEvent | null | undefined; +} + export interface ReplayNetworkOptions { /** * Capture request/response details for XHR/Fetch requests that match the given URLs. @@ -267,6 +271,19 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ maskAllText: boolean; + /** + * Callback before adding a custom recording event + * + * Events added by the underlying DOM recording library can *not* be modified, + * only custom recording events from the Replay integration will trigger the + * callback listeners. This can be used to scrub certain fields in an event (e.g. URLs from navigation events). + * + * Returning a `null` will drop the event completely. Note, dropping a recording + * event is not the same as dropping the replay, the replay will still exist and + * continue to function. + */ + beforeAddRecordingEvent?: BeforeAddRecordingEvent; + /** * _experiments allows users to enable experimental or internal features. * We don't consider such features as part of the public API and hence we don't guarantee semver for them. @@ -285,6 +302,7 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { scrollTimeout: number; ignoreSelectors: string[]; }; + delayFlushOnCheckout: number; }>; } @@ -330,7 +348,7 @@ export interface ReplayIntegrationPrivacyOptions { /** * A callback function to customize how your text is masked. */ - maskFn?: Pick; + maskFn?: (s: string) => string; } // These are optional for ReplayPluginOptions because the plugin sets default values @@ -515,7 +533,9 @@ export interface ReplayContainer { startRecording(): void; stopRecording(): boolean; sendBufferedReplayOrFlush(options?: SendBufferedReplayOptions): Promise; + conditionalFlush(): Promise; flushImmediate(): Promise; + cancelFlush(): void; triggerUserActivity(): void; addUpdate(cb: AddUpdateCallback): void; getOptions(): ReplayPluginOptions; diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 8064515c3f87..e40a9c2f6486 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -2,6 +2,7 @@ import { getCurrentHub } from '@sentry/core'; import { logger } from '@sentry/utils'; import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types'; +import { EventType } from '../types/rrweb'; import { timestampToMs } from './timestampToMs'; /** @@ -38,7 +39,18 @@ export async function addEvent( replay.eventBuffer.clear(); } - return await replay.eventBuffer.addEvent(event); + const replayOptions = replay.getOptions(); + + const eventAfterPossibleCallback = + typeof replayOptions.beforeAddRecordingEvent === 'function' && event.type === EventType.Custom + ? replayOptions.beforeAddRecordingEvent(event) + : event; + + if (!eventAfterPossibleCallback) { + return; + } + + return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { __DEBUG_BUILD__ && logger.error(error); await replay.stop('addEvent'); diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index f72850f5536c..3a9dcc211edd 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -80,6 +80,30 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa } } + const options = replay.getOptions(); + + // TODO: We want this as an experiment so that we can test + // internally and create metrics before making this the default + if (options._experiments.delayFlushOnCheckout) { + // If the full snapshot is due to an initial load, we will not have + // a previous session ID. In this case, we want to buffer events + // for a set amount of time before flushing. This can help avoid + // capturing replays of users that immediately close the window. + setTimeout(() => replay.conditionalFlush(), options._experiments.delayFlushOnCheckout); + + // Cancel any previously debounced flushes to ensure there are no [near] + // simultaneous flushes happening. The latter request should be + // insignificant in this case, so wait for additional user interaction to + // trigger a new flush. + // + // This can happen because there's no guarantee that a recording event + // happens first. e.g. a mouse click can happen and trigger a debounced + // flush before the checkout. + replay.cancelFlush(); + + return true; + } + // Flush immediately so that we do not miss the first segment, otherwise // it can prevent loading on the UI. This will cause an increase in short // replays (e.g. opening and closing a tab quickly), but these can be diff --git a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts new file mode 100644 index 000000000000..c01140045389 --- /dev/null +++ b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts @@ -0,0 +1,145 @@ +import * as SentryCore from '@sentry/core'; +import type { Transport } from '@sentry/types'; +import * as SentryUtils from '@sentry/utils'; + +import type { Replay } from '../../src'; +import type { ReplayContainer } from '../../src/replay'; +import { clearSession } from '../../src/session/clearSession'; +import * as SendReplayRequest from '../../src/util/sendReplayRequest'; +import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; +import { useFakeTimers } from '../utils/use-fake-timers'; + +useFakeTimers(); + +async function advanceTimers(time: number) { + jest.advanceTimersByTime(time); + await new Promise(process.nextTick); +} + +type MockTransportSend = jest.MockedFunction; + +describe('Integration | beforeAddRecordingEvent', () => { + let replay: ReplayContainer; + let integration: Replay; + let mockTransportSend: MockTransportSend; + let mockSendReplayRequest: jest.SpyInstance; + let domHandler: (args: any) => any; + const { record: mockRecord } = mockRrweb(); + + beforeAll(async () => { + jest.setSystemTime(new Date(BASE_TIMESTAMP)); + jest.spyOn(SentryUtils, 'addInstrumentationHandler').mockImplementation((type, handler: (args: any) => any) => { + if (type === 'dom') { + domHandler = handler; + } + }); + + ({ replay, integration } = await mockSdk({ + replayOptions: { + beforeAddRecordingEvent: event => { + const eventData = event.data as Record; + + if (eventData.tag === 'breadcrumb' && eventData.payload.category === 'ui.click') { + return { + ...event, + data: { + ...eventData, + payload: { + ...eventData.payload, + message: 'beforeAddRecordingEvent', + }, + }, + }; + } + + // This should not do anything because callback should not be called + // for `event.type != 5` + if (event.type === 2) { + return null; + } + + if (eventData.tag === 'options') { + return null; + } + + return event; + }, + _experiments: { + captureExceptions: true, + }, + }, + })); + + mockSendReplayRequest = jest.spyOn(SendReplayRequest, 'sendReplayRequest'); + + jest.runAllTimers(); + mockTransportSend = SentryCore.getCurrentHub()?.getClient()?.getTransport()?.send as MockTransportSend; + }); + + beforeEach(() => { + jest.setSystemTime(new Date(BASE_TIMESTAMP)); + mockRecord.takeFullSnapshot.mockClear(); + mockTransportSend.mockClear(); + + // Create a new session and clear mocks because a segment (from initial + // checkout) will have already been uploaded by the time the tests run + clearSession(replay); + replay['_loadAndCheckSession'](); + + mockSendReplayRequest.mockClear(); + }); + + afterEach(async () => { + jest.runAllTimers(); + await new Promise(process.nextTick); + jest.setSystemTime(new Date(BASE_TIMESTAMP)); + clearSession(replay); + replay['_loadAndCheckSession'](); + }); + + afterAll(() => { + integration && integration.stop(); + }); + + it('changes click breadcrumbs message', async () => { + domHandler({ + name: 'click', + }); + + await advanceTimers(5000); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + recordingData: JSON.stringify([ + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'ui.click', + message: 'beforeAddRecordingEvent', + data: {}, + }, + }, + }, + ]), + }); + }); + + it('filters out the options event, but *NOT* full snapshot', async () => { + mockTransportSend.mockClear(); + await integration.stop(); + + integration.start(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }]), + }); + }); +}); diff --git a/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts b/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts new file mode 100644 index 000000000000..c0b711c028f8 --- /dev/null +++ b/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts @@ -0,0 +1,759 @@ +import { captureException, getCurrentHub } from '@sentry/core'; + +import { + BUFFER_CHECKOUT_TIME, + DEFAULT_FLUSH_MIN_DELAY, + MAX_SESSION_LIFE, + REPLAY_SESSION_KEY, + SESSION_IDLE_EXPIRE_DURATION, + WINDOW, +} from '../../src/constants'; +import type { ReplayContainer } from '../../src/replay'; +import { clearSession } from '../../src/session/clearSession'; +import { addEvent } from '../../src/util/addEvent'; +import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; +import { PerformanceEntryResource } from '../fixtures/performanceEntry/resource'; +import type { RecordMock } from '../index'; +import { BASE_TIMESTAMP } from '../index'; +import { resetSdkMock } from '../mocks/resetSdkMock'; +import type { DomHandler } from '../types'; +import { useFakeTimers } from '../utils/use-fake-timers'; + +useFakeTimers(); + +async function advanceTimers(time: number) { + jest.advanceTimersByTime(time); + await new Promise(process.nextTick); +} + +async function waitForBufferFlush() { + await new Promise(process.nextTick); + await new Promise(process.nextTick); +} + +async function waitForFlush() { + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); +} + +describe('Integration | errorSampleRate with delayed flush', () => { + let replay: ReplayContainer; + let mockRecord: RecordMock; + let domHandler: DomHandler; + + beforeEach(async () => { + ({ mockRecord, domHandler, replay } = await resetSdkMock({ + replayOptions: { + stickySession: true, + _experiments: { + delayFlushOnCheckout: DEFAULT_FLUSH_MIN_DELAY, + }, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + }); + + afterEach(async () => { + clearSession(replay); + replay.stop(); + }); + + it('uploads a replay when `Sentry.captureException` is called and continues recording', async () => { + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + // Does not capture on mouse click + domHandler({ + name: 'click', + }); + jest.runAllTimers(); + await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + + captureException(new Error('testing')); + + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + + await waitForFlush(); + + // This is from when we stop recording and start a session recording + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 40, type: 2 }]), + }); + + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + + // Check that click will get captured + domHandler({ + name: 'click', + }); + + await waitForFlush(); + + expect(replay).toHaveLastSentReplay({ + recordingData: JSON.stringify([ + { + type: 5, + timestamp: BASE_TIMESTAMP + 10000 + 80, + data: { + tag: 'breadcrumb', + payload: { + timestamp: (BASE_TIMESTAMP + 10000 + 80) / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + }); + + it('manually flushes replay and does not continue to record', async () => { + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + // Does not capture on mouse click + domHandler({ + name: 'click', + }); + jest.runAllTimers(); + await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + + replay.sendBufferedReplayOrFlush({ continueRecording: false }); + + await waitForBufferFlush(); + + expect(replay).toHaveSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + // Check that click will not get captured + domHandler({ + name: 'click', + }); + + await waitForFlush(); + + // This is still the last replay sent since we passed `continueRecording: + // false`. + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + { + type: 5, + timestamp: BASE_TIMESTAMP, + data: { + tag: 'breadcrumb', + payload: { + timestamp: BASE_TIMESTAMP / 1000, + type: 'default', + category: 'ui.click', + message: '', + data: {}, + }, + }, + }, + ]), + }); + }); + + it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_EXPIRE_DURATION]ms', async () => { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'visible'; + }, + }); + + jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); + + document.dispatchEvent(new Event('visibilitychange')); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + }); + + it('does not send a replay if user hides the tab and comes back within 60 seconds', async () => { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'hidden'; + }, + }); + document.dispatchEvent(new Event('visibilitychange')); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + + // User comes back before `SESSION_IDLE_EXPIRE_DURATION` elapses + jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION - 100); + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'visible'; + }, + }); + document.dispatchEvent(new Event('visibilitychange')); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + }); + + it('does not upload a replay event when document becomes hidden', async () => { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { + return 'hidden'; + }, + }); + + // Pretend 5 seconds have passed + const ELAPSED = 5000; + jest.advanceTimersByTime(ELAPSED); + + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; + addEvent(replay, TEST_EVENT); + + document.dispatchEvent(new Event('visibilitychange')); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + }); + + it('does not upload a replay event if 5 seconds have elapsed since the last replay event occurred', async () => { + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + // Pretend 5 seconds have passed + const ELAPSED = 5000; + await advanceTimers(ELAPSED); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + }); + + it('does not upload a replay event if 15 seconds have elapsed since the last replay upload', async () => { + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + // Fire a new event every 4 seconds, 4 times + [...Array(4)].forEach(() => { + mockRecord._emitter(TEST_EVENT); + jest.advanceTimersByTime(4000); + }); + + // We are at time = +16seconds now (relative to BASE_TIMESTAMP) + // The next event should cause an upload immediately + mockRecord._emitter(TEST_EVENT); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + + // There should also not be another attempt at an upload 5 seconds after the last replay event + await waitForFlush(); + expect(replay).not.toHaveLastSentReplay(); + + // Let's make sure it continues to work + mockRecord._emitter(TEST_EVENT); + await waitForFlush(); + jest.runAllTimers(); + await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + }); + + // When the error session records as a normal session, we want to stop + // recording after the session ends. Otherwise, we get into a state where the + // new session is a session type replay (this could conflict with the session + // sample rate of 0.0), or an error session that has no errors. Instead we + // simply stop the session replay completely and wait for a new page load to + // resample. + it.each([ + ['MAX_SESSION_LIFE', MAX_SESSION_LIFE], + ['SESSION_IDLE_DURATION', SESSION_IDLE_EXPIRE_DURATION], + ])( + 'stops replay if session had an error and exceeds %s and does not start a new session thereafter', + async (_label, waitTime) => { + expect(replay.session?.shouldRefresh).toBe(true); + + captureException(new Error('testing')); + + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + + await waitForFlush(); + + // segment_id is 1 because it sends twice on error + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + expect(replay.session?.shouldRefresh).toBe(false); + + // Idle for given time + jest.advanceTimersByTime(waitTime + 1); + await new Promise(process.nextTick); + + const TEST_EVENT = { + data: { name: 'lost event' }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + mockRecord._emitter(TEST_EVENT); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // We stop recording after 15 minutes of inactivity in error mode + + // still no new replay sent + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + + expect(replay.isEnabled()).toBe(false); + + domHandler({ + name: 'click', + }); + + // Remains disabled! + expect(replay.isEnabled()).toBe(false); + }, + ); + + it.each([ + ['MAX_SESSION_LIFE', MAX_SESSION_LIFE], + ['SESSION_IDLE_EXPIRE_DURATION', SESSION_IDLE_EXPIRE_DURATION], + ])('continues buffering replay if session had no error and exceeds %s', async (_label, waitTime) => { + expect(replay).not.toHaveLastSentReplay(); + + // Idle for given time + jest.advanceTimersByTime(waitTime + 1); + await new Promise(process.nextTick); + + const TEST_EVENT = { + data: { name: 'lost event' }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + mockRecord._emitter(TEST_EVENT); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // still no new replay sent + expect(replay).not.toHaveLastSentReplay(); + + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('buffer'); + + domHandler({ + name: 'click', + }); + + await waitForFlush(); + + expect(replay).not.toHaveLastSentReplay(); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('buffer'); + + // should still react to errors later on + captureException(new Error('testing')); + + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('session'); + expect(replay.session?.sampled).toBe('buffer'); + expect(replay.session?.shouldRefresh).toBe(false); + }); + + // Should behave the same as above test + it('stops replay if user has been idle for more than SESSION_IDLE_EXPIRE_DURATION and does not start a new session thereafter', async () => { + // Idle for 15 minutes + jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); + + const TEST_EVENT = { + data: { name: 'lost event' }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + mockRecord._emitter(TEST_EVENT); + expect(replay).not.toHaveLastSentReplay(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // We stop recording after SESSION_IDLE_EXPIRE_DURATION of inactivity in error mode + expect(replay).not.toHaveLastSentReplay(); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('buffer'); + + // should still react to errors later on + captureException(new Error('testing')); + + await new Promise(process.nextTick); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('session'); + expect(replay.session?.sampled).toBe('buffer'); + expect(replay.session?.shouldRefresh).toBe(false); + }); + + it('has the correct timestamps with deferred root event and last replay update', async () => { + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + const optionsEvent = createOptionsEvent(replay); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + + captureException(new Error('testing')); + + await new Promise(process.nextTick); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + + expect(replay).toHaveSentReplay({ + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + ]), + replayEventPayload: expect.objectContaining({ + replay_start_timestamp: BASE_TIMESTAMP / 1000, + // the exception happens roughly 10 seconds after BASE_TIMESTAMP + // (advance timers + waiting for flush after the checkout) and + // extra time is likely due to async of `addMemoryEntry()` + + timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 40) / 1000, + error_ids: [expect.any(String)], + trace_ids: [], + urls: ['http://localhost/'], + replay_id: expect.any(String), + }), + recordingPayloadHeader: { segment_id: 0 }, + }); + }); + + it('has correct timestamps when error occurs much later than initial pageload/checkout', async () => { + const ELAPSED = BUFFER_CHECKOUT_TIME; + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + + // add a mock performance event + replay.performanceEvents.push(PerformanceEntryResource()); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + jest.advanceTimersByTime(ELAPSED); + + // in production, this happens at a time interval + // session started time should be updated to this current timestamp + mockRecord.takeFullSnapshot(true); + const optionsEvent = createOptionsEvent(replay); + + jest.runAllTimers(); + jest.advanceTimersByTime(20); + await new Promise(process.nextTick); + + captureException(new Error('testing')); + + await waitForBufferFlush(); + + expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 20); + + // Does not capture mouse click + expect(replay).toHaveSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + // Make sure the old performance event is thrown out + replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + 20) / 1000, + }), + recordingData: JSON.stringify([ + { + data: { isCheckout: true }, + timestamp: BASE_TIMESTAMP + ELAPSED + 20, + type: 2, + }, + optionsEvent, + ]), + }); + }); + + it('stops replay when user goes idle', async () => { + jest.setSystemTime(BASE_TIMESTAMP); + + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + captureException(new Error('testing')); + + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay(); + + // Flush from calling `stopRecording` + await waitForFlush(); + + // Now wait after session expires - should stop recording + mockRecord.takeFullSnapshot.mockClear(); + (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); + + expect(replay).not.toHaveLastSentReplay(); + + // Go idle + jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + + expect(replay).not.toHaveLastSentReplay(); + + await waitForFlush(); + + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(false); + }); + + it('stops replay when session exceeds max length', async () => { + jest.setSystemTime(BASE_TIMESTAMP); + + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + captureException(new Error('testing')); + + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + + expect(replay).toHaveLastSentReplay(); + + // Wait a bit, shortly before session expires + jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + replay.triggerUserActivity(); + + expect(replay).toHaveLastSentReplay(); + + // Now wait after session expires - should stop recording + mockRecord.takeFullSnapshot.mockClear(); + (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); + + jest.advanceTimersByTime(10_000); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + replay.triggerUserActivity(); + + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(false); + }); +}); + +/** + * This is testing a case that should only happen with error-only sessions. + * Previously we had assumed that loading a session from session storage meant + * that the session was not new. However, this is not the case with error-only + * sampling since we can load a saved session that did not have an error (and + * thus no replay was created). + */ +it('sends a replay after loading the session from storage', async () => { + // Pretend that a session is already saved before loading replay + WINDOW.sessionStorage.setItem( + REPLAY_SESSION_KEY, + `{"segmentId":0,"id":"fd09adfc4117477abc8de643e5a5798a","sampled":"buffer","started":${BASE_TIMESTAMP},"lastActivity":${BASE_TIMESTAMP}}`, + ); + const { mockRecord, replay, integration } = await resetSdkMock({ + replayOptions: { + stickySession: true, + _experiments: { + delayFlushOnCheckout: DEFAULT_FLUSH_MIN_DELAY, + }, + }, + sentryOptions: { + replaysOnErrorSampleRate: 1.0, + }, + autoStart: false, + }); + integration['_initialize'](); + const optionsEvent = createOptionsEvent(replay); + + jest.runAllTimers(); + + await new Promise(process.nextTick); + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; + mockRecord._emitter(TEST_EVENT); + + expect(replay).not.toHaveLastSentReplay(); + + captureException(new Error('testing')); + + // 2 ticks to send replay from an error + await waitForBufferFlush(); + + // Buffered events before error + expect(replay).toHaveSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, + optionsEvent, + TEST_EVENT, + ]), + }); + + // `startRecording()` after switching to session mode to continue recording + await waitForFlush(); + + // Latest checkout when we call `startRecording` again after uploading segment + // after an error occurs (e.g. when we switch to session replay recording) + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, type: 2 }, + ]), + }); +}); diff --git a/packages/replay/test/unit/util/createReplayEnvelope.test.ts b/packages/replay/test/unit/util/createReplayEnvelope.test.ts index 76f22709b4cb..150da47edf00 100644 --- a/packages/replay/test/unit/util/createReplayEnvelope.test.ts +++ b/packages/replay/test/unit/util/createReplayEnvelope.test.ts @@ -42,7 +42,7 @@ describe('Unit | util | createReplayEnvelope', () => { projectId: '123', protocol: 'https', publicKey: 'abc', - }); + })!; it('creates an envelope for a given Replay event', () => { const envelope = createReplayEnvelope(replayEvent, payloadWithSequence, dsn); diff --git a/packages/serverless/package.json b/packages/serverless/package.json index ec0623fa8016..92b9abc8ef28 100644 --- a/packages/serverless/package.json +++ b/packages/serverless/package.json @@ -34,8 +34,7 @@ "find-up": "^5.0.0", "google-gax": "^2.9.0", "nock": "^13.0.4", - "npm-packlist": "^2.1.4", - "read-pkg": "^5.2.0" + "npm-packlist": "^2.1.4" }, "scripts": { "build": "run-p build:transpile build:types build:bundle", diff --git a/packages/serverless/scripts/buildLambdaLayer.ts b/packages/serverless/scripts/buildLambdaLayer.ts index 459560a660fe..99140ccb0513 100644 --- a/packages/serverless/scripts/buildLambdaLayer.ts +++ b/packages/serverless/scripts/buildLambdaLayer.ts @@ -4,6 +4,7 @@ import * as fs from 'fs'; import * as rimraf from 'rimraf'; import { ensureBundleBuildPrereqs } from '../../../scripts/ensure-bundle-deps'; +import { version } from '../package.json'; /** * Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current @@ -46,6 +47,11 @@ async function buildLambdaLayer(): Promise { '../build/npm/cjs/awslambda-auto.js', 'build/aws/dist-serverless/nodejs/node_modules/@sentry/serverless/dist/awslambda-auto.js', ); + + const zipFilename = `sentry-node-serverless-${version}.zip`; + console.log(`Creating final layer zip file ${zipFilename}.`); + // need to preserve the symlink above with -y + run(`zip -r -y ${zipFilename} .`, { cwd: 'build/aws/dist-serverless' }); } void buildLambdaLayer(); diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 3f2883385422..888b57852327 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -3,14 +3,7 @@ import type { Scope } from '@sentry/node'; import * as Sentry from '@sentry/node'; import { captureException, captureMessage, flush, getCurrentHub, withScope } from '@sentry/node'; import type { Integration } from '@sentry/types'; -import { - baggageHeaderToDynamicSamplingContext, - dsnFromString, - dsnToString, - extractTraceparentData, - isString, - logger, -} from '@sentry/utils'; +import { baggageHeaderToDynamicSamplingContext, extractTraceparentData, isString, logger } from '@sentry/utils'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil // eslint-disable-next-line import/no-unresolved import type { Context, Handler } from 'aws-lambda'; @@ -56,27 +49,6 @@ export interface WrapperOptions { export const defaultIntegrations: Integration[] = [...Sentry.defaultIntegrations, new AWSServices({ optional: true })]; -/** - * Changes a Dsn to point to the `relay` server running in the Lambda Extension. - * - * This is only used by the serverless integration for AWS Lambda. - * - * @param originalDsn The original Dsn of the customer. - * @returns Dsn pointing to Lambda extension. - */ -function extensionRelayDSN(originalDsn: string | undefined): string | undefined { - if (originalDsn === undefined) { - return undefined; - } - - const dsn = dsnFromString(originalDsn); - dsn.host = 'localhost'; - dsn.port = '5333'; - dsn.protocol = 'http'; - - return dsnToString(dsn); -} - interface AWSLambdaOptions extends Sentry.NodeOptions { /** * Internal field that is set to `true` when init() is called by the Sentry AWS Lambda layer. @@ -106,12 +78,6 @@ export function init(options: AWSLambdaOptions = {}): void { version: Sentry.SDK_VERSION, }; - // If invoked by the Sentry Lambda Layer point the SDK to the Lambda Extension (inside the layer) instead of the host - // specified in the DSN - if (options._invokedByLambdaLayer) { - options.dsn = extensionRelayDSN(options.dsn); - } - Sentry.init(options); Sentry.addGlobalEventProcessor(serverlessEventProcessor); } diff --git a/packages/serverless/tsconfig.json b/packages/serverless/tsconfig.json index 05c709778aa0..a2731860dfa0 100644 --- a/packages/serverless/tsconfig.json +++ b/packages/serverless/tsconfig.json @@ -5,6 +5,7 @@ "compilerOptions": { // package-specific options - "target": "ES2018" + "target": "ES2018", + "resolveJsonModule": true } } diff --git a/packages/sveltekit/src/client/load.ts b/packages/sveltekit/src/client/load.ts index aa288b9e2ddd..1996523346e2 100644 --- a/packages/sveltekit/src/client/load.ts +++ b/packages/sveltekit/src/client/load.ts @@ -6,6 +6,7 @@ import { captureException } from '@sentry/svelte'; import type { ClientOptions, SanitizedRequestData } from '@sentry/types'; import { addExceptionMechanism, + addNonEnumerableProperty, getSanitizedUrlString, objectify, parseFetchArgs, @@ -14,8 +15,11 @@ import { } from '@sentry/utils'; import type { LoadEvent } from '@sveltejs/kit'; +import type { SentryWrappedFlag } from '../common/utils'; import { isRedirect } from '../common/utils'; +type PatchedLoadEvent = LoadEvent & Partial; + 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. @@ -66,13 +70,20 @@ export function wrapLoadWithSentry any>(origLoad: T) return new Proxy(origLoad, { apply: (wrappingTarget, thisArg, args: Parameters) => { // Type casting here because `T` cannot extend `Load` (see comment above function signature) - const event = args[0] as LoadEvent; + const event = args[0] as PatchedLoadEvent; - const patchedEvent = { + // Check if already wrapped + if (event.__sentry_wrapped__) { + return wrappingTarget.apply(thisArg, args); + } + + const patchedEvent: PatchedLoadEvent = { ...event, fetch: instrumentSvelteKitFetch(event.fetch), }; + addNonEnumerableProperty(patchedEvent as unknown as Record, '__sentry_wrapped__', true); + const routeId = event.route.id; return trace( { diff --git a/packages/sveltekit/src/common/utils.ts b/packages/sveltekit/src/common/utils.ts index 86035117b6f6..ecf762cee8c4 100644 --- a/packages/sveltekit/src/common/utils.ts +++ b/packages/sveltekit/src/common/utils.ts @@ -1,5 +1,13 @@ import type { Redirect } from '@sveltejs/kit'; +export type SentryWrappedFlag = { + /** + * If this flag is set, we know that the load event was already wrapped once + * and we shouldn't wrap it again. + */ + __sentry_wrapped__?: true; +}; + /** * Determines if a thrown "error" is a Redirect object which SvelteKit users can throw to redirect to another route * see: https://kit.svelte.dev/docs/modules#sveltejs-kit-redirect diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index a5350c6075c2..ab9f9ebdafb3 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -2,12 +2,16 @@ import { trace } from '@sentry/core'; import { captureException } from '@sentry/node'; import type { TransactionContext } from '@sentry/types'; -import { addExceptionMechanism, objectify } from '@sentry/utils'; +import { addExceptionMechanism, addNonEnumerableProperty, objectify } from '@sentry/utils'; import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; +import type { SentryWrappedFlag } from '../common/utils'; import { 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; } @@ -59,7 +63,15 @@ export function wrapLoadWithSentry any>(origLoad: T) return new Proxy(origLoad, { apply: (wrappingTarget, thisArg, args: Parameters) => { // Type casting here because `T` cannot extend `Load` (see comment above function signature) - const event = args[0] as LoadEvent; + // Also, this event possibly already has a sentry wrapped flag attached + const event = args[0] as PatchedLoadEvent; + + if (event.__sentry_wrapped__) { + return wrappingTarget.apply(thisArg, args); + } + + addNonEnumerableProperty(event as unknown as Record, '__sentry_wrapped__', true); + const routeId = event.route && event.route.id; const traceLoadContext: TransactionContext = { @@ -102,7 +114,15 @@ export function wrapServerLoadWithSentry any>(origSe return new Proxy(origServerLoad, { apply: (wrappingTarget, thisArg, args: Parameters) => { // Type casting here because `T` cannot extend `ServerLoad` (see comment above function signature) - const event = args[0] as ServerLoadEvent; + // Also, this event possibly already has a sentry wrapped flag attached + const event = args[0] as PatchedServerLoadEvent; + + if (event.__sentry_wrapped__) { + return wrappingTarget.apply(thisArg, args); + } + + addNonEnumerableProperty(event as unknown as Record, '__sentry_wrapped__', true); + const routeId = event.route && event.route.id; const { dynamicSamplingContext, traceparentData } = getTracePropagationData(event); diff --git a/packages/sveltekit/test/client/load.test.ts b/packages/sveltekit/test/client/load.test.ts index a6b5ebcbd278..b07e0c12108f 100644 --- a/packages/sveltekit/test/client/load.test.ts +++ b/packages/sveltekit/test/client/load.test.ts @@ -450,4 +450,17 @@ describe('wrapLoadWithSentry', () => { { handled: false, type: 'sveltekit', data: { function: 'load' } }, ); }); + + it("doesn't wrap load more than once if the wrapper was applied multiple times", async () => { + async function load({ params }: Parameters[0]): Promise> { + return { + post: params.id, + }; + } + + const wrappedLoad = wrapLoadWithSentry(wrapLoadWithSentry(load)); + await wrappedLoad(MOCK_LOAD_ARGS); + + expect(mockTrace).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index 4c57d6245451..fa57dc6b7c46 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -48,70 +48,80 @@ function getById(_id?: string) { throw new Error('error'); } -const MOCK_LOAD_ARGS: any = { - params: { id: '123' }, - route: { - id: '/users/[id]', - }, - url: new URL('http://localhost:3000/users/123'), -}; - -const MOCK_LOAD_NO_ROUTE_ARGS: any = { - params: { id: '123' }, - url: new URL('http://localhost:3000/users/123'), -}; - -const MOCK_SERVER_ONLY_LOAD_ARGS: any = { - ...MOCK_LOAD_ARGS, - request: { - method: 'GET', - headers: { - get: (key: string) => { - if (key === 'sentry-trace') { - return '1234567890abcdef1234567890abcdef-1234567890abcdef-1'; - } - - if (key === 'baggage') { - return ( - 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + - 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + - 'sentry-trace_id=1234567890abcdef1234567890abcdef,sentry-sample_rate=1' - ); - } - - return null; +function getLoadArgs() { + return { + params: { id: '123' }, + route: { + id: '/users/[id]', + }, + url: new URL('http://localhost:3000/users/123'), + }; +} + +function getLoadArgsWithoutRoute() { + return { + params: { id: '123' }, + url: new URL('http://localhost:3000/users/123'), + }; +} + +function getServerOnlyArgs() { + return { + ...getLoadArgs(), + request: { + method: 'GET', + headers: { + get: (key: string) => { + if (key === 'sentry-trace') { + return '1234567890abcdef1234567890abcdef-1234567890abcdef-1'; + } + + if (key === 'baggage') { + return ( + 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + + 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + + 'sentry-trace_id=1234567890abcdef1234567890abcdef,sentry-sample_rate=1' + ); + } + + return null; + }, }, }, - }, -}; - -const MOCK_SERVER_ONLY_NO_TRACE_LOAD_ARGS: any = { - ...MOCK_LOAD_ARGS, - request: { - method: 'GET', - headers: { - get: (_: string) => { - return null; + }; +} + +function getServerArgsWithoutTracingHeaders() { + return { + ...getLoadArgs(), + request: { + method: 'GET', + headers: { + get: (_: string) => { + return null; + }, }, }, - }, -}; - -const MOCK_SERVER_ONLY_NO_BAGGAGE_LOAD_ARGS: any = { - ...MOCK_LOAD_ARGS, - request: { - method: 'GET', - headers: { - get: (key: string) => { - if (key === 'sentry-trace') { - return '1234567890abcdef1234567890abcdef-1234567890abcdef-1'; - } - - return null; + }; +} + +function getServerArgsWithoutBaggageHeader() { + return { + ...getLoadArgs(), + request: { + method: 'GET', + headers: { + get: (key: string) => { + if (key === 'sentry-trace') { + return '1234567890abcdef1234567890abcdef-1234567890abcdef-1'; + } + + return null; + }, }, }, - }, -}; + }; +} beforeAll(() => { addTracingExtensions(); @@ -136,7 +146,7 @@ describe.each([ } const wrappedLoad = wrapLoadWithSentry(load); - const res = wrappedLoad(MOCK_LOAD_ARGS); + const res = wrappedLoad(getLoadArgs()); await expect(res).rejects.toThrow(); expect(mockCaptureException).toHaveBeenCalledTimes(1); @@ -162,7 +172,7 @@ describe.each([ } const wrappedLoad = wrapLoadWithSentry(load); - const res = wrappedLoad({ ...MOCK_LOAD_ARGS }); + const res = wrappedLoad(getLoadArgs()); await expect(res).rejects.toThrow(); expect(mockCaptureException).toHaveBeenCalledTimes(times); @@ -170,12 +180,12 @@ describe.each([ }); it("doesn't call captureException for thrown `Redirect`s", async () => { - async function load(_: Parameters[0]): Promise> { + async function load(_params: any): Promise> { throw redirect(300, 'other/route'); } const wrappedLoad = wrapLoadWithSentry(load); - const res = wrappedLoad(MOCK_LOAD_ARGS); + const res = wrappedLoad(getLoadArgs()); await expect(res).rejects.toThrow(); expect(mockCaptureException).not.toHaveBeenCalled(); @@ -194,7 +204,7 @@ describe.each([ } const wrappedLoad = sentryLoadWrapperFn.call(this, load); - const res = wrappedLoad(MOCK_SERVER_ONLY_LOAD_ARGS); + const res = wrappedLoad(getServerOnlyArgs()); await expect(res).rejects.toThrow(); expect(addEventProcessorSpy).toBeCalledTimes(1); @@ -206,7 +216,7 @@ describe.each([ }); }); describe('wrapLoadWithSentry calls trace', () => { - async function load({ params }: Parameters[0]): Promise> { + async function load({ params }): Promise> { return { post: params.id, }; @@ -214,7 +224,7 @@ describe('wrapLoadWithSentry calls trace', () => { it('with the context of the universal load function', async () => { const wrappedLoad = wrapLoadWithSentry(load); - await wrappedLoad(MOCK_LOAD_ARGS); + await wrappedLoad(getLoadArgs()); expect(mockTrace).toHaveBeenCalledTimes(1); expect(mockTrace).toHaveBeenCalledWith( @@ -233,7 +243,7 @@ describe('wrapLoadWithSentry calls trace', () => { it('falls back to the raw url if `event.route.id` is not available', async () => { const wrappedLoad = wrapLoadWithSentry(load); - await wrappedLoad(MOCK_LOAD_NO_ROUTE_ARGS); + await wrappedLoad(getLoadArgsWithoutRoute()); expect(mockTrace).toHaveBeenCalledTimes(1); expect(mockTrace).toHaveBeenCalledWith( @@ -249,10 +259,17 @@ describe('wrapLoadWithSentry calls trace', () => { expect.any(Function), ); }); + + it("doesn't wrap load more than once if the wrapper was applied multiple times", async () => { + const wrappedLoad = wrapLoadWithSentry(wrapLoadWithSentry(wrapLoadWithSentry(load))); + await wrappedLoad(getLoadArgs()); + + expect(mockTrace).toHaveBeenCalledTimes(1); + }); }); describe('wrapServerLoadWithSentry calls trace', () => { - async function serverLoad({ params }: Parameters[0]): Promise> { + async function serverLoad({ params }): Promise> { return { post: params.id, }; @@ -260,7 +277,7 @@ describe('wrapServerLoadWithSentry calls trace', () => { it('attaches trace data if available', async () => { const wrappedLoad = wrapServerLoadWithSentry(serverLoad); - await wrappedLoad(MOCK_SERVER_ONLY_LOAD_ARGS); + await wrappedLoad(getServerOnlyArgs()); expect(mockTrace).toHaveBeenCalledTimes(1); expect(mockTrace).toHaveBeenCalledWith( @@ -294,7 +311,7 @@ describe('wrapServerLoadWithSentry calls trace', () => { it("doesn't attach trace data if it's not available", async () => { const wrappedLoad = wrapServerLoadWithSentry(serverLoad); - await wrappedLoad(MOCK_SERVER_ONLY_NO_TRACE_LOAD_ARGS); + await wrappedLoad(getServerArgsWithoutTracingHeaders()); expect(mockTrace).toHaveBeenCalledTimes(1); expect(mockTrace).toHaveBeenCalledWith( @@ -316,7 +333,7 @@ describe('wrapServerLoadWithSentry calls trace', () => { it("doesn't attach the DSC data if the baggage header not available", async () => { const wrappedLoad = wrapServerLoadWithSentry(serverLoad); - await wrappedLoad(MOCK_SERVER_ONLY_NO_BAGGAGE_LOAD_ARGS); + await wrappedLoad(getServerArgsWithoutBaggageHeader()); expect(mockTrace).toHaveBeenCalledTimes(1); expect(mockTrace).toHaveBeenCalledWith( @@ -341,7 +358,8 @@ describe('wrapServerLoadWithSentry calls trace', () => { }); it('falls back to the raw url if `event.route.id` is not available', async () => { - const event = { ...MOCK_SERVER_ONLY_LOAD_ARGS }; + const event = getServerOnlyArgs(); + // @ts-ignore - this is fine (just tests here) delete event.route; const wrappedLoad = wrapServerLoadWithSentry(serverLoad); await wrappedLoad(event); @@ -375,4 +393,11 @@ describe('wrapServerLoadWithSentry calls trace', () => { expect.any(Function), ); }); + + it("doesn't wrap server load more than once if the wrapper was applied multiple times", async () => { + const wrappedLoad = wrapServerLoadWithSentry(wrapServerLoadWithSentry(serverLoad)); + await wrappedLoad(getServerOnlyArgs()); + + expect(mockTrace).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 94587c0380b1..284d8f339435 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -175,6 +175,15 @@ export function fetchCallback( // TODO (kmclb) remove this once types PR goes through // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access span.setHttpStatus(handlerData.response.status); + + const contentLength: string = + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); + + const contentLengthNum = parseInt(contentLength); + if (contentLengthNum > 0) { + span.setData('http.response_content_length', contentLengthNum); + } } else if (handlerData.error) { span.setStatus('internal_error'); } @@ -186,9 +195,6 @@ export function fetchCallback( return; } - const contentLength = - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length'); const currentScope = getCurrentHub().getScope(); const currentSpan = currentScope && currentScope.getSpan(); const activeTransaction = currentSpan && currentSpan.transaction; @@ -199,7 +205,6 @@ export function fetchCallback( data: { url, type: 'fetch', - ...(contentLength ? { 'http.response_content_length': contentLength } : {}), 'http.method': method, }, description: `${method} ${url}`, diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index c677818752cc..ee714e111a8b 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -214,9 +214,8 @@ describe('callbacks', () => { expect(newSpan).toBeUndefined(); }); - it('adds content-length to span data', () => { - const spans = {}; - fetchHandlerData['response'] = { headers: { 'content-length': 123 } }; + it('adds content-length to span data on finish', () => { + const spans: Record = {}; // triggered by request being sent fetchCallback(fetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); @@ -224,8 +223,21 @@ describe('callbacks', () => { const newSpan = transaction.spanRecorder?.spans[1] as Span; expect(newSpan).toBeDefined(); - expect(newSpan).toBeInstanceOf(Span); - expect(newSpan.data).toEqual({ + + const postRequestFetchHandlerData = { + ...fetchHandlerData, + endTimestamp, + response: { status: 404, headers: { get: () => 123 } }, + }; + + // triggered by response coming back + fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, alwaysAttachHeaders, spans); + + const finishedSpan = transaction.spanRecorder?.spans[1] as Span; + + expect(finishedSpan).toBeDefined(); + expect(finishedSpan).toBeInstanceOf(Span); + expect(finishedSpan.data).toEqual({ 'http.response_content_length': 123, 'http.method': 'GET', type: 'fetch', diff --git a/packages/tracing/README.md b/packages/tracing/README.md index 5ba5309906d3..462f2da52ff8 100644 --- a/packages/tracing/README.md +++ b/packages/tracing/README.md @@ -4,6 +4,8 @@

+> ⚠️ **Deprecation Notice:** From SDK versions >= `7.47.0` onwards, the `@sentry/tracing` package is officially deprecated. This package will be removed in a future major release. More details can be found in the [migration docs](https://github.com/getsentry/sentry-javascript/blob/7.47.0/MIGRATION.md/#remove-requirement-for-sentrytracing-package-since-7460). + # Sentry Tracing Extensions [![npm version](https://img.shields.io/npm/v/@sentry/tracing.svg)](https://www.npmjs.com/package/@sentry/tracing) diff --git a/packages/utils/src/dsn.ts b/packages/utils/src/dsn.ts index f506a8fcb9be..f149b67a9386 100644 --- a/packages/utils/src/dsn.ts +++ b/packages/utils/src/dsn.ts @@ -1,6 +1,6 @@ import type { DsnComponents, DsnLike, DsnProtocol } from '@sentry/types'; -import { SentryError } from './error'; +import { logger } from './logger'; /** Regular expression used to parse a Dsn. */ const DSN_REGEX = /^(?:(\w+):)\/\/(?:(\w+)(?::(\w+)?)?@)([\w.-]+)(?::(\d+))?\/(.+)/; @@ -30,13 +30,16 @@ export function dsnToString(dsn: DsnComponents, withPassword: boolean = false): * Parses a Dsn from a given string. * * @param str A Dsn as string - * @returns Dsn as DsnComponents + * @returns Dsn as DsnComponents or undefined if @param str is not a valid DSN string */ -export function dsnFromString(str: string): DsnComponents { +export function dsnFromString(str: string): DsnComponents | undefined { const match = DSN_REGEX.exec(str); if (!match) { - throw new SentryError(`Invalid Sentry Dsn: ${str}`); + // This should be logged to the console + // eslint-disable-next-line no-console + console.error(`Invalid Sentry Dsn: ${str}`); + return undefined; } const [protocol, publicKey, pass = '', host, port = '', lastPath] = match.slice(1); @@ -71,38 +74,52 @@ function dsnFromComponents(components: DsnComponents): DsnComponents { }; } -function validateDsn(dsn: DsnComponents): boolean | void { +function validateDsn(dsn: DsnComponents): boolean { if (!__DEBUG_BUILD__) { - return; + return true; } const { port, projectId, protocol } = dsn; const requiredComponents: ReadonlyArray = ['protocol', 'publicKey', 'host', 'projectId']; - requiredComponents.forEach(component => { + const hasMissingRequiredComponent = requiredComponents.find(component => { if (!dsn[component]) { - throw new SentryError(`Invalid Sentry Dsn: ${component} missing`); + logger.error(`Invalid Sentry Dsn: ${component} missing`); + return true; } + return false; }); + if (hasMissingRequiredComponent) { + return false; + } + if (!projectId.match(/^\d+$/)) { - throw new SentryError(`Invalid Sentry Dsn: Invalid projectId ${projectId}`); + logger.error(`Invalid Sentry Dsn: Invalid projectId ${projectId}`); + return false; } if (!isValidProtocol(protocol)) { - throw new SentryError(`Invalid Sentry Dsn: Invalid protocol ${protocol}`); + logger.error(`Invalid Sentry Dsn: Invalid protocol ${protocol}`); + return false; } if (port && isNaN(parseInt(port, 10))) { - throw new SentryError(`Invalid Sentry Dsn: Invalid port ${port}`); + logger.error(`Invalid Sentry Dsn: Invalid port ${port}`); + return false; } return true; } -/** The Sentry Dsn, identifying a Sentry instance and project. */ -export function makeDsn(from: DsnLike): DsnComponents { +/** + * Creates a valid Sentry Dsn object, identifying a Sentry instance and project. + * @returns a valid DsnComponents object or `undefined` if @param from is an invalid DSN source + */ +export function makeDsn(from: DsnLike): DsnComponents | undefined { const components = typeof from === 'string' ? dsnFromString(from) : dsnFromComponents(from); - validateDsn(components); + if (!components || !validateDsn(components)) { + return undefined; + } return components; } diff --git a/packages/utils/test/dsn.test.ts b/packages/utils/test/dsn.test.ts index 3a5bb3e7da6c..3de435bf5fcf 100644 --- a/packages/utils/test/dsn.test.ts +++ b/packages/utils/test/dsn.test.ts @@ -1,11 +1,18 @@ import { dsnToString, makeDsn } from '../src/dsn'; -import { SentryError } from '../src/error'; +import { logger } from '../src/logger'; function testIf(condition: boolean): jest.It { return condition ? test : test.skip; } +const loggerErrorSpy = jest.spyOn(logger, 'error').mockImplementation(() => {}); +const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + describe('Dsn', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + describe('fromComponents', () => { test('applies all components', () => { const dsn = makeDsn({ @@ -16,13 +23,13 @@ describe('Dsn', () => { protocol: 'https', publicKey: 'abc', }); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe('xyz'); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe('1234'); - expect(dsn.path).toBe(''); - expect(dsn.projectId).toBe('123'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe('xyz'); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe('1234'); + expect(dsn?.path).toBe(''); + expect(dsn?.projectId).toBe('123'); }); test('applies partial components', () => { @@ -32,60 +39,62 @@ describe('Dsn', () => { protocol: 'https', publicKey: 'abc', }); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe(''); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe(''); - expect(dsn.path).toBe(''); - expect(dsn.projectId).toBe('123'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe(''); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe(''); + expect(dsn?.path).toBe(''); + expect(dsn?.projectId).toBe('123'); }); - testIf(__DEBUG_BUILD__)('throws for missing components', () => { - expect(() => + testIf(__DEBUG_BUILD__)('returns `undefined` for missing components', () => { + expect( makeDsn({ host: '', projectId: '123', protocol: 'https', publicKey: 'abc', }), - ).toThrow(SentryError); - expect(() => + ).toBeUndefined(); + expect( makeDsn({ host: 'sentry.io', projectId: '', protocol: 'https', publicKey: 'abc', }), - ).toThrow(SentryError); - expect(() => + ).toBeUndefined(); + expect( makeDsn({ host: 'sentry.io', projectId: '123', protocol: '' as 'http', // Trick the type checker here publicKey: 'abc', }), - ).toThrow(SentryError); - expect(() => + ).toBeUndefined(); + expect( makeDsn({ host: 'sentry.io', projectId: '123', protocol: 'https', publicKey: '', }), - ).toThrow(SentryError); + ).toBeUndefined(); + + expect(loggerErrorSpy).toHaveBeenCalledTimes(4); }); - testIf(__DEBUG_BUILD__)('throws for invalid components', () => { - expect(() => + testIf(__DEBUG_BUILD__)('returns `undefined` if components are invalid', () => { + expect( makeDsn({ host: 'sentry.io', projectId: '123', protocol: 'httpx' as 'http', // Trick the type checker here publicKey: 'abc', }), - ).toThrow(SentryError); - expect(() => + ).toBeUndefined(); + expect( makeDsn({ host: 'sentry.io', port: 'xxx', @@ -93,108 +102,114 @@ describe('Dsn', () => { protocol: 'https', publicKey: 'abc', }), - ).toThrow(SentryError); + ).toBeUndefined(); + + expect(loggerErrorSpy).toHaveBeenCalledTimes(2); }); }); describe('fromString', () => { test('parses a valid full Dsn', () => { const dsn = makeDsn('https://abc:xyz@sentry.io:1234/123'); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe('xyz'); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe('1234'); - expect(dsn.path).toBe(''); - expect(dsn.projectId).toBe('123'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe('xyz'); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe('1234'); + expect(dsn?.path).toBe(''); + expect(dsn?.projectId).toBe('123'); }); test('parses a valid partial Dsn', () => { const dsn = makeDsn('https://abc@sentry.io/123/321'); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe(''); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe(''); - expect(dsn.path).toBe('123'); - expect(dsn.projectId).toBe('321'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe(''); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe(''); + expect(dsn?.path).toBe('123'); + expect(dsn?.projectId).toBe('321'); }); test('parses a Dsn with empty password', () => { const dsn = makeDsn('https://abc:@sentry.io/123/321'); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe(''); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe(''); - expect(dsn.path).toBe('123'); - expect(dsn.projectId).toBe('321'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe(''); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe(''); + expect(dsn?.path).toBe('123'); + expect(dsn?.projectId).toBe('321'); }); test('with a long path', () => { const dsn = makeDsn('https://abc@sentry.io/sentry/custom/installation/321'); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe(''); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe(''); - expect(dsn.path).toBe('sentry/custom/installation'); - expect(dsn.projectId).toBe('321'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe(''); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe(''); + expect(dsn?.path).toBe('sentry/custom/installation'); + expect(dsn?.projectId).toBe('321'); }); test('with a query string', () => { const dsn = makeDsn('https://abc@sentry.io/321?sample.rate=0.1&other=value'); - expect(dsn.protocol).toBe('https'); - expect(dsn.publicKey).toBe('abc'); - expect(dsn.pass).toBe(''); - expect(dsn.host).toBe('sentry.io'); - expect(dsn.port).toBe(''); - expect(dsn.path).toBe(''); - expect(dsn.projectId).toBe('321'); + expect(dsn?.protocol).toBe('https'); + expect(dsn?.publicKey).toBe('abc'); + expect(dsn?.pass).toBe(''); + expect(dsn?.host).toBe('sentry.io'); + expect(dsn?.port).toBe(''); + expect(dsn?.path).toBe(''); + expect(dsn?.projectId).toBe('321'); }); - testIf(__DEBUG_BUILD__)('throws when provided invalid Dsn', () => { - expect(() => makeDsn('some@random.dsn')).toThrow(SentryError); + testIf(__DEBUG_BUILD__)('returns undefined when provided invalid Dsn', () => { + expect(makeDsn('some@random.dsn')).toBeUndefined(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); }); - testIf(__DEBUG_BUILD__)('throws without mandatory fields', () => { - expect(() => makeDsn('://abc@sentry.io/123')).toThrow(SentryError); - expect(() => makeDsn('https://@sentry.io/123')).toThrow(SentryError); - expect(() => makeDsn('https://abc@123')).toThrow(SentryError); - expect(() => makeDsn('https://abc@sentry.io/')).toThrow(SentryError); + testIf(__DEBUG_BUILD__)('returns undefined if mandatory fields are missing', () => { + expect(makeDsn('://abc@sentry.io/123')).toBeUndefined(); + expect(makeDsn('https://@sentry.io/123')).toBeUndefined(); + expect(makeDsn('https://abc@123')).toBeUndefined(); + expect(makeDsn('https://abc@sentry.io/')).toBeUndefined(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(4); }); - testIf(__DEBUG_BUILD__)('throws for invalid fields', () => { - expect(() => makeDsn('httpx://abc@sentry.io/123')).toThrow(SentryError); - expect(() => makeDsn('httpx://abc@sentry.io:xxx/123')).toThrow(SentryError); - expect(() => makeDsn('http://abc@sentry.io/abc')).toThrow(SentryError); + testIf(__DEBUG_BUILD__)('returns undefined if fields are invalid', () => { + expect(makeDsn('httpx://abc@sentry.io/123')).toBeUndefined(); + expect(makeDsn('httpx://abc@sentry.io:xxx/123')).toBeUndefined(); + expect(makeDsn('http://abc@sentry.io/abc')).toBeUndefined(); + expect(loggerErrorSpy).toHaveBeenCalledTimes(2); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); }); }); describe('toString', () => { test('excludes the password by default', () => { const dsn = makeDsn('https://abc:xyz@sentry.io:1234/123'); - expect(dsnToString(dsn)).toBe('https://abc@sentry.io:1234/123'); + expect(dsnToString(dsn!)).toBe('https://abc@sentry.io:1234/123'); }); test('optionally includes the password', () => { const dsn = makeDsn('https://abc:xyz@sentry.io:1234/123'); - expect(dsnToString(dsn, true)).toBe('https://abc:xyz@sentry.io:1234/123'); + expect(dsnToString(dsn!, true)).toBe('https://abc:xyz@sentry.io:1234/123'); }); test('renders no password if missing', () => { const dsn = makeDsn('https://abc@sentry.io:1234/123'); - expect(dsnToString(dsn, true)).toBe('https://abc@sentry.io:1234/123'); + expect(dsnToString(dsn!, true)).toBe('https://abc@sentry.io:1234/123'); }); test('renders no port if missing', () => { const dsn = makeDsn('https://abc@sentry.io/123'); - expect(dsnToString(dsn)).toBe('https://abc@sentry.io/123'); + expect(dsnToString(dsn!)).toBe('https://abc@sentry.io/123'); }); test('renders the full path correctly', () => { const dsn = makeDsn('https://abc@sentry.io/sentry/custom/installation/321'); - expect(dsnToString(dsn)).toBe('https://abc@sentry.io/sentry/custom/installation/321'); + expect(dsnToString(dsn!)).toBe('https://abc@sentry.io/sentry/custom/installation/321'); }); }); }); diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index 2996b6dcde06..9649da0e2108 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -36,7 +36,7 @@ describe('envelope', () => { expect(headers).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }); }); - it.only('serializes an envelope with attachments', () => { + it('serializes an envelope with attachments', () => { const items: EventEnvelope[1] = [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], [{ type: 'attachment', filename: 'bar.txt', length: 6 }, Uint8Array.from([1, 2, 3, 4, 5, 6])], diff --git a/packages/vue/package.json b/packages/vue/package.json index 9f630d742200..56b2e07b9d33 100644 --- a/packages/vue/package.json +++ b/packages/vue/package.json @@ -30,12 +30,10 @@ }, "scripts": { "build": "run-p build:transpile build:types", - "build:bundle": "rollup --config rollup.bundle.config.js", "build:dev": "run-p build:transpile build:types", "build:transpile": "rollup -c rollup.npm.config.js", "build:types": "tsc -p tsconfig.types.json", "build:watch": "run-p build:transpile:watch build:types:watch", - "build:bundle:watch": "rollup --config --watch", "build:dev:watch": "run-p build:transpile:watch build:types:watch", "build:transpile:watch": "rollup -c rollup.npm.config.js --watch", "build:types:watch": "tsc -p tsconfig.types.json --watch", diff --git a/packages/vue/rollup.bundle.config.js b/packages/vue/rollup.bundle.config.js deleted file mode 100644 index 4df9b0d5b614..000000000000 --- a/packages/vue/rollup.bundle.config.js +++ /dev/null @@ -1,11 +0,0 @@ -import { makeBaseBundleConfig, makeBundleConfigVariants } from '../../rollup/index.js'; - -const baseBundleConfig = makeBaseBundleConfig({ - bundleType: 'standalone', - entrypoints: ['src/index.bundle.ts'], - jsVersion: 'es6', - licenseTitle: '@sentry/vue', - outputFileBase: () => 'bundle.vue', -}); - -export default makeBundleConfigVariants(baseBundleConfig); diff --git a/scripts/aws-delete-local-layers.sh b/scripts/aws-delete-local-layers.sh new file mode 100755 index 000000000000..5fa14fd57cba --- /dev/null +++ b/scripts/aws-delete-local-layers.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# +# Deletes all versions of the layer specified in LAYER_NAME in one region. +# + +set -euo pipefail + +# override default AWS region +export AWS_REGION=eu-central-1 + +LAYER_NAME=SentryNodeServerlessSDK-local-dev +VERSION="0" + +while [[ $VERSION != "1" ]] +do + VERSION=$(aws lambda list-layer-versions --layer-name $LAYER_NAME | jq '.LayerVersions[0].Version') + aws lambda delete-layer-version --layer-name $LAYER_NAME --version-number $VERSION +done diff --git a/scripts/aws-deploy-local-layer.sh b/scripts/aws-deploy-local-layer.sh index a979ddbbdcbe..db5398adbc63 100755 --- a/scripts/aws-deploy-local-layer.sh +++ b/scripts/aws-deploy-local-layer.sh @@ -16,74 +16,23 @@ rm -rf dist-serverless/ rm -rf ./packages/serverless/build rm -rf ./packages/serverless/dist rm -rf ./packages/serverless/node_modules -rm -f sentry-node-serverless-*.zip # Creating Lambda layer echo "Creating Lambda layer in ./packages/serverless/build/aws/dist-serverless..." cd packages/serverless yarn build -cd ../../ echo "Done creating Lambda layer in ./packages/serverless/build/aws/dist-serverless." -# Move dist-serverless/ to the root folder for the action to pick it up. -# This is only needed in this script, because in GitHub workflow -# this is done with the upload-artifact/download-artifact actions -echo "Copying Lambda layer in ./packages/serverless/build/aws/dist-serverless to working directory..." -mv ./packages/serverless/build/aws/dist-serverless . -echo "Done copying Lambda layer in ./packages/serverless/build/aws/dist-serverless to working directory." - -# IMPORTANT: -# Please make sure that this does the same as the GitHub action that -# is building the Lambda layer in production! -# see: https://github.com/getsentry/action-build-aws-lambda-extension/blob/main/action.yml#L23-L40 - -echo "Downloading relay..." -# Make directory (if not existing) -mkdir -p dist-serverless/relay -# Download releay from release registry to dist-serverless/relay/relay -curl -0 --silent \ - --output dist-serverless/relay/relay \ - "$(curl -s https://release-registry.services.sentry.io/apps/relay/latest | jq -r .files.\"relay-Linux-x86_64\".url)" -# Make file executable -chmod +x dist-serverless/relay/relay -echo "Done downloading relay." - -echo "Creating start script..." -# Make directory (if not existing) -mkdir -p dist-serverless/extensions -# Create 'sentry-lambda-extension' script that starts relay. -# The file has to have exactly this name, because the executable files of -# Lambda extensions need to have same file name as the name that they use -# to register with AWS Lambda environment -cat > dist-serverless/extensions/sentry-lambda-extension << EOT -#!/bin/bash -set -euo pipefail -exec /opt/relay/relay run \ - --mode=proxy \ - --shutdown-timeout=2 \ - --upstream-dsn="\$SENTRY_DSN" \ - --aws-runtime-api="\$AWS_LAMBDA_RUNTIME_API" -EOT -# Make script executable -chmod +x dist-serverless/extensions/sentry-lambda-extension -echo "Done creating start script." - -# Zip Lambda layer and included Lambda extension -echo "Zipping Lambda layer and included Lambda extension..." -cd dist-serverless/ -zip -r -y ../sentry-node-serverless-x.x.x-dev.zip . -cd .. -echo "Done Zipping Lambda layer and included Lambda extension to ./sentry-node-serverless-x.x.x-dev.zip." - # Deploying zipped Lambda layer to AWS -echo "Deploying zipped Lambda layer to AWS..." +ZIP=$(ls build/aws/dist-serverless | grep sentry-node-serverless | head -n 1) +echo "Deploying zipped Lambda layer $ZIP to AWS..." aws lambda publish-layer-version \ --layer-name "SentryNodeServerlessSDK-local-dev" \ --region "eu-central-1" \ - --zip-file "fileb://sentry-node-serverless-x.x.x-dev.zip" \ + --zip-file "fileb://build/aws/dist-serverless/$ZIP" \ --description "Local test build of SentryNodeServerlessSDK (can be deleted)" \ - --no-cli-pager + --compatible-runtimes nodejs10.x nodejs12.x nodejs14.x nodejs16.x nodejs18.x echo "Done deploying zipped Lambda layer to AWS as 'SentryNodeServerlessSDK-local-dev'."