From 7f571dc079f64f2d50fc1f96d572edb944be8c0e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Sep 2023 14:45:04 +0200 Subject: [PATCH] feat(integrations): Introduce `processEvent` hook on `Integration` This adds a new (optional) `processEvent` hook on the `Integration` interface, which allows to register an event processor **for the current client only**. This has actually correct semantics in that the processor will only be registered for the client that the integration is added for. This is done by adding a new `addEventProcessor` method on the baseclient, which for now are called after all global & scope event processors. --- packages/browser/src/integrations/dedupe.ts | 46 +++--- .../browser/src/integrations/httpcontext.ts | 45 +++--- packages/browser/src/profiling/integration.ts | 2 +- packages/core/src/baseclient.ts | 69 ++++---- packages/core/src/eventProcessors.ts | 51 ++++++ packages/core/src/index.ts | 3 +- packages/core/src/integration.ts | 16 +- .../core/src/integrations/inboundfilters.ts | 26 ++- packages/core/src/scope.ts | 63 +------- .../lib/integrations/inboundfilters.test.ts | 43 +++-- packages/integrations/src/contextlines.ts | 23 +-- packages/integrations/src/dedupe.ts | 46 +++--- packages/integrations/src/extraerrordata.ts | 149 +++++++++--------- packages/integrations/src/rewriteframes.ts | 21 +-- packages/integrations/src/sessiontiming.ts | 19 ++- packages/integrations/src/transaction.ts | 39 +++-- packages/integrations/test/dedupe.test.ts | 52 ++---- packages/node/src/integrations/context.ts | 50 +++--- .../node/src/integrations/contextlines.ts | 29 ++-- .../node/src/integrations/localvariables.ts | 52 ++++-- packages/node/src/integrations/modules.ts | 40 ++--- packages/node/src/integrations/requestdata.ts | 101 ++++++------ .../test/integrations/requestdata.test.ts | 47 +++--- packages/types/src/client.ts | 8 + packages/types/src/integration.ts | 6 + packages/wasm/src/index.ts | 35 ++-- 26 files changed, 560 insertions(+), 521 deletions(-) create mode 100644 packages/core/src/eventProcessors.ts diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts index 673ded6484cb..e1eb0cd18bcb 100644 --- a/packages/browser/src/integrations/dedupe.ts +++ b/packages/browser/src/integrations/dedupe.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Event, Exception, Integration, StackFrame } from '@sentry/types'; import { logger } from '@sentry/utils'; /** Deduplication filter */ @@ -22,36 +22,32 @@ export class Dedupe implements Integration { this.name = Dedupe.id; } + /** @inheritDoc */ + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - const eventProcessor: EventProcessor = currentEvent => { - // We want to ignore any non-error type events, e.g. transactions or replays - // These should never be deduped, and also not be compared against as _previousEvent. - if (currentEvent.type) { - return currentEvent; - } + public processEvent(currentEvent: Event): Event | null { + // We want to ignore any non-error type events, e.g. transactions or replays + // These should never be deduped, and also not be compared against as _previousEvent. + if (currentEvent.type) { + return currentEvent; + } - const self = getCurrentHub().getIntegration(Dedupe); - if (self) { - // Juuust in case something goes wrong - try { - if (_shouldDropEvent(currentEvent, self._previousEvent)) { - __DEBUG_BUILD__ && logger.warn('Event dropped due to being a duplicate of previously captured event.'); - return null; - } - } catch (_oO) { - return (self._previousEvent = currentEvent); - } - - return (self._previousEvent = currentEvent); + // Juuust in case something goes wrong + try { + if (_shouldDropEvent(currentEvent, this._previousEvent)) { + __DEBUG_BUILD__ && logger.warn('Event dropped due to being a duplicate of previously captured event.'); + return null; } - return currentEvent; - }; + } catch (_oO) { + return (this._previousEvent = currentEvent); + } - eventProcessor.id = this.name; - addGlobalEventProcessor(eventProcessor); + return (this._previousEvent = currentEvent); } } diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts index f37963b46619..a2bdab99377f 100644 --- a/packages/browser/src/integrations/httpcontext.ts +++ b/packages/browser/src/integrations/httpcontext.ts @@ -1,4 +1,3 @@ -import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import type { Event, Integration } from '@sentry/types'; import { WINDOW } from '../helpers'; @@ -23,28 +22,28 @@ export class HttpContext implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor((event: Event) => { - if (getCurrentHub().getIntegration(HttpContext)) { - // if none of the information we want exists, don't bother - if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) { - return event; - } - - // grab as much info as exists and add it to the event - const url = (event.request && event.request.url) || (WINDOW.location && WINDOW.location.href); - const { referrer } = WINDOW.document || {}; - const { userAgent } = WINDOW.navigator || {}; - - const headers = { - ...(event.request && event.request.headers), - ...(referrer && { Referer: referrer }), - ...(userAgent && { 'User-Agent': userAgent }), - }; - const request = { ...event.request, ...(url && { url }), headers }; - - return { ...event, request }; - } + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Event { + // if none of the information we want exists, don't bother + if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) { return event; - }); + } + + // grab as much info as exists and add it to the event + const url = (event.request && event.request.url) || (WINDOW.location && WINDOW.location.href); + const { referrer } = WINDOW.document || {}; + const { userAgent } = WINDOW.navigator || {}; + + const headers = { + ...(event.request && event.request.headers), + ...(referrer && { Referer: referrer }), + ...(userAgent && { 'User-Agent': userAgent }), + }; + const request = { ...event.request, ...(url && { url }), headers }; + + return { ...event, request }; } } diff --git a/packages/browser/src/profiling/integration.ts b/packages/browser/src/profiling/integration.ts index 0d506b7493cd..dc7cb4d15120 100644 --- a/packages/browser/src/profiling/integration.ts +++ b/packages/browser/src/profiling/integration.ts @@ -35,7 +35,7 @@ export class BrowserProfilingIntegration implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { this.getCurrentHub = getCurrentHub; const client = this.getCurrentHub().getClient() as BrowserClient; diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 1d1ad6aaa377..e20ea7a76bbb 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -12,6 +12,7 @@ import type { Event, EventDropReason, EventHint, + EventProcessor, Integration, IntegrationClass, Outcome, @@ -43,6 +44,7 @@ import { import { getEnvelopeEndpointWithUrlEncodedAuth } from './api'; import { createEventEnvelope, createSessionEnvelope } from './envelope'; +import { notifyEventProcessors } from './eventProcessors'; import type { IntegrationIndex } from './integration'; import { setupIntegration, setupIntegrations } from './integration'; import type { Scope } from './scope'; @@ -107,6 +109,8 @@ export abstract class BaseClient implements Client { // eslint-disable-next-line @typescript-eslint/ban-types private _hooks: Record; + private _eventProcessors: EventProcessor[]; + /** * Initializes this client instance. * @@ -119,6 +123,7 @@ export abstract class BaseClient implements Client { this._numProcessing = 0; this._outcomes = {}; this._hooks = {}; + this._eventProcessors = []; if (options.dsn) { this._dsn = makeDsn(options.dsn); @@ -280,6 +285,11 @@ export abstract class BaseClient implements Client { }); } + /** @inheritDoc */ + public addEventProcessor(eventProcessor: EventProcessor): void { + this._eventProcessors.push(eventProcessor); + } + /** * Sets up the integrations */ @@ -545,36 +555,41 @@ export abstract class BaseClient implements Client { this.emit('preprocessEvent', event, hint); - return prepareEvent(options, event, hint, scope).then(evt => { - if (evt === null) { - return evt; - } + return prepareEvent(options, event, hint, scope) + .then(evt => { + // Process client-scoped event processors + return notifyEventProcessors(this._eventProcessors, evt, hint); + }) + .then(evt => { + if (evt === null) { + return evt; + } - // If a trace context is not set on the event, we use the propagationContext set on the event to - // generate a trace context. If the propagationContext does not have a dynamic sampling context, we - // also generate one for it. - const { propagationContext } = evt.sdkProcessingMetadata || {}; - const trace = evt.contexts && evt.contexts.trace; - if (!trace && propagationContext) { - const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext as PropagationContext; - evt.contexts = { - trace: { - trace_id, - span_id: spanId, - parent_span_id: parentSpanId, - }, - ...evt.contexts, - }; + // If a trace context is not set on the event, we use the propagationContext set on the event to + // generate a trace context. If the propagationContext does not have a dynamic sampling context, we + // also generate one for it. + const { propagationContext } = evt.sdkProcessingMetadata || {}; + const trace = evt.contexts && evt.contexts.trace; + if (!trace && propagationContext) { + const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext as PropagationContext; + evt.contexts = { + trace: { + trace_id, + span_id: spanId, + parent_span_id: parentSpanId, + }, + ...evt.contexts, + }; - const dynamicSamplingContext = dsc ? dsc : getDynamicSamplingContextFromClient(trace_id, this, scope); + const dynamicSamplingContext = dsc ? dsc : getDynamicSamplingContextFromClient(trace_id, this, scope); - evt.sdkProcessingMetadata = { - dynamicSamplingContext, - ...evt.sdkProcessingMetadata, - }; - } - return evt; - }); + evt.sdkProcessingMetadata = { + dynamicSamplingContext, + ...evt.sdkProcessingMetadata, + }; + } + return evt; + }); } /** diff --git a/packages/core/src/eventProcessors.ts b/packages/core/src/eventProcessors.ts new file mode 100644 index 000000000000..4596788b9dcb --- /dev/null +++ b/packages/core/src/eventProcessors.ts @@ -0,0 +1,51 @@ +import type { Event, EventHint, EventProcessor } from '@sentry/types'; +import { getGlobalSingleton, isThenable, logger, SyncPromise } from '@sentry/utils'; + +/** + * Returns the global event processors. + */ +export function getGlobalEventProcessors(): EventProcessor[] { + return getGlobalSingleton('globalEventProcessors', () => []); +} + +/** + * Add a EventProcessor to be kept globally. + * @param callback EventProcessor to add + */ +export function addGlobalEventProcessor(callback: EventProcessor): void { + getGlobalEventProcessors().push(callback); +} + +/** + * Process an array of event processors, returning the processed event (or `null` if the event was dropped). + */ +export function notifyEventProcessors( + processors: EventProcessor[], + event: Event | null, + hint: EventHint, + index: number = 0, +): PromiseLike { + return new SyncPromise((resolve, reject) => { + const processor = processors[index]; + if (event === null || typeof processor !== 'function') { + resolve(event); + } else { + const result = processor({ ...event }, hint) as Event | null; + + __DEBUG_BUILD__ && + processor.id && + result === null && + logger.log(`Event processor "${processor.id}" dropped event`); + + if (isThenable(result)) { + void result + .then(final => notifyEventProcessors(processors, final, hint, index + 1).then(resolve)) + .then(null, reject); + } else { + void notifyEventProcessors(processors, result, hint, index + 1) + .then(resolve) + .then(null, reject); + } + } + }); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 67c28a3e3c57..21f7cab37505 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -36,7 +36,8 @@ export { } from './hub'; export { makeSession, closeSession, updateSession } from './session'; export { SessionFlusher } from './sessionflusher'; -export { addGlobalEventProcessor, Scope } from './scope'; +export { Scope } from './scope'; +export { addGlobalEventProcessor } from './eventProcessors'; export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from './api'; export { BaseClient } from './baseclient'; export { ServerRuntimeClient } from './server-runtime-client'; diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index b2c8e4547ab8..aa8968edc8dc 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -1,8 +1,8 @@ -import type { Client, Integration, Options } from '@sentry/types'; +import type { Client, Event, EventHint, Integration, Options } from '@sentry/types'; import { arrayify, logger } from '@sentry/utils'; +import { addGlobalEventProcessor } from './eventProcessors'; import { getCurrentHub } from './hub'; -import { addGlobalEventProcessor } from './scope'; declare module '@sentry/types' { interface Integration { @@ -107,10 +107,20 @@ export function setupIntegration(client: Client, integration: Integration, integ } if (client.on && typeof integration.preprocessEvent === 'function') { - const callback = integration.preprocessEvent.bind(integration); + const callback = integration.preprocessEvent.bind(integration) as typeof integration.preprocessEvent; client.on('preprocessEvent', (event, hint) => callback(event, hint, client)); } + if (client.addEventProcessor && typeof integration.processEvent === 'function') { + const callback = integration.processEvent.bind(integration) as typeof integration.processEvent; + + const processor = Object.assign((event: Event, hint: EventHint) => callback(event, hint, client), { + id: integration.name, + }); + + client.addEventProcessor(processor); + } + __DEBUG_BUILD__ && logger.log(`Integration installed: ${integration.name}`); } diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 37ea8bac06e9..9c6e8c9a546a 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Client, Event, EventHint, Integration, StackFrame } from '@sentry/types'; import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/utils'; // "Script error." is hard coded into browsers for errors that it can't read. @@ -48,23 +48,15 @@ export class InboundFilters implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (processor: EventProcessor) => void, getCurrentHub: () => Hub): void { - const eventProcess: EventProcessor = (event: Event) => { - const hub = getCurrentHub(); - if (hub) { - const self = hub.getIntegration(InboundFilters); - if (self) { - const client = hub.getClient(); - const clientOptions = client ? client.getOptions() : {}; - const options = _mergeOptions(self._options, clientOptions); - return _shouldDropEvent(event, options) ? null : event; - } - } - return event; - }; + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } - eventProcess.id = this.name; - addGlobalEventProcessor(eventProcess); + /** @inheritDoc */ + public processEvent(event: Event, _eventHint: EventHint, client: Client): Event | null { + const clientOptions = client.getOptions(); + const options = _mergeOptions(this._options, clientOptions); + return _shouldDropEvent(event, options) ? null : event; } } diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index ef6832bc773d..ba4ccac23adb 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -22,17 +22,9 @@ import type { Transaction, User, } from '@sentry/types'; -import { - arrayify, - dateTimestampInSeconds, - getGlobalSingleton, - isPlainObject, - isThenable, - logger, - SyncPromise, - uuid4, -} from '@sentry/utils'; +import { arrayify, dateTimestampInSeconds, isPlainObject, uuid4 } from '@sentry/utils'; +import { getGlobalEventProcessors, notifyEventProcessors } from './eventProcessors'; import { updateSession } from './session'; /** @@ -525,7 +517,7 @@ export class Scope implements ScopeInterface { propagationContext: this._propagationContext, }; - return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint); + return notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint); } /** @@ -559,40 +551,6 @@ export class Scope implements ScopeInterface { return this._breadcrumbs; } - /** - * This will be called after {@link applyToEvent} is finished. - */ - protected _notifyEventProcessors( - processors: EventProcessor[], - event: Event | null, - hint: EventHint, - index: number = 0, - ): PromiseLike { - return new SyncPromise((resolve, reject) => { - const processor = processors[index]; - if (event === null || typeof processor !== 'function') { - resolve(event); - } else { - const result = processor({ ...event }, hint) as Event | null; - - __DEBUG_BUILD__ && - processor.id && - result === null && - logger.log(`Event processor "${processor.id}" dropped event`); - - if (isThenable(result)) { - void result - .then(final => this._notifyEventProcessors(processors, final, hint, index + 1).then(resolve)) - .then(null, reject); - } else { - void this._notifyEventProcessors(processors, result, hint, index + 1) - .then(resolve) - .then(null, reject); - } - } - }); - } - /** * This will be called on every set call. */ @@ -629,21 +587,6 @@ export class Scope implements ScopeInterface { } } -/** - * Returns the global event processors. - */ -function getGlobalEventProcessors(): EventProcessor[] { - return getGlobalSingleton('globalEventProcessors', () => []); -} - -/** - * Add a EventProcessor to be kept globally. - * @param callback EventProcessor to add - */ -export function addGlobalEventProcessor(callback: EventProcessor): void { - getGlobalEventProcessors().push(callback); -} - function generatePropagationContext(): PropagationContext { return { traceId: uuid4(), diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index 9888a59eedff..8e42c5bc29ee 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -2,6 +2,9 @@ import type { Event, EventProcessor } from '@sentry/types'; import type { InboundFiltersOptions } from '../../../src/integrations/inboundfilters'; import { InboundFilters } from '../../../src/integrations/inboundfilters'; +import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; + +const PUBLIC_DSN = 'https://username@domain/123'; /** * Creates an instance of the InboundFilters integration and returns @@ -25,30 +28,22 @@ function createInboundFiltersEventProcessor( options: Partial = {}, clientOptions: Partial = {}, ): EventProcessor { - const eventProcessors: EventProcessor[] = []; - const inboundFiltersInstance = new InboundFilters(options); - - function addGlobalEventProcessor(processor: EventProcessor): void { - eventProcessors.push(processor); - expect(eventProcessors).toHaveLength(1); - } - - function getCurrentHub(): any { - return { - getIntegration(_integration: any): any { - // pretend integration is enabled - return inboundFiltersInstance; - }, - getClient(): any { - return { - getOptions: () => clientOptions, - }; - }, - }; - } - - inboundFiltersInstance.setupOnce(addGlobalEventProcessor, getCurrentHub); - return eventProcessors[0]; + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + ...clientOptions, + defaultIntegrations: false, + integrations: [new InboundFilters(options)], + }), + ); + + client.setupIntegrations(); + + const eventProcessors = client['_eventProcessors']; + const eventProcessor = eventProcessors.find(processor => processor.id === 'InboundFilters'); + + expect(eventProcessor).toBeDefined(); + return eventProcessor!; } // Fixtures diff --git a/packages/integrations/src/contextlines.ts b/packages/integrations/src/contextlines.ts index 3bc483958b42..d528477718c1 100644 --- a/packages/integrations/src/contextlines.ts +++ b/packages/integrations/src/contextlines.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Event, Integration, StackFrame } from '@sentry/types'; import { addContextToFrame, GLOBAL_OBJ, stripUrlQueryAndFragment } from '@sentry/utils'; const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; @@ -44,17 +44,20 @@ export class ContextLines implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(ContextLines); - if (!self) { - return event; - } - return this.addSourceContext(event); - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Event { + return this.addSourceContext(event); } - /** Processes an event and adds context lines */ + /** + * Processes an event and adds context lines. + * + * TODO (v8): Make this internal/private + */ public addSourceContext(event: Event): Event { const doc = WINDOW.document; const htmlFilename = WINDOW.location && stripUrlQueryAndFragment(WINDOW.location.href); diff --git a/packages/integrations/src/dedupe.ts b/packages/integrations/src/dedupe.ts index 8f156e76784d..49865de3cd79 100644 --- a/packages/integrations/src/dedupe.ts +++ b/packages/integrations/src/dedupe.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Event, Exception, Integration, StackFrame } from '@sentry/types'; import { logger } from '@sentry/utils'; /** Deduplication filter */ @@ -22,36 +22,32 @@ export class Dedupe implements Integration { this.name = Dedupe.id; } + /** @inheritDoc */ + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - const eventProcessor: EventProcessor = currentEvent => { - // We want to ignore any non-error type events, e.g. transactions or replays - // These should never be deduped, and also not be compared against as _previousEvent. - if (currentEvent.type) { - return currentEvent; - } + public processEvent(currentEvent: Event): Event | null { + // We want to ignore any non-error type events, e.g. transactions or replays + // These should never be deduped, and also not be compared against as _previousEvent. + if (currentEvent.type) { + return currentEvent; + } - const self = getCurrentHub().getIntegration(Dedupe); - if (self) { - // Juuust in case something goes wrong - try { - if (_shouldDropEvent(currentEvent, self._previousEvent)) { - __DEBUG_BUILD__ && logger.warn('Event dropped due to being a duplicate of previously captured event.'); - return null; - } - } catch (_oO) { - return (self._previousEvent = currentEvent); - } - - return (self._previousEvent = currentEvent); + // Juuust in case something goes wrong + try { + if (_shouldDropEvent(currentEvent, this._previousEvent)) { + __DEBUG_BUILD__ && logger.warn('Event dropped due to being a duplicate of previously captured event.'); + return null; } - return currentEvent; - }; + } catch (_oO) { + return (this._previousEvent = currentEvent); + } - eventProcessor.id = this.name; - addGlobalEventProcessor(eventProcessor); + return (this._previousEvent = currentEvent); } } diff --git a/packages/integrations/src/extraerrordata.ts b/packages/integrations/src/extraerrordata.ts index 86d9343ef5e3..0ac2729e3baf 100644 --- a/packages/integrations/src/extraerrordata.ts +++ b/packages/integrations/src/extraerrordata.ts @@ -1,9 +1,9 @@ -import type { Contexts, Event, EventHint, EventProcessor, ExtendedError, Hub, Integration } from '@sentry/types'; +import type { Contexts, Event, EventHint, ExtendedError, Integration } from '@sentry/types'; import { addNonEnumerableProperty, isError, isPlainObject, logger, normalize } from '@sentry/utils'; /** JSDoc */ interface ExtraErrorDataOptions { - depth?: number; + depth: number; } /** Patch toString calls to return proper name for wrapped functions */ @@ -24,7 +24,7 @@ export class ExtraErrorData implements Integration { /** * @inheritDoc */ - public constructor(options?: ExtraErrorDataOptions) { + public constructor(options?: Partial) { this.name = ExtraErrorData.id; this._options = { @@ -36,94 +36,99 @@ export class ExtraErrorData implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor((event: Event, hint: EventHint) => { - const self = getCurrentHub().getIntegration(ExtraErrorData); - if (!self) { - return event; - } - return self.enhanceEventWithErrorData(event, hint); - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event, hint: EventHint): Event { + return this.enhanceEventWithErrorData(event, hint); } /** - * Attaches extracted information from the Error object to extra field in the Event + * Attaches extracted information from the Error object to extra field in the Event. + * + * TODO (v8): Drop this public function. */ public enhanceEventWithErrorData(event: Event, hint: EventHint = {}): Event { - if (!hint.originalException || !isError(hint.originalException)) { - return event; - } - const exceptionName = (hint.originalException as ExtendedError).name || hint.originalException.constructor.name; + return _enhanceEventWithErrorData(event, hint, this._options.depth); + } +} - const errorData = this._extractErrorData(hint.originalException as ExtendedError); +function _enhanceEventWithErrorData(event: Event, hint: EventHint = {}, depth: number): Event { + if (!hint.originalException || !isError(hint.originalException)) { + return event; + } + const exceptionName = (hint.originalException as ExtendedError).name || hint.originalException.constructor.name; - if (errorData) { - const contexts: Contexts = { - ...event.contexts, - }; + const errorData = _extractErrorData(hint.originalException as ExtendedError); - const normalizedErrorData = normalize(errorData, this._options.depth); + if (errorData) { + const contexts: Contexts = { + ...event.contexts, + }; - if (isPlainObject(normalizedErrorData)) { - // We mark the error data as "already normalized" here, because we don't want other normalization procedures to - // potentially truncate the data we just already normalized, with a certain depth setting. - addNonEnumerableProperty(normalizedErrorData, '__sentry_skip_normalization__', true); - contexts[exceptionName] = normalizedErrorData; - } + const normalizedErrorData = normalize(errorData, depth); - return { - ...event, - contexts, - }; + if (isPlainObject(normalizedErrorData)) { + // We mark the error data as "already normalized" here, because we don't want other normalization procedures to + // potentially truncate the data we just already normalized, with a certain depth setting. + addNonEnumerableProperty(normalizedErrorData, '__sentry_skip_normalization__', true); + contexts[exceptionName] = normalizedErrorData; } - return event; + return { + ...event, + contexts, + }; } - /** - * Extract extra information from the Error object - */ - private _extractErrorData(error: ExtendedError): Record | null { - // We are trying to enhance already existing event, so no harm done if it won't succeed - try { - const nativeKeys = [ - 'name', - 'message', - 'stack', - 'line', - 'column', - 'fileName', - 'lineNumber', - 'columnNumber', - 'toJSON', - ]; - - const extraErrorInfo: Record = {}; - - // We want only enumerable properties, thus `getOwnPropertyNames` is redundant here, as we filter keys anyway. - for (const key of Object.keys(error)) { - if (nativeKeys.indexOf(key) !== -1) { - continue; - } - const value = error[key]; - extraErrorInfo[key] = isError(value) ? value.toString() : value; + return event; +} + +/** + * Extract extra information from the Error object + */ +function _extractErrorData(error: ExtendedError): Record | null { + // We are trying to enhance already existing event, so no harm done if it won't succeed + try { + const nativeKeys = [ + 'name', + 'message', + 'stack', + 'line', + 'column', + 'fileName', + 'lineNumber', + 'columnNumber', + 'toJSON', + ]; + + const extraErrorInfo: Record = {}; + + // We want only enumerable properties, thus `getOwnPropertyNames` is redundant here, as we filter keys anyway. + for (const key of Object.keys(error)) { + if (nativeKeys.indexOf(key) !== -1) { + continue; } + const value = error[key]; + extraErrorInfo[key] = isError(value) ? value.toString() : value; + } - // Check if someone attached `toJSON` method to grab even more properties (eg. axios is doing that) - if (typeof error.toJSON === 'function') { - const serializedError = error.toJSON() as Record; + // Check if someone attached `toJSON` method to grab even more properties (eg. axios is doing that) + if (typeof error.toJSON === 'function') { + const serializedError = error.toJSON() as Record; - for (const key of Object.keys(serializedError)) { - const value = serializedError[key]; - extraErrorInfo[key] = isError(value) ? value.toString() : value; - } + for (const key of Object.keys(serializedError)) { + const value = serializedError[key]; + extraErrorInfo[key] = isError(value) ? value.toString() : value; } - - return extraErrorInfo; - } catch (oO) { - __DEBUG_BUILD__ && logger.error('Unable to extract extra data from the Error object:', oO); } - return null; + return extraErrorInfo; + } catch (oO) { + __DEBUG_BUILD__ && logger.error('Unable to extract extra data from the Error object:', oO); } + + return null; } diff --git a/packages/integrations/src/rewriteframes.ts b/packages/integrations/src/rewriteframes.ts index 1564d54a4970..67f146e650bd 100644 --- a/packages/integrations/src/rewriteframes.ts +++ b/packages/integrations/src/rewriteframes.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration, StackFrame, Stacktrace } from '@sentry/types'; +import type { Event, Integration, StackFrame, Stacktrace } from '@sentry/types'; import { basename, relative } from '@sentry/utils'; type StackFrameIteratee = (frame: StackFrame) => StackFrame; @@ -43,17 +43,18 @@ export class RewriteFrames implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(RewriteFrames); - if (self) { - return self.process(event); - } - return event; - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop } - /** JSDoc */ + /** @inheritDoc */ + public processEvent(event: Event): Event { + return this.process(event); + } + + /** + * TODO (v8): Make this private/internal + */ public process(originalEvent: Event): Event { let processedEvent = originalEvent; diff --git a/packages/integrations/src/sessiontiming.ts b/packages/integrations/src/sessiontiming.ts index 584163ce008e..016d71f336e3 100644 --- a/packages/integrations/src/sessiontiming.ts +++ b/packages/integrations/src/sessiontiming.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration } from '@sentry/types'; +import type { Event, Integration } from '@sentry/types'; /** This function adds duration since Sentry was initialized till the time event was sent */ export class SessionTiming implements Integration { @@ -23,18 +23,17 @@ export class SessionTiming implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(SessionTiming); - if (self) { - return self.process(event); - } - return event; - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Event { + return this.process(event); } /** - * @inheritDoc + * TODO (v8): make this private/internal */ public process(event: Event): Event { const now = Date.now(); diff --git a/packages/integrations/src/transaction.ts b/packages/integrations/src/transaction.ts index 1bb3ebfa816d..ae9f826cba55 100644 --- a/packages/integrations/src/transaction.ts +++ b/packages/integrations/src/transaction.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Event, Integration, StackFrame } from '@sentry/types'; /** Add node transaction to the event */ export class Transaction implements Integration { @@ -19,43 +19,40 @@ export class Transaction implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(Transaction); - if (self) { - return self.process(event); - } - return event; - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Event { + return this.processEvent(event); } /** - * @inheritDoc + * TODO (v8): Make this private/internal */ public process(event: Event): Event { - const frames = this._getFramesFromEvent(event); + const frames = _getFramesFromEvent(event); // use for loop so we don't have to reverse whole frames array for (let i = frames.length - 1; i >= 0; i--) { const frame = frames[i]; if (frame.in_app === true) { - event.transaction = this._getTransaction(frame); + event.transaction = _getTransaction(frame); break; } } return event; } +} - /** JSDoc */ - private _getFramesFromEvent(event: Event): StackFrame[] { - const exception = event.exception && event.exception.values && event.exception.values[0]; - return (exception && exception.stacktrace && exception.stacktrace.frames) || []; - } +function _getFramesFromEvent(event: Event): StackFrame[] { + const exception = event.exception && event.exception.values && event.exception.values[0]; + return (exception && exception.stacktrace && exception.stacktrace.frames) || []; +} - /** JSDoc */ - private _getTransaction(frame: StackFrame): string { - return frame.module || frame.function ? `${frame.module || '?'}/${frame.function || '?'}` : ''; - } +function _getTransaction(frame: StackFrame): string { + return frame.module || frame.function ? `${frame.module || '?'}/${frame.function || '?'}` : ''; } diff --git a/packages/integrations/test/dedupe.test.ts b/packages/integrations/test/dedupe.test.ts index 7ffc30d1bdcf..f4a703662e0c 100644 --- a/packages/integrations/test/dedupe.test.ts +++ b/packages/integrations/test/dedupe.test.ts @@ -1,4 +1,4 @@ -import type { Event as SentryEvent, EventProcessor, Exception, Hub, StackFrame, Stacktrace } from '@sentry/types'; +import type { Event as SentryEvent, Exception, StackFrame, Stacktrace } from '@sentry/types'; import { _shouldDropEvent, Dedupe } from '../src/dedupe'; @@ -176,47 +176,29 @@ describe('Dedupe', () => { }); }); - describe('setupOnce', () => { - let dedupeFunc: EventProcessor; - - beforeEach(function () { + describe('processEvent', () => { + it('ignores consecutive errors', () => { const integration = new Dedupe(); - const addGlobalEventProcessor = (callback: EventProcessor) => { - dedupeFunc = callback; - }; - - const getCurrentHub = () => { - return { - getIntegration() { - return integration; - }, - } as unknown as Hub; - }; - - integration.setupOnce(addGlobalEventProcessor, getCurrentHub); - }); - it('ignores consecutive errors', () => { - expect(dedupeFunc(clone(exceptionEvent), {})).not.toBeNull(); - expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull(); - expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull(); + expect(integration.processEvent(clone(exceptionEvent))).not.toBeNull(); + expect(integration.processEvent(clone(exceptionEvent))).toBeNull(); + expect(integration.processEvent(clone(exceptionEvent))).toBeNull(); }); it('ignores transactions between errors', () => { - expect(dedupeFunc(clone(exceptionEvent), {})).not.toBeNull(); + const integration = new Dedupe(); + + expect(integration.processEvent(clone(exceptionEvent))).not.toBeNull(); expect( - dedupeFunc( - { - event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', - message: 'someMessage', - transaction: 'wat', - type: 'transaction', - }, - {}, - ), + integration.processEvent({ + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + message: 'someMessage', + transaction: 'wat', + type: 'transaction', + }), ).not.toBeNull(); - expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull(); - expect(dedupeFunc(clone(exceptionEvent), {})).toBeNull(); + expect(integration.processEvent(clone(exceptionEvent))).toBeNull(); + expect(integration.processEvent(clone(exceptionEvent))).toBeNull(); }); }); }); diff --git a/packages/node/src/integrations/context.ts b/packages/node/src/integrations/context.ts index 5b683d5c688d..a3d7e495a81b 100644 --- a/packages/node/src/integrations/context.ts +++ b/packages/node/src/integrations/context.ts @@ -6,7 +6,6 @@ import type { CultureContext, DeviceContext, Event, - EventProcessor, Integration, OsContext, } from '@sentry/types'; @@ -63,17 +62,26 @@ export class Context implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void { - addGlobalEventProcessor(event => this.addContext(event)); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Promise { + return this.addContext(event); } - /** Processes an event and adds context */ + /** + * Processes an event and adds context. + * + * TODO (v8): Make this private/internal. + */ public async addContext(event: Event): Promise { if (this._cachedContext === undefined) { this._cachedContext = this._getContexts(); } - const updatedContext = this._updateContext(await this._cachedContext); + const updatedContext = _updateContext(await this._cachedContext); event.contexts = { ...event.contexts, @@ -87,22 +95,6 @@ export class Context implements Integration { return event; } - /** - * Updates the context with dynamic values that can change - */ - private _updateContext(contexts: Contexts): Contexts { - // Only update properties if they exist - if (contexts?.app?.app_memory) { - contexts.app.app_memory = process.memoryUsage().rss; - } - - if (contexts?.device?.free_memory) { - contexts.device.free_memory = os.freemem(); - } - - return contexts; - } - /** * Gets the contexts for the current environment */ @@ -137,6 +129,22 @@ export class Context implements Integration { } } +/** + * Updates the context with dynamic values that can change + */ +function _updateContext(contexts: Contexts): Contexts { + // Only update properties if they exist + if (contexts?.app?.app_memory) { + contexts.app.app_memory = process.memoryUsage().rss; + } + + if (contexts?.device?.free_memory) { + contexts.device.free_memory = os.freemem(); + } + + return contexts; +} + /** * Returns the operating system context. * diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index d1c2056c0c66..9e31d8518fdf 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/packages/node/src/integrations/contextlines.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Event, Integration, StackFrame } from '@sentry/types'; import { addContextToFrame } from '@sentry/utils'; import { readFile } from 'fs'; import { LRUMap } from 'lru_map'; @@ -56,17 +56,20 @@ export class ContextLines implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(ContextLines); - if (!self) { - return event; - } - return this.addSourceContext(event); - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop } - /** Processes an event and adds context lines */ + /** @inheritDoc */ + public processEvent(event: Event): Promise { + return this.addSourceContext(event); + } + + /** + * Processes an event and adds context lines. + * + * TODO (v8): Make this internal/private + * */ public async addSourceContext(event: Event): Promise { // keep a lookup map of which files we've already enqueued to read, // so we don't enqueue the same file multiple times which would cause multiple i/o reads @@ -117,7 +120,11 @@ export class ContextLines implements Integration { return event; } - /** Adds context lines to frames */ + /** + * Adds context lines to frames. + * + * TODO (v8): Make this internal/private + */ public addSourceContextToFrames(frames: StackFrame[]): void { for (const frame of frames) { // Only add context if we have a filename and it hasn't already been added diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index f98011d94832..c85d2194d9cb 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -1,4 +1,14 @@ -import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame, StackParser } from '@sentry/types'; +import type { + Client, + Event, + EventHint, + EventProcessor, + Exception, + Hub, + Integration, + StackFrame, + StackParser, +} from '@sentry/types'; import { logger } from '@sentry/utils'; import type { Debugger, InspectorNotification, Runtime, Session } from 'inspector'; import { LRUMap } from 'lru_map'; @@ -278,29 +288,39 @@ export class LocalVariables implements Integration { this._setup(addGlobalEventProcessor, getCurrentHub().getClient()?.getOptions()); } + /** @inheritDoc */ + public processEvent(event: Event, hint: EventHint | undefined, client: Client): Event { + const clientOptions = client.getOptions() as NodeClientOptions; + if (!this._session || !clientOptions.includeLocalVariables) { + return event; + } + + return this._addLocalVariables(event); + } + /** Setup in a way that's easier to call from tests */ private _setup( addGlobalEventProcessor: (callback: EventProcessor) => void, clientOptions: NodeClientOptions | undefined, ): void { - if (this._session && clientOptions?.includeLocalVariables) { - // Only setup this integration if the Node version is >= v18 - // https://github.com/getsentry/sentry-javascript/issues/7697 - const unsupportedNodeVersion = (NODE_VERSION.major || 0) < 18; - - if (unsupportedNodeVersion) { - logger.log('The `LocalVariables` integration is only supported on Node >= v18.'); - return; - } + if (!this._session || !clientOptions?.includeLocalVariables) { + return; + } - this._session.configureAndConnect( - (ev, complete) => - this._handlePaused(clientOptions.stackParser, ev as InspectorNotification, complete), - !!this._options.captureAllExceptions, - ); + // Only setup this integration if the Node version is >= v18 + // https://github.com/getsentry/sentry-javascript/issues/7697 + const unsupportedNodeVersion = (NODE_VERSION.major || 0) < 18; - addGlobalEventProcessor(async event => this._addLocalVariables(event)); + if (unsupportedNodeVersion) { + logger.log('The `LocalVariables` integration is only supported on Node >= v18.'); + return; } + + this._session.configureAndConnect( + (ev, complete) => + this._handlePaused(clientOptions.stackParser, ev as InspectorNotification, complete), + !!this._options.captureAllExceptions, + ); } /** diff --git a/packages/node/src/integrations/modules.ts b/packages/node/src/integrations/modules.ts index 376c0b0144f6..e9f4709ad466 100644 --- a/packages/node/src/integrations/modules.ts +++ b/packages/node/src/integrations/modules.ts @@ -1,4 +1,4 @@ -import type { EventProcessor, Hub, Integration } from '@sentry/types'; +import type { Event, Integration } from '@sentry/types'; import { existsSync, readFileSync } from 'fs'; import { dirname, join } from 'path'; @@ -80,26 +80,26 @@ export class Modules implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - if (!getCurrentHub().getIntegration(Modules)) { - return event; - } - return { - ...event, - modules: { - ...event.modules, - ...this._getModules(), - }, - }; - }); + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop } - /** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */ - private _getModules(): { [key: string]: string } { - if (!moduleCache) { - moduleCache = collectModules(); - } - return moduleCache; + /** @inheritDoc */ + public processEvent(event: Event): Event { + return { + ...event, + modules: { + ...event.modules, + ..._getModules(), + }, + }; + } +} + +/** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */ +function _getModules(): { [key: string]: string } { + if (!moduleCache) { + moduleCache = collectModules(); } + return moduleCache; } diff --git a/packages/node/src/integrations/requestdata.ts b/packages/node/src/integrations/requestdata.ts index 5521345a7b98..544c5b55790a 100644 --- a/packages/node/src/integrations/requestdata.ts +++ b/packages/node/src/integrations/requestdata.ts @@ -1,7 +1,7 @@ // TODO (v8 or v9): Whenever this becomes a default integration for `@sentry/browser`, move this to `@sentry/core`. For // now, we leave it in `@sentry/integrations` so that it doesn't contribute bytes to our CDN bundles. -import type { Event, EventProcessor, Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/types'; +import type { Client, Event, EventHint, Integration, PolymorphicRequest, Transaction } from '@sentry/types'; import { extractPathForTransaction } from '@sentry/utils'; import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '../requestdata'; @@ -99,65 +99,66 @@ export class RequestData implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (eventProcessor: EventProcessor) => void, getCurrentHub: () => Hub): void { + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event, hint: EventHint | undefined, client: Client): Event { // Note: In the long run, most of the logic here should probably move into the request data utility functions. For // the moment it lives here, though, until https://github.com/getsentry/sentry-javascript/issues/5718 is addressed. // (TL;DR: Those functions touch many parts of the repo in many different ways, and need to be clened up. Once // that's happened, it will be easier to add this logic in without worrying about unexpected side effects.) - const { transactionNamingScheme } = this._options; - addGlobalEventProcessor(event => { - const hub = getCurrentHub(); - const self = hub.getIntegration(RequestData); + const { transactionNamingScheme } = this._options; - const { sdkProcessingMetadata = {} } = event; - const req = sdkProcessingMetadata.request; + const { sdkProcessingMetadata = {} } = event; + const req = sdkProcessingMetadata.request; - // If the globally installed instance of this integration isn't associated with the current hub, `self` will be - // undefined - if (!self || !req) { - return event; - } + // If the globally installed instance of this integration isn't associated with the current hub, `self` will be + // undefined + if (!req) { + return event; + } - // The Express request handler takes a similar `include` option to that which can be passed to this integration. - // If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express and GCP people to use this - // integration, so that all of this passing and conversion isn't necessary - const addRequestDataOptions = - sdkProcessingMetadata.requestDataOptionsFromExpressHandler || - sdkProcessingMetadata.requestDataOptionsFromGCPWrapper || - convertReqDataIntegrationOptsToAddReqDataOpts(this._options); + // The Express request handler takes a similar `include` option to that which can be passed to this integration. + // If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express and GCP people to use this + // integration, so that all of this passing and conversion isn't necessary + const addRequestDataOptions = + sdkProcessingMetadata.requestDataOptionsFromExpressHandler || + sdkProcessingMetadata.requestDataOptionsFromGCPWrapper || + convertReqDataIntegrationOptsToAddReqDataOpts(this._options); - const processedEvent = this._addRequestData(event, req, addRequestDataOptions); + const processedEvent = this._addRequestData(event, req, addRequestDataOptions); - // Transaction events already have the right `transaction` value - if (event.type === 'transaction' || transactionNamingScheme === 'handler') { - return processedEvent; - } + // Transaction events already have the right `transaction` value + if (event.type === 'transaction' || transactionNamingScheme === 'handler') { + return processedEvent; + } - // In all other cases, use the request's associated transaction (if any) to overwrite the event's `transaction` - // value with a high-quality one - const reqWithTransaction = req as { _sentryTransaction?: Transaction }; - const transaction = reqWithTransaction._sentryTransaction; - if (transaction) { - // TODO (v8): Remove the nextjs check and just base it on `transactionNamingScheme` for all SDKs. (We have to - // keep it the way it is for the moment, because changing the names of transactions in Sentry has the potential - // to break things like alert rules.) - const shouldIncludeMethodInTransactionName = - getSDKName(hub) === 'sentry.javascript.nextjs' - ? transaction.name.startsWith('/api') - : transactionNamingScheme !== 'path'; - - const [transactionValue] = extractPathForTransaction(req, { - path: true, - method: shouldIncludeMethodInTransactionName, - customRoute: transaction.name, - }); - - processedEvent.transaction = transactionValue; - } + // In all other cases, use the request's associated transaction (if any) to overwrite the event's `transaction` + // value with a high-quality one + const reqWithTransaction = req as { _sentryTransaction?: Transaction }; + const transaction = reqWithTransaction._sentryTransaction; + if (transaction) { + // TODO (v8): Remove the nextjs check and just base it on `transactionNamingScheme` for all SDKs. (We have to + // keep it the way it is for the moment, because changing the names of transactions in Sentry has the potential + // to break things like alert rules.) + const shouldIncludeMethodInTransactionName = + getSDKName(client) === 'sentry.javascript.nextjs' + ? transaction.name.startsWith('/api') + : transactionNamingScheme !== 'path'; + + const [transactionValue] = extractPathForTransaction(req, { + path: true, + method: shouldIncludeMethodInTransactionName, + customRoute: transaction.name, + }); + + processedEvent.transaction = transactionValue; + } - return processedEvent; - }); + return processedEvent; } } @@ -203,12 +204,12 @@ function convertReqDataIntegrationOptsToAddReqDataOpts( }; } -function getSDKName(hub: Hub): string | undefined { +function getSDKName(client: Client): string | undefined { try { // For a long chain like this, it's fewer bytes to combine a try-catch with assuming everything is there than to // write out a long chain of `a && a.b && a.b.c && ...` // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return hub.getClient()!.getOptions()!._metadata!.sdk!.name; + return client.getOptions()!._metadata!.sdk!.name; } catch (err) { // In theory we should never get here return undefined; diff --git a/packages/node/test/integrations/requestdata.test.ts b/packages/node/test/integrations/requestdata.test.ts index 52e20c9d6e4b..606f2618a6ab 100644 --- a/packages/node/test/integrations/requestdata.test.ts +++ b/packages/node/test/integrations/requestdata.test.ts @@ -1,4 +1,4 @@ -import { getCurrentHub, Hub, makeMain } from '@sentry/core'; +import { getCurrentHub } from '@sentry/core'; import type { Event, EventProcessor, PolymorphicRequest } from '@sentry/types'; import * as http from 'http'; @@ -10,7 +10,6 @@ import * as requestDataModule from '../../src/requestdata'; import { getDefaultNodeClientOptions } from '../helper/node-client-options'; const addRequestDataToEventSpy = jest.spyOn(requestDataModule, 'addRequestDataToEvent'); -const requestDataEventProcessor = jest.fn(); const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' }; const method = 'wagging'; @@ -19,10 +18,7 @@ const hostname = 'the.dog.park'; const path = '/by/the/trees/'; const queryString = 'chase=me&please=thankyou'; -function initWithRequestDataIntegrationOptions(integrationOptions: RequestDataIntegrationOptions): void { - const setMockEventProcessor = (eventProcessor: EventProcessor) => - requestDataEventProcessor.mockImplementationOnce(eventProcessor); - +function initWithRequestDataIntegrationOptions(integrationOptions: RequestDataIntegrationOptions): EventProcessor { const requestDataIntegration = new RequestData({ ...integrationOptions, }); @@ -33,12 +29,15 @@ function initWithRequestDataIntegrationOptions(integrationOptions: RequestDataIn integrations: [requestDataIntegration], }), ); - client.setupIntegrations = () => requestDataIntegration.setupOnce(setMockEventProcessor, getCurrentHub); - client.getIntegration = () => requestDataIntegration as any; - const hub = new Hub(client); + getCurrentHub().bindClient(client); + + const eventProcessors = client['_eventProcessors'] as EventProcessor[]; + const eventProcessor = eventProcessors.find(processor => processor.id === 'RequestData'); + + expect(eventProcessor).toBeDefined(); - makeMain(hub); + return eventProcessor!; } describe('`RequestData` integration', () => { @@ -61,9 +60,9 @@ describe('`RequestData` integration', () => { describe('option conversion', () => { it('leaves `ip` and `user` at top level of `include`', () => { - initWithRequestDataIntegrationOptions({ include: { ip: false, user: true } }); + const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ include: { ip: false, user: true } }); - requestDataEventProcessor(event); + void requestDataEventProcessor(event, {}); const passedOptions = addRequestDataToEventSpy.mock.calls[0][2]; @@ -71,9 +70,9 @@ describe('`RequestData` integration', () => { }); it('moves `transactionNamingScheme` to `transaction` include', () => { - initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' }); + const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' }); - requestDataEventProcessor(event); + void requestDataEventProcessor(event, {}); const passedOptions = addRequestDataToEventSpy.mock.calls[0][2]; @@ -81,9 +80,11 @@ describe('`RequestData` integration', () => { }); it('moves `true` request keys into `request` include, but omits `false` ones', async () => { - initWithRequestDataIntegrationOptions({ include: { data: true, cookies: false } }); + const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ + include: { data: true, cookies: false }, + }); - requestDataEventProcessor(event); + void requestDataEventProcessor(event, {}); const passedOptions = addRequestDataToEventSpy.mock.calls[0][2]; @@ -92,9 +93,11 @@ describe('`RequestData` integration', () => { }); it('moves `true` user keys into `user` include, but omits `false` ones', async () => { - initWithRequestDataIntegrationOptions({ include: { user: { id: true, email: false } } }); + const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ + include: { user: { id: true, email: false } }, + }); - requestDataEventProcessor(event); + void requestDataEventProcessor(event, {}); const passedOptions = addRequestDataToEventSpy.mock.calls[0][2]; @@ -109,12 +112,12 @@ describe('`RequestData` integration', () => { const res = new http.ServerResponse(req); const next = jest.fn(); - initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' }); + const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' }); sentryRequestMiddleware(req, res, next); await getCurrentHub().getScope()!.applyToEvent(event, {}); - requestDataEventProcessor(event); + await requestDataEventProcessor(event, {}); const passedOptions = addRequestDataToEventSpy.mock.calls[0][2]; @@ -138,12 +141,12 @@ describe('`RequestData` integration', () => { const wrappedGCPFunction = mockGCPWrapper(jest.fn(), { include: { transaction: 'methodPath' } }); const res = new http.ServerResponse(req); - initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' }); + const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' }); wrappedGCPFunction(req, res); await getCurrentHub().getScope()!.applyToEvent(event, {}); - requestDataEventProcessor(event); + await requestDataEventProcessor(event, {}); const passedOptions = addRequestDataToEventSpy.mock.calls[0][2]; diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 00361d0ada99..8ad048ee08c4 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -5,6 +5,7 @@ import type { DataCategory } from './datacategory'; import type { DsnComponents } from './dsn'; import type { DynamicSamplingContext, Envelope } from './envelope'; import type { Event, EventHint } from './event'; +import type { EventProcessor } from './eventprocessor'; import type { Integration, IntegrationClass } from './integration'; import type { ClientOptions } from './options'; import type { Scope } from './scope'; @@ -120,6 +121,13 @@ export interface Client { */ flush(timeout?: number): PromiseLike; + /** + * Adds an event processor that applies to any event processed by this client. + * + * TODO (v8): Make this a required method. + */ + addEventProcessor?(eventProcessor: EventProcessor): void; + /** Returns the client's instance of the given integration class, it any. */ getIntegration(integration: IntegrationClass): T | null; diff --git a/packages/types/src/integration.ts b/packages/types/src/integration.ts index 0c1feae65323..b985311f4fe2 100644 --- a/packages/types/src/integration.ts +++ b/packages/types/src/integration.ts @@ -30,4 +30,10 @@ export interface Integration { * An optional hook that allows to preprocess an event _before_ it is passed to all other event processors. */ preprocessEvent?(event: Event, hint: EventHint | undefined, client: Client): void; + + /** + * An optional hook that allows to process an event. + * Return `null` to drop the event, or mutate the event & return it. + */ + processEvent?(event: Event, hint: EventHint | undefined, client: Client): Event | null | PromiseLike; } diff --git a/packages/wasm/src/index.ts b/packages/wasm/src/index.ts index c07f9cd7ca16..a45b18ac4e96 100644 --- a/packages/wasm/src/index.ts +++ b/packages/wasm/src/index.ts @@ -1,4 +1,4 @@ -import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; +import type { Event, Integration, StackFrame } from '@sentry/types'; import { patchWebAssembly } from './patchWebAssembly'; import { getImage, getImages } from './registry'; @@ -49,26 +49,27 @@ export class Wasm implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void { + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { patchWebAssembly(); + } - addGlobalEventProcessor((event: Event) => { - let haveWasm = false; + /** @inheritDoc */ + public processEvent(event: Event): Event { + let haveWasm = false; - if (event.exception && event.exception.values) { - event.exception.values.forEach(exception => { - if (exception?.stacktrace?.frames) { - haveWasm = haveWasm || patchFrames(exception.stacktrace.frames); - } - }); - } + if (event.exception && event.exception.values) { + event.exception.values.forEach(exception => { + if (exception?.stacktrace?.frames) { + haveWasm = haveWasm || patchFrames(exception.stacktrace.frames); + } + }); + } - if (haveWasm) { - event.debug_meta = event.debug_meta || {}; - event.debug_meta.images = [...(event.debug_meta.images || []), ...getImages()]; - } + if (haveWasm) { + event.debug_meta = event.debug_meta || {}; + event.debug_meta.images = [...(event.debug_meta.images || []), ...getImages()]; + } - return event; - }); + return event; } }