From 8f67ee181d4a29d0e6d882ad1a91d4649f031352 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 8 Dec 2021 22:30:00 -0800 Subject: [PATCH 1/9] add processing metadata property to both event and scope --- packages/hub/src/scope.ts | 6 ++++++ packages/types/src/event.ts | 2 ++ 2 files changed, 8 insertions(+) diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 940d0443c9f7..3489f0f0013c 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -75,6 +75,12 @@ export class Scope implements ScopeInterface { /** Request Mode Session Status */ protected _requestSession?: RequestSession; + /** + * A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get + * sent to Sentry + */ + protected _processingMetadata?: { [key: string]: any } = {}; + /** * Inherit values from the parent scope. * @param scope to clone. diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 67f6d9358792..51d2ca900f36 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -44,6 +44,8 @@ export interface Event { spans?: Span[]; measurements?: Measurements; debug_meta?: DebugMeta; + // A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get sent to Sentry + processingMetadata?: { [key: string]: any }; } /** JSDoc */ From 22febe3958ffd316d9e0b10639e3afcd0c25b5c9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 8 Dec 2021 22:30:47 -0800 Subject: [PATCH 2/9] add method to set processing metadata on scope --- packages/hub/src/scope.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 3489f0f0013c..6619b8392905 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -450,6 +450,15 @@ export class Scope implements ScopeInterface { return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint); } + /** + * Add data which will be accessible during event processing but won't get sent to Sentry + */ + public setProcessingMetadata(newData: { [key: string]: any }): this { + this._processingMetadata = { ...this._processingMetadata, ...newData }; + + return this; + } + /** * This will be called after {@link applyToEvent} is finished. */ From 4d37b31070c7151ae41961296eaaa27635bc15d1 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 8 Dec 2021 22:31:11 -0800 Subject: [PATCH 3/9] apply scope processing metadata to event --- packages/hub/src/scope.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 6619b8392905..63c13854952c 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -447,6 +447,8 @@ export class Scope implements ScopeInterface { event.breadcrumbs = [...(event.breadcrumbs || []), ...this._breadcrumbs]; event.breadcrumbs = event.breadcrumbs.length > 0 ? event.breadcrumbs : undefined; + event.processingMetadata = this._processingMetadata; + return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint); } From b67be60b9b744924ae0772811295df21b51a8b54 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 8 Dec 2021 22:32:00 -0800 Subject: [PATCH 4/9] add scope tests --- packages/hub/test/scope.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index 9b1760dcd11c..b087f95d4b2c 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -129,6 +129,12 @@ describe('Scope', () => { expect((scope as any)._span).toEqual(undefined); }); + test('setProcessingMetadata', () => { + const scope = new Scope(); + scope.setProcessingMetadata({ dogs: 'are great!' }); + expect((scope as any)._processingMetadata.dogs).toEqual('are great!'); + }); + test('chaining', () => { const scope = new Scope(); scope.setLevel('critical').setUser({ id: '1' }); @@ -189,7 +195,8 @@ describe('Scope', () => { describe('applyToEvent', () => { test('basic usage', () => { - expect.assertions(8); + expect.assertions(9); + const scope = new Scope(); scope.setExtra('a', 2); scope.setTag('a', 'b'); @@ -199,6 +206,8 @@ describe('Scope', () => { scope.setTransactionName('/abc'); scope.addBreadcrumb({ message: 'test' }); scope.setContext('os', { id: '1' }); + scope.setProcessingMetadata({ dogs: 'are great!' }); + const event: Event = {}; return scope.applyToEvent(event).then(processedEvent => { expect(processedEvent!.extra).toEqual({ a: 2 }); @@ -209,6 +218,7 @@ describe('Scope', () => { expect(processedEvent!.transaction).toEqual('/abc'); expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test'); expect(processedEvent!.contexts).toEqual({ os: { id: '1' } }); + expect(processedEvent!.processingMetadata).toEqual({ dogs: 'are great!' }); }); }); From 5a51c34a01a36a71587a5abc9aa68055aa337dff Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 8 Dec 2021 22:32:29 -0800 Subject: [PATCH 5/9] remove processing metadata from final event --- packages/core/src/request.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index db1810cdbbbd..9f6c90ad977a 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -63,6 +63,9 @@ export function eventToSentryRequest(event: Event, api: APIDetails): SentryReque event.debug_meta = metadata; } + // prevent this data from being sent to sentry + delete event.processingMetadata; + const req: SentryRequest = { body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event), type: eventType, From 13f7ed0120b30a3270877425164afd28cb0b7821 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 8 Dec 2021 22:32:57 -0800 Subject: [PATCH 6/9] add test for removing processing metadata from final event --- packages/core/test/lib/request.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/core/test/lib/request.test.ts b/packages/core/test/lib/request.test.ts index bd9c3e424bac..4a27d6e2e708 100644 --- a/packages/core/test/lib/request.test.ts +++ b/packages/core/test/lib/request.test.ts @@ -162,6 +162,15 @@ describe('eventToSentryRequest', () => { }), ); }); + + it('removes processing metadata before serializing event', () => { + event.processingMetadata = { dogs: 'are great!' }; + + const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.event.processingMetadata).toBeUndefined(); + }); }); describe('sessionToSentryRequest', () => { From dfa091643c5bd339defeb1dcdc9be389b8d2a2dc Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 10 Dec 2021 15:39:44 -0800 Subject: [PATCH 7/9] fix baseclient tests --- packages/core/test/lib/base.test.ts | 405 ++++++++++++++++++++-------- 1 file changed, 296 insertions(+), 109 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index a5156e40c26e..a318827dcec5 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -66,18 +66,22 @@ describe('BaseClient', () => { describe('constructor() / getDsn()', () => { test('returns the Dsn', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); expect(dsnToString(client.getDsn())).toBe(PUBLIC_DSN); }); test('allows missing Dsn', () => { expect.assertions(1); + const client = new TestClient({}); + expect(client.getDsn()).toBeUndefined(); }); test('throws with invalid Dsn', () => { expect.assertions(1); + expect(() => new TestClient({ dsn: 'abc' })).toThrow(SentryError); }); }); @@ -85,8 +89,10 @@ describe('BaseClient', () => { describe('getOptions()', () => { test('returns the options', () => { expect.assertions(1); + const options = { dsn: PUBLIC_DSN, test: true }; const client = new TestClient(options); + expect(client.getOptions()).toEqual(options); }); }); @@ -94,8 +100,10 @@ describe('BaseClient', () => { describe('getTransport()', () => { test('returns the transport from backend', () => { expect.assertions(2); + const options = { dsn: PUBLIC_DSN, transport: FakeTransport }; const client = new TestClient(options); + expect(client.getTransport()).toBeInstanceOf(FakeTransport); expect(TestBackend.instance!.getTransport()).toBe(client.getTransport()); }); @@ -104,82 +112,106 @@ describe('BaseClient', () => { describe('getBreadcrumbs() / addBreadcrumb()', () => { test('adds a breadcrumb', () => { expect.assertions(1); + const client = new TestClient({}); const scope = new Scope(); const hub = new Hub(client, scope); + scope.addBreadcrumb({ message: 'hello' }, 100); hub.addBreadcrumb({ message: 'world' }); + expect((scope as any)._breadcrumbs[1].message).toBe('world'); }); test('adds a timestamp to new breadcrumbs', () => { expect.assertions(1); + const client = new TestClient({}); const scope = new Scope(); const hub = new Hub(client, scope); + scope.addBreadcrumb({ message: 'hello' }, 100); hub.addBreadcrumb({ message: 'world' }); + expect((scope as any)._breadcrumbs[1].timestamp).toBeGreaterThan(1); }); test('discards breadcrumbs beyond maxBreadcrumbs', () => { expect.assertions(2); + const client = new TestClient({ maxBreadcrumbs: 1 }); const scope = new Scope(); const hub = new Hub(client, scope); + scope.addBreadcrumb({ message: 'hello' }, 100); hub.addBreadcrumb({ message: 'world' }); + expect((scope as any)._breadcrumbs.length).toBe(1); expect((scope as any)._breadcrumbs[0].message).toBe('world'); }); test('allows concurrent updates', () => { expect.assertions(1); + const client = new TestClient({}); const scope = new Scope(); const hub = new Hub(client, scope); + hub.addBreadcrumb({ message: 'hello' }); hub.addBreadcrumb({ message: 'world' }); + expect((scope as any)._breadcrumbs).toHaveLength(2); }); test('calls beforeBreadcrumb and adds the breadcrumb without any changes', () => { expect.assertions(1); + const beforeBreadcrumb = jest.fn(breadcrumb => breadcrumb); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); const hub = new Hub(client, scope); + hub.addBreadcrumb({ message: 'hello' }); + expect((scope as any)._breadcrumbs[0].message).toBe('hello'); }); test('calls beforeBreadcrumb and uses the new one', () => { expect.assertions(1); + const beforeBreadcrumb = jest.fn(() => ({ message: 'changed' })); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); const hub = new Hub(client, scope); + hub.addBreadcrumb({ message: 'hello' }); + expect((scope as any)._breadcrumbs[0].message).toBe('changed'); }); test('calls beforeBreadcrumb and discards the breadcrumb when returned null', () => { expect.assertions(1); + const beforeBreadcrumb = jest.fn(() => null); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); const hub = new Hub(client, scope); + hub.addBreadcrumb({ message: 'hello' }); + expect((scope as any)._breadcrumbs.length).toBe(0); }); test('calls beforeBreadcrumb gets an access to a hint as a second argument', () => { expect.assertions(2); + const beforeBreadcrumb = jest.fn((breadcrumb, hint) => ({ ...breadcrumb, data: hint.data })); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); const hub = new Hub(client, scope); + hub.addBreadcrumb({ message: 'hello' }, { data: 'someRandomThing' }); + expect((scope as any)._breadcrumbs[0].message).toBe('hello'); expect((scope as any)._breadcrumbs[0].data).toBe('someRandomThing'); }); @@ -188,26 +220,31 @@ describe('BaseClient', () => { describe('captureException', () => { test('captures and sends exceptions', () => { const client = new TestClient({ dsn: PUBLIC_DSN }); + client.captureException(new Error('test exception')); - expect(TestBackend.instance!.event).toEqual({ - environment: 'production', - event_id: '42', - exception: { - values: [ - { - type: 'Error', - value: 'test exception', - }, - ], - }, - timestamp: 2020, - }); + + expect(TestBackend.instance!.event).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + exception: { + values: [ + { + type: 'Error', + value: 'test exception', + }, + ], + }, + timestamp: 2020, + }), + ); }); test('allows for providing explicit scope', () => { const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setExtra('foo', 'wat'); + client.captureException( new Error('test exception'), { @@ -219,6 +256,7 @@ describe('BaseClient', () => { }, scope, ); + expect(TestBackend.instance!.event).toEqual( expect.objectContaining({ extra: { @@ -233,6 +271,7 @@ describe('BaseClient', () => { const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setExtra('foo', 'wat'); + client.captureException( new Error('test exception'), { @@ -244,6 +283,7 @@ describe('BaseClient', () => { }, scope, ); + expect(TestBackend.instance!.event).toEqual( expect.objectContaining({ extra: { @@ -281,14 +321,18 @@ describe('BaseClient', () => { describe('captureMessage', () => { test('captures and sends messages', () => { const client = new TestClient({ dsn: PUBLIC_DSN }); + client.captureMessage('test message'); - expect(TestBackend.instance!.event).toEqual({ - environment: 'production', - event_id: '42', - level: 'info', - message: 'test message', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + level: 'info', + message: 'test message', + timestamp: 2020, + }), + ); }); test('should call eventFromException if input to captureMessage is not a primitive', () => { @@ -300,10 +344,12 @@ describe('BaseClient', () => { client.captureMessage(undefined as any); client.captureMessage(1 as any); client.captureMessage(false as any); + expect(spy.mock.calls.length).toEqual(0); client.captureMessage({} as any); client.captureMessage([] as any); + expect(spy.mock.calls.length).toEqual(2); }); @@ -311,6 +357,7 @@ describe('BaseClient', () => { const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setExtra('foo', 'wat'); + client.captureMessage( 'test message', 'warning', @@ -323,6 +370,7 @@ describe('BaseClient', () => { }, scope, ); + expect(TestBackend.instance!.event).toEqual( expect.objectContaining({ extra: { @@ -338,17 +386,23 @@ describe('BaseClient', () => { describe('captureEvent() / prepareEvent()', () => { test('skips when disabled', () => { expect.assertions(1); + const client = new TestClient({ enabled: false, dsn: PUBLIC_DSN }); const scope = new Scope(); + client.captureEvent({}, undefined, scope); + expect(TestBackend.instance!.event).toBeUndefined(); }); test('skips without a Dsn', () => { expect.assertions(1); + const client = new TestClient({}); const scope = new Scope(); + client.captureEvent({}, undefined, scope); + expect(TestBackend.instance!.event).toBeUndefined(); }); @@ -383,115 +437,153 @@ describe('BaseClient', () => { test('sends an event', () => { expect.assertions(2); + const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); + client.captureEvent({ message: 'message' }, undefined, scope); + expect(TestBackend.instance!.event!.message).toBe('message'); - expect(TestBackend.instance!.event).toEqual({ - environment: 'production', - event_id: '42', - message: 'message', - timestamp: 2020, - }); + expect(TestBackend.instance!.event).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + message: 'message', + timestamp: 2020, + }), + ); }); test('does not overwrite existing timestamp', () => { expect.assertions(2); + const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); + client.captureEvent({ message: 'message', timestamp: 1234 }, undefined, scope); + expect(TestBackend.instance!.event!.message).toBe('message'); - expect(TestBackend.instance!.event).toEqual({ - environment: 'production', - event_id: '42', - message: 'message', - timestamp: 1234, - }); + expect(TestBackend.instance!.event).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + message: 'message', + timestamp: 1234, + }), + ); }); test('adds event_id from hint if available', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); + client.captureEvent({ message: 'message' }, { event_id: 'wat' }, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: 'production', - event_id: 'wat', - message: 'message', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: 'wat', + message: 'message', + timestamp: 2020, + }), + ); }); - test('sets default environment to `production` it none provided', () => { + test('sets default environment to `production` if none provided', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, }); const scope = new Scope(); + client.captureEvent({ message: 'message' }, undefined, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: 'production', - event_id: '42', - message: 'message', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + message: 'message', + timestamp: 2020, + }), + ); }); test('adds the configured environment', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, environment: 'env', }); const scope = new Scope(); + client.captureEvent({ message: 'message' }, undefined, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: 'env', - event_id: '42', - message: 'message', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: 'env', + event_id: '42', + message: 'message', + timestamp: 2020, + }), + ); }); test('allows for environment to be explicitly set to falsy value', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, environment: undefined, }); const scope = new Scope(); + client.captureEvent({ message: 'message' }, undefined, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: undefined, - event_id: '42', - message: 'message', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: undefined, + event_id: '42', + message: 'message', + timestamp: 2020, + }), + ); }); test('adds the configured release', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, release: 'v1.0.0', }); const scope = new Scope(); + client.captureEvent({ message: 'message' }, undefined, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: 'production', - event_id: '42', - message: 'message', - release: 'v1.0.0', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + message: 'message', + release: 'v1.0.0', + timestamp: 2020, + }), + ); }); test('adds breadcrumbs', () => { expect.assertions(4); + const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.addBreadcrumb({ message: 'breadcrumb' }, 100); + client.captureEvent({ message: 'message' }, undefined, scope); + expect(TestBackend.instance!.event!).toHaveProperty('event_id', '42'); expect(TestBackend.instance!.event!).toHaveProperty('message', 'message'); expect(TestBackend.instance!.event!).toHaveProperty('breadcrumbs'); @@ -500,54 +592,69 @@ describe('BaseClient', () => { test('limits previously saved breadcrumbs', () => { expect.assertions(2); + const client = new TestClient({ dsn: PUBLIC_DSN, maxBreadcrumbs: 1 }); const scope = new Scope(); const hub = new Hub(client, scope); hub.addBreadcrumb({ message: '1' }); hub.addBreadcrumb({ message: '2' }); + client.captureEvent({ message: 'message' }, undefined, scope); + expect(TestBackend.instance!.event!.breadcrumbs).toHaveLength(1); expect(TestBackend.instance!.event!.breadcrumbs![0].message).toEqual('2'); }); test('adds context data', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setExtra('b', 'b'); scope.setTag('a', 'a'); scope.setUser({ id: 'user' }); + client.captureEvent({ message: 'message' }, undefined, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: 'production', - event_id: '42', - extra: { b: 'b' }, - message: 'message', - tags: { a: 'a' }, - timestamp: 2020, - user: { id: 'user' }, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + extra: { b: 'b' }, + message: 'message', + tags: { a: 'a' }, + timestamp: 2020, + user: { id: 'user' }, + }), + ); }); test('adds fingerprint', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setFingerprint(['abcd']); + client.captureEvent({ message: 'message' }, undefined, scope); - expect(TestBackend.instance!.event!).toEqual({ - environment: 'production', - event_id: '42', - fingerprint: ['abcd'], - message: 'message', - timestamp: 2020, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + environment: 'production', + event_id: '42', + fingerprint: ['abcd'], + message: 'message', + timestamp: 2020, + }), + ); }); test('adds installed integrations to sdk info', () => { const client = new TestClient({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()] }); client.setupIntegrations(); + client.captureEvent({ message: 'message' }); + expect(TestBackend.instance!.event!.sdk).toEqual({ integrations: ['TestIntegration'], }); @@ -555,6 +662,7 @@ describe('BaseClient', () => { test('normalizes event with default depth of 3', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const fourLevelsObject = { a: { @@ -582,25 +690,30 @@ describe('BaseClient', () => { data: normalizedObject, message: 'wat', }; + client.captureEvent({ breadcrumbs: [fourLevelBreadcrumb, fourLevelBreadcrumb, fourLevelBreadcrumb], contexts: fourLevelsObject, extra: fourLevelsObject, user: fourLevelsObject, }); - expect(TestBackend.instance!.event!).toEqual({ - breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], - contexts: normalizedObject, - environment: 'production', - event_id: '42', - extra: normalizedObject, - timestamp: 2020, - user: normalizedObject, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], + contexts: normalizedObject, + environment: 'production', + event_id: '42', + extra: normalizedObject, + timestamp: 2020, + user: normalizedObject, + }), + ); }); test('normalization respects `normalizeDepth` option', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, normalizeDepth: 2, @@ -628,25 +741,30 @@ describe('BaseClient', () => { data: normalizedObject, message: 'wat', }; + client.captureEvent({ breadcrumbs: [fourLevelBreadcrumb, fourLevelBreadcrumb, fourLevelBreadcrumb], contexts: fourLevelsObject, extra: fourLevelsObject, user: fourLevelsObject, }); - expect(TestBackend.instance!.event!).toEqual({ - breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], - contexts: normalizedObject, - environment: 'production', - event_id: '42', - extra: normalizedObject, - timestamp: 2020, - user: normalizedObject, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], + contexts: normalizedObject, + environment: 'production', + event_id: '42', + extra: normalizedObject, + timestamp: 2020, + user: normalizedObject, + }), + ); }); test('skips normalization when `normalizeDepth: 0`', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, normalizeDepth: 0, @@ -679,25 +797,30 @@ describe('BaseClient', () => { data: normalizedObject, message: 'wat', }; + client.captureEvent({ breadcrumbs: [fourLevelBreadcrumb, fourLevelBreadcrumb, fourLevelBreadcrumb], contexts: fourLevelsObject, extra: fourLevelsObject, user: fourLevelsObject, }); - expect(TestBackend.instance!.event!).toEqual({ - breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], - contexts: normalizedObject, - environment: 'production', - event_id: '42', - extra: normalizedObject, - timestamp: 2020, - user: normalizedObject, - }); + + expect(TestBackend.instance!.event!).toEqual( + expect.objectContaining({ + breadcrumbs: [normalizedBreadcrumb, normalizedBreadcrumb, normalizedBreadcrumb], + contexts: normalizedObject, + environment: 'production', + event_id: '42', + extra: normalizedObject, + timestamp: 2020, + user: normalizedObject, + }), + ); }); test('normalization applies to Transaction and Span consistently', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const transaction: Event = { contexts: { @@ -741,33 +864,44 @@ describe('BaseClient', () => { // both, such that the expected normalizedTransaction is the same as the // input transaction. const normalizedTransaction = JSON.parse(JSON.stringify(transaction)); // deep-copy + client.captureEvent(transaction); + expect(TestBackend.instance!.event!).toEqual(normalizedTransaction); }); test('calls beforeSend and uses original event without any changes', () => { expect.assertions(1); + const beforeSend = jest.fn(event => event); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }); + expect(TestBackend.instance!.event!.message).toBe('hello'); }); test('calls beforeSend and uses the new one', () => { expect.assertions(1); + const beforeSend = jest.fn(() => ({ message: 'changed1' })); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }); + expect(TestBackend.instance!.event!.message).toBe('changed1'); }); test('calls beforeSend and discards the event', () => { expect.assertions(3); + const beforeSend = jest.fn(() => null); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); const captureExceptionSpy = jest.spyOn(client, 'captureException'); const loggerErrorSpy = jest.spyOn(logger, 'error'); + client.captureEvent({ message: 'hello' }); + expect(TestBackend.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toBeCalled(); expect(loggerErrorSpy).toBeCalledWith(new SentryError('`beforeSend` returned `null`, will not send event.')); @@ -782,7 +916,9 @@ describe('BaseClient', () => { // @ts-ignore we need to test regular-js behavior const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); const loggerErrorSpy = jest.spyOn(logger, 'error'); + client.captureEvent({ message: 'hello' }); + expect(TestBackend.instance!.event).toBeUndefined(); expect(loggerErrorSpy).toBeCalledWith( new SentryError('`beforeSend` method has to return `null` or a valid event.'), @@ -793,6 +929,7 @@ describe('BaseClient', () => { test('calls async beforeSend and uses original event without any changes', done => { jest.useFakeTimers(); expect.assertions(1); + const beforeSend = jest.fn( async event => new Promise(resolve => { @@ -802,20 +939,25 @@ describe('BaseClient', () => { }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }); jest.runOnlyPendingTimers(); + TestBackend.sendEventCalled = (event: Event) => { expect(event.message).toBe('hello'); }; + setTimeout(() => { done(); }, 5); + jest.runOnlyPendingTimers(); }); test('calls async beforeSend and uses the new one', done => { jest.useFakeTimers(); expect.assertions(1); + const beforeSend = jest.fn( async () => new Promise(resolve => { @@ -824,22 +966,26 @@ describe('BaseClient', () => { }, 1); }), ); - const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }); jest.runOnlyPendingTimers(); + TestBackend.sendEventCalled = (event: Event) => { expect(event.message).toBe('changed2'); }; + setTimeout(() => { done(); }, 5); + jest.runOnlyPendingTimers(); }); test('calls async beforeSend and discards the event', () => { jest.useFakeTimers(); expect.assertions(1); + const beforeSend = jest.fn( async () => new Promise(resolve => { @@ -849,29 +995,34 @@ describe('BaseClient', () => { }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }); jest.runAllTimers(); + expect(TestBackend.instance!.event).toBeUndefined(); }); test('beforeSend gets access to a hint as a second argument', () => { expect.assertions(2); + const beforeSend = jest.fn((event, hint) => ({ ...event, data: hint.data })); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' }); + expect(TestBackend.instance!.event!.message).toBe('hello'); expect((TestBackend.instance!.event! as any).data).toBe('someRandomThing'); }); test('beforeSend records dropped events', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend() { return null; }, }); - const recordLostEventSpy = jest.fn(); jest.spyOn(client, 'getTransport').mockImplementationOnce( () => @@ -887,12 +1038,15 @@ describe('BaseClient', () => { test('eventProcessor can drop the even when it returns null', () => { expect.assertions(3); + const client = new TestClient({ dsn: PUBLIC_DSN }); const captureExceptionSpy = jest.spyOn(client, 'captureException'); const loggerErrorSpy = jest.spyOn(logger, 'error'); const scope = new Scope(); scope.addEventProcessor(() => null); + client.captureEvent({ message: 'hello' }, {}, scope); + expect(TestBackend.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toBeCalled(); expect(loggerErrorSpy).toBeCalledWith(new SentryError('An event processor returned null, will not send event.')); @@ -900,6 +1054,7 @@ describe('BaseClient', () => { test('eventProcessor records dropped events', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const recordLostEventSpy = jest.fn(); @@ -912,6 +1067,7 @@ describe('BaseClient', () => { const scope = new Scope(); scope.addEventProcessor(() => null); + client.captureEvent({ message: 'hello' }, {}, scope); expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'event'); @@ -919,6 +1075,7 @@ describe('BaseClient', () => { test('eventProcessor sends an event and logs when it crashes', () => { expect.assertions(3); + const client = new TestClient({ dsn: PUBLIC_DSN }); const captureExceptionSpy = jest.spyOn(client, 'captureException'); const loggerErrorSpy = jest.spyOn(logger, 'error'); @@ -927,7 +1084,9 @@ describe('BaseClient', () => { scope.addEventProcessor(() => { throw exception; }); + client.captureEvent({ message: 'hello' }, {}, scope); + expect(TestBackend.instance!.event!.exception!.values![0]).toStrictEqual({ type: 'Error', value: 'sorry' }); expect(captureExceptionSpy).toBeCalledWith(exception, { data: { @@ -944,6 +1103,7 @@ describe('BaseClient', () => { test('records events dropped due to sampleRate', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN, sampleRate: 0, @@ -969,39 +1129,46 @@ describe('BaseClient', () => { test('setup each one of them on setupIntegration call', () => { expect.assertions(2); + const client = new TestClient({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()], }); client.setupIntegrations(); + expect(Object.keys((client as any)._integrations).length).toBe(1); expect(client.getIntegration(TestIntegration)).toBeTruthy(); }); test('skips installation if DSN is not provided', () => { expect.assertions(2); + const client = new TestClient({ integrations: [new TestIntegration()], }); client.setupIntegrations(); + expect(Object.keys((client as any)._integrations).length).toBe(0); expect(client.getIntegration(TestIntegration)).toBeFalsy(); }); test('skips installation if enabled is set to false', () => { expect.assertions(2); + const client = new TestClient({ dsn: PUBLIC_DSN, enabled: false, integrations: [new TestIntegration()], }); client.setupIntegrations(); + expect(Object.keys((client as any)._integrations).length).toBe(0); expect(client.getIntegration(TestIntegration)).toBeFalsy(); }); test('skips installation if integrations are already installed', () => { expect.assertions(4); + const client = new TestClient({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()], @@ -1011,12 +1178,14 @@ describe('BaseClient', () => { // it should install the first time, because integrations aren't yet installed... client.setupIntegrations(); + expect(Object.keys((client as any)._integrations).length).toBe(1); expect(client.getIntegration(TestIntegration)).toBeTruthy(); expect(setupIntegrationsHelper).toHaveBeenCalledTimes(1); // ...but it shouldn't try to install a second time client.setupIntegrations(); + expect(setupIntegrationsHelper).toHaveBeenCalledTimes(1); }); }); @@ -1025,6 +1194,7 @@ describe('BaseClient', () => { test('flush', async () => { jest.useRealTimers(); expect.assertions(5); + const client = new TestClient({ dsn: PUBLIC_DSN, enableSend: true, @@ -1036,10 +1206,13 @@ describe('BaseClient', () => { transportInstance.delay = delay; client.captureMessage('test'); + expect(transportInstance).toBeInstanceOf(FakeTransport); expect(transportInstance.sendCalled).toEqual(1); expect(transportInstance.sentCount).toEqual(0); + await client.flush(delay); + expect(transportInstance.sentCount).toEqual(1); expect(transportInstance.sendCalled).toEqual(1); }); @@ -1047,6 +1220,7 @@ describe('BaseClient', () => { test('flush with some events being processed async', async () => { jest.useRealTimers(); expect.assertions(5); + const client = new TestClient({ dsn: PUBLIC_DSN, enableSend: true, @@ -1066,18 +1240,23 @@ describe('BaseClient', () => { client.captureMessage('test async'); client.captureMessage('test non-async'); + expect(transportInstance).toBeInstanceOf(FakeTransport); expect(transportInstance.sendCalled).toEqual(1); expect(transportInstance.sentCount).toEqual(0); + await client.flush(delay); + expect(transportInstance.sentCount).toEqual(2); expect(transportInstance.sendCalled).toEqual(2); + spy.mockRestore(); }); test('close', async () => { jest.useRealTimers(); expect.assertions(2); + const client = new TestClient({ dsn: PUBLIC_DSN, enableSend: true, @@ -1089,7 +1268,9 @@ describe('BaseClient', () => { transportInstance.delay = delay; expect(client.captureMessage('test')).toBeTruthy(); + await client.close(delay); + // Sends after close shouldn't work anymore expect(client.captureMessage('test')).toBeFalsy(); }); @@ -1117,17 +1298,23 @@ describe('BaseClient', () => { describe('captureSession()', () => { test('sends sessions to the backend', () => { expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); const session = new Session({ release: 'test' }); + client.captureSession(session); + expect(TestBackend.instance!.session).toEqual(session); }); test('skips when disabled', () => { expect.assertions(1); + const client = new TestClient({ enabled: false, dsn: PUBLIC_DSN }); const session = new Session({ release: 'test' }); + client.captureSession(session); + expect(TestBackend.instance!.session).toBeUndefined(); }); }); From 97b8798834481cfcba5efe39c66f339cd18b3c8b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 10 Jan 2022 20:31:07 -0800 Subject: [PATCH 8/9] rename field to sdkProcessingMetadata --- packages/core/src/request.ts | 2 +- packages/core/test/lib/request.test.ts | 2 +- packages/hub/src/scope.ts | 8 ++++---- packages/hub/test/scope.test.ts | 8 ++++---- packages/types/src/event.ts | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index 9f6c90ad977a..31c51d4f87a5 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -64,7 +64,7 @@ export function eventToSentryRequest(event: Event, api: APIDetails): SentryReque } // prevent this data from being sent to sentry - delete event.processingMetadata; + delete event.sdkProcessingMetadata; const req: SentryRequest = { body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event), diff --git a/packages/core/test/lib/request.test.ts b/packages/core/test/lib/request.test.ts index 4a27d6e2e708..c75ef32fa0b6 100644 --- a/packages/core/test/lib/request.test.ts +++ b/packages/core/test/lib/request.test.ts @@ -164,7 +164,7 @@ describe('eventToSentryRequest', () => { }); it('removes processing metadata before serializing event', () => { - event.processingMetadata = { dogs: 'are great!' }; + event.sdkProcessingMetadata = { dogs: 'are great!' }; const result = eventToSentryRequest(event, api); const envelope = parseEnvelopeRequest(result); diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 63c13854952c..4faf7c9ef99b 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -79,7 +79,7 @@ export class Scope implements ScopeInterface { * A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get * sent to Sentry */ - protected _processingMetadata?: { [key: string]: any } = {}; + protected _sdkProcessingMetadata?: { [key: string]: any } = {}; /** * Inherit values from the parent scope. @@ -447,7 +447,7 @@ export class Scope implements ScopeInterface { event.breadcrumbs = [...(event.breadcrumbs || []), ...this._breadcrumbs]; event.breadcrumbs = event.breadcrumbs.length > 0 ? event.breadcrumbs : undefined; - event.processingMetadata = this._processingMetadata; + event.sdkProcessingMetadata = this._sdkProcessingMetadata; return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint); } @@ -455,8 +455,8 @@ export class Scope implements ScopeInterface { /** * Add data which will be accessible during event processing but won't get sent to Sentry */ - public setProcessingMetadata(newData: { [key: string]: any }): this { - this._processingMetadata = { ...this._processingMetadata, ...newData }; + public setSDKProcessingMetadata(newData: { [key: string]: any }): this { + this._sdkProcessingMetadata = { ...this._sdkProcessingMetadata, ...newData }; return this; } diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index b087f95d4b2c..c92b544fb3fb 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -131,8 +131,8 @@ describe('Scope', () => { test('setProcessingMetadata', () => { const scope = new Scope(); - scope.setProcessingMetadata({ dogs: 'are great!' }); - expect((scope as any)._processingMetadata.dogs).toEqual('are great!'); + scope.setSDKProcessingMetadata({ dogs: 'are great!' }); + expect((scope as any)._sdkProcessingMetadata.dogs).toEqual('are great!'); }); test('chaining', () => { @@ -206,7 +206,7 @@ describe('Scope', () => { scope.setTransactionName('/abc'); scope.addBreadcrumb({ message: 'test' }); scope.setContext('os', { id: '1' }); - scope.setProcessingMetadata({ dogs: 'are great!' }); + scope.setSDKProcessingMetadata({ dogs: 'are great!' }); const event: Event = {}; return scope.applyToEvent(event).then(processedEvent => { @@ -218,7 +218,7 @@ describe('Scope', () => { expect(processedEvent!.transaction).toEqual('/abc'); expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test'); expect(processedEvent!.contexts).toEqual({ os: { id: '1' } }); - expect(processedEvent!.processingMetadata).toEqual({ dogs: 'are great!' }); + expect(processedEvent!.sdkProcessingMetadata).toEqual({ dogs: 'are great!' }); }); }); diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 51d2ca900f36..ce8698d34364 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -45,7 +45,7 @@ export interface Event { measurements?: Measurements; debug_meta?: DebugMeta; // A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get sent to Sentry - processingMetadata?: { [key: string]: any }; + sdkProcessingMetadata?: { [key: string]: any }; } /** JSDoc */ From 7b6cb1c2b7a8c9d2b99cf66f967c21ac83b2bc61 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 10 Jan 2022 21:00:54 -0800 Subject: [PATCH 9/9] s/any/unknown to make linter happy --- packages/hub/src/scope.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 4faf7c9ef99b..8c3cc70e6b02 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -79,7 +79,7 @@ export class Scope implements ScopeInterface { * A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get * sent to Sentry */ - protected _sdkProcessingMetadata?: { [key: string]: any } = {}; + protected _sdkProcessingMetadata?: { [key: string]: unknown } = {}; /** * Inherit values from the parent scope. @@ -455,7 +455,7 @@ export class Scope implements ScopeInterface { /** * Add data which will be accessible during event processing but won't get sent to Sentry */ - public setSDKProcessingMetadata(newData: { [key: string]: any }): this { + public setSDKProcessingMetadata(newData: { [key: string]: unknown }): this { this._sdkProcessingMetadata = { ...this._sdkProcessingMetadata, ...newData }; return this;