diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 34ecedad4925..01638900d2e4 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -641,7 +641,7 @@ export abstract class BaseClient implements Client { .then(prepared => { if (prepared === null) { this.recordDroppedEvent('event_processor', event.type || 'error'); - throw new SentryError('An event processor returned null, will not send event.', 'log'); + throw new SentryError('An event processor returned `null`, will not send event.', 'log'); } const isInternalException = hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true; diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index beb4d412a2f7..edd43c7cb142 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -887,7 +887,7 @@ describe('BaseClient', () => { expect(capturedEvent).toEqual(normalizedTransaction); }); - test('calls beforeSend and uses original event without any changes', () => { + test('calls `beforeSend` and uses original event without any changes', () => { expect.assertions(1); const beforeSend = jest.fn(event => event); @@ -899,7 +899,19 @@ describe('BaseClient', () => { expect(TestClient.instance!.event!.message).toBe('hello'); }); - test('calls beforeSend and uses the new one', () => { + test('calls `beforeSendTransaction` and uses original event without any changes', () => { + expect.assertions(1); + + const beforeSendTransaction = jest.fn(event => event); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + + expect(TestClient.instance!.event!.transaction).toBe('/dogs/are/great'); + }); + + test('calls `beforeSend` and uses the modified event', () => { expect.assertions(1); const beforeSend = jest.fn(() => ({ message: 'changed1' })); @@ -911,23 +923,50 @@ describe('BaseClient', () => { expect(TestClient.instance!.event!.message).toBe('changed1'); }); - test('calls beforeSend and discards the event', () => { - expect.assertions(3); + test('calls `beforeSendTransaction` and uses the modified event', () => { + expect.assertions(1); + + const beforeSendTransaction = jest.fn(event => { + event.transaction = '/adopt/dont/shop'; + return event; + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + + expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop'); + }); + + test('calls `beforeSend` and discards the event', () => { + expect.assertions(2); const beforeSend = jest.fn(() => null); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); const client = new TestClient(options); - const captureExceptionSpy = jest.spyOn(client, 'captureException'); const loggerWarnSpy = jest.spyOn(logger, 'log'); client.captureEvent({ message: 'hello' }); expect(TestClient.instance!.event).toBeUndefined(); - expect(captureExceptionSpy).not.toBeCalled(); expect(loggerWarnSpy).toBeCalledWith('`beforeSend` returned `null`, will not send event.'); }); - test('calls beforeSend and log info about invalid return value', () => { + test('calls `beforeSendTransaction` and discards the event', () => { + expect.assertions(2); + + const beforeSendTransaction = jest.fn(() => null); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + const loggerWarnSpy = jest.spyOn(logger, 'log'); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + + expect(TestClient.instance!.event).toBeUndefined(); + expect(loggerWarnSpy).toBeCalledWith('`beforeSendTransaction` returned `null`, will not send event.'); + }); + + test('calls `beforeSend` and logs info about invalid return value', () => { const invalidValues = [undefined, false, true, [], 1]; expect.assertions(invalidValues.length * 2); @@ -940,14 +979,32 @@ describe('BaseClient', () => { client.captureEvent({ message: 'hello' }); + expect(TestClient.instance!.event).toBeUndefined(); + expect(loggerWarnSpy).toBeCalledWith(new SentryError('`beforeSend` must return `null` or a valid event.')); + } + }); + + test('calls `beforeSendTransaction` and logs info about invalid return value', () => { + const invalidValues = [undefined, false, true, [], 1]; + expect.assertions(invalidValues.length * 2); + + for (const val of invalidValues) { + const beforeSendTransaction = jest.fn(() => val); + // @ts-ignore we need to test regular-js behavior + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + const loggerWarnSpy = jest.spyOn(logger, 'warn'); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + expect(TestClient.instance!.event).toBeUndefined(); expect(loggerWarnSpy).toBeCalledWith( - new SentryError('`beforeSend` method has to return `null` or a valid event.'), + new SentryError('`beforeSendTransaction` must return `null` or a valid event.'), ); } }); - test('calls async beforeSend and uses original event without any changes', done => { + test('calls async `beforeSend` and uses original event without any changes', done => { jest.useFakeTimers(); expect.assertions(1); @@ -976,18 +1033,47 @@ describe('BaseClient', () => { jest.runOnlyPendingTimers(); }); - test('calls async beforeSend and uses the new one', done => { + test('calls async `beforeSendTransaction` and uses original event without any changes', done => { jest.useFakeTimers(); expect.assertions(1); - const beforeSend = jest.fn( - async () => + const beforeSendTransaction = jest.fn( + async event => new Promise(resolve => { setTimeout(() => { - resolve({ message: 'changed2' }); + resolve(event); }, 1); }), ); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + jest.runOnlyPendingTimers(); + + TestClient.sendEventCalled = (event: Event) => { + expect(event.transaction).toBe('/dogs/are/great'); + }; + + setTimeout(() => { + done(); + }, 5); + + jest.runOnlyPendingTimers(); + }); + + test('calls async `beforeSend` and uses the modified event', done => { + jest.useFakeTimers(); + expect.assertions(1); + + const beforeSend = jest.fn(async event => { + event.message = 'changed2'; + return new Promise(resolve => { + setTimeout(() => { + resolve(event); + }, 1); + }); + }); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); const client = new TestClient(options); @@ -1005,7 +1091,36 @@ describe('BaseClient', () => { jest.runOnlyPendingTimers(); }); - test('calls async beforeSend and discards the event', () => { + test('calls async `beforeSendTransaction` and uses the modified event', done => { + jest.useFakeTimers(); + expect.assertions(1); + + const beforeSendTransaction = jest.fn(async event => { + event.transaction = '/adopt/dont/shop'; + return new Promise(resolve => { + setTimeout(() => { + resolve(event); + }, 1); + }); + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + jest.runOnlyPendingTimers(); + + TestClient.sendEventCalled = (event: Event) => { + expect(event.transaction).toBe('/adopt/dont/shop'); + }; + + setTimeout(() => { + done(); + }, 5); + + jest.runOnlyPendingTimers(); + }); + + test('calls async `beforeSend` and discards the event', () => { jest.useFakeTimers(); expect.assertions(1); @@ -1026,7 +1141,28 @@ describe('BaseClient', () => { expect(TestClient.instance!.event).toBeUndefined(); }); - test('beforeSend gets access to a hint as a second argument', () => { + test('calls async `beforeSendTransaction` and discards the event', () => { + jest.useFakeTimers(); + expect.assertions(1); + + const beforeSendTransaction = jest.fn( + async () => + new Promise(resolve => { + setTimeout(() => { + resolve(null); + }); + }), + ); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + jest.runAllTimers(); + + expect(TestClient.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 })); @@ -1036,10 +1172,26 @@ describe('BaseClient', () => { client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' }); expect(TestClient.instance!.event!.message).toBe('hello'); - expect((TestClient.instance!.event! as any).data).toBe('someRandomThing'); + expect((TestClient.instance!.event! as any).data).toEqual('someRandomThing'); + }); + + test('`beforeSendTransaction` gets access to a hint as a second argument', () => { + expect.assertions(2); + + const beforeSendTransaction = jest.fn((event, hint) => ({ ...event, data: hint.data })); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent( + { transaction: '/dogs/are/great', type: 'transaction' }, + { data: { dogs: 'yes', cats: 'maybe' } }, + ); + + expect(TestClient.instance!.event!.transaction).toBe('/dogs/are/great'); + expect((TestClient.instance!.event! as any).data).toEqual({ dogs: 'yes', cats: 'maybe' }); }); - test('beforeSend records dropped events', () => { + test('`beforeSend` records dropped events', () => { expect.assertions(1); const client = new TestClient( @@ -1058,11 +1210,29 @@ describe('BaseClient', () => { expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error'); }); - test('eventProcessor can drop the even when it returns null', () => { - expect.assertions(3); + test('`beforeSendTransaction` records dropped events', () => { + expect.assertions(1); + + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + beforeSendTransaction() { + return null; + }, + }), + ); + + const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent'); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + + expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction'); + }); + + test('event processor can drop error event when it returns `null`', () => { + expect.assertions(2); const client = new TestClient(getDefaultTestClientOptions({ dsn: PUBLIC_DSN })); - const captureExceptionSpy = jest.spyOn(client, 'captureException'); const loggerLogSpy = jest.spyOn(logger, 'log'); const scope = new Scope(); scope.addEventProcessor(() => null); @@ -1070,11 +1240,24 @@ describe('BaseClient', () => { client.captureEvent({ message: 'hello' }, {}, scope); expect(TestClient.instance!.event).toBeUndefined(); - expect(captureExceptionSpy).not.toBeCalled(); - expect(loggerLogSpy).toBeCalledWith('An event processor returned null, will not send event.'); + expect(loggerLogSpy).toBeCalledWith('An event processor returned `null`, will not send event.'); + }); + + test('event processor can drop transaction event when it returns `null`', () => { + expect.assertions(2); + + const client = new TestClient(getDefaultTestClientOptions({ dsn: PUBLIC_DSN })); + const loggerLogSpy = jest.spyOn(logger, 'log'); + const scope = new Scope(); + scope.addEventProcessor(() => null); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }, {}, scope); + + expect(TestClient.instance!.event).toBeUndefined(); + expect(loggerLogSpy).toBeCalledWith('An event processor returned `null`, will not send event.'); }); - test('eventProcessor records dropped events', () => { + test('event processor records dropped error events', () => { expect.assertions(1); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); @@ -1090,24 +1273,28 @@ describe('BaseClient', () => { expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error'); }); - test('mutating transaction name with event processors sets transaction name change metadata', () => { + test('event processor records dropped transaction events', () => { + expect.assertions(1); + + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent'); + + const scope = new Scope(); + scope.addEventProcessor(() => null); + + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }, {}, scope); + + expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction'); + }); + + test('mutating transaction name with event processor sets transaction name change metadata', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableSend: true }); const client = new TestClient(options); const transaction: Event = { - contexts: { - trace: { - op: 'pageload', - span_id: 'a3df84a60c2e4e76', - trace_id: '86f39e84263a4de99c326acab3bfe3bd', - }, - }, - environment: 'production', - event_id: '972f45b826a248bba98e990878a177e1', - spans: [], - start_timestamp: 1591603196.614865, - timestamp: 1591603196.728485, - transaction: 'initialName', + transaction: '/dogs/are/great', type: 'transaction', transaction_info: { source: 'url', @@ -1118,12 +1305,44 @@ describe('BaseClient', () => { const scope = new Scope(); scope.addEventProcessor(event => { - event.transaction = 'updatedName'; + event.transaction = '/adopt/dont/shop'; return event; }); client.captureEvent(transaction, {}, scope); - expect(TestClient.instance!.event!.transaction).toEqual('updatedName'); + expect(TestClient.instance!.event!.transaction).toEqual('/adopt/dont/shop'); + expect(TestClient.instance!.event!.transaction_info).toEqual({ + source: 'custom', + changes: [ + { + propagations: 3, + source: 'custom', + timestamp: expect.any(Number), + }, + ], + propagations: 3, + }); + }); + + test('mutating transaction name with `beforeSendTransaction` sets transaction name change metadata', () => { + const beforeSendTransaction = jest.fn(event => { + event.transaction = '/adopt/dont/shop'; + return event; + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + + client.captureEvent({ + transaction: '/dogs/are/great', + type: 'transaction', + transaction_info: { + source: 'url', + changes: [], + propagations: 3, + }, + }); + + expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop'); expect(TestClient.instance!.event!.transaction_info).toEqual({ source: 'custom', changes: [ @@ -1184,7 +1403,7 @@ describe('BaseClient', () => { global.__SENTRY__ = {}; }); - test('setup each one of them on setupIntegration call', () => { + test('setup each one of them on `setupIntegrations` call', () => { expect.assertions(2); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()] }); @@ -1206,7 +1425,7 @@ describe('BaseClient', () => { expect(client.getIntegration(TestIntegration)).toBeFalsy(); }); - test('skips installation if enabled is set to false', () => { + test('skips installation if `enabled` is set to `false`', () => { expect.assertions(2); const options = getDefaultTestClientOptions({ @@ -1373,7 +1592,7 @@ describe('BaseClient', () => { }); describe('recordDroppedEvent()/_clearOutcomes()', () => { - test('record and return outcomes', () => { + it('records and returns outcomes', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); @@ -1381,7 +1600,8 @@ describe('BaseClient', () => { client.recordDroppedEvent('ratelimit_backoff', 'error'); client.recordDroppedEvent('network_error', 'transaction'); client.recordDroppedEvent('network_error', 'transaction'); - client.recordDroppedEvent('before_send', 'session'); + client.recordDroppedEvent('before_send', 'error'); + client.recordDroppedEvent('before_send', 'transaction'); client.recordDroppedEvent('event_processor', 'attachment'); client.recordDroppedEvent('network_error', 'transaction'); @@ -1401,7 +1621,12 @@ describe('BaseClient', () => { }, { reason: 'before_send', - category: 'session', + category: 'error', + quantity: 1, + }, + { + reason: 'before_send', + category: 'transaction', quantity: 1, }, { @@ -1413,7 +1638,7 @@ describe('BaseClient', () => { ); }); - test('to clear outcomes', () => { + it('clears outcomes', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); diff --git a/packages/core/test/lib/hint.test.ts b/packages/core/test/lib/hint.test.ts index b0affcada15b..d19dd34fe263 100644 --- a/packages/core/test/lib/hint.test.ts +++ b/packages/core/test/lib/hint.test.ts @@ -20,7 +20,7 @@ describe('Hint', () => { }); describe('attachments', () => { - test('can be mutated in beforeSend', () => { + test('can be mutated in `beforeSend`', () => { expect.assertions(1); const options = getDefaultTestClientOptions({ @@ -38,7 +38,26 @@ describe('Hint', () => { expect(hint).toEqual({ attachments: [{ filename: 'another.file', data: 'more text' }] }); }); - test('gets passed through to beforeSend and can be further mutated', () => { + test('can be mutated in `beforeSendTransaction`', () => { + expect.assertions(1); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + beforeSendTransaction: (event, hint) => { + hint.attachments = [...(hint.attachments || []), { filename: 'another.file', data: 'more text' }]; + return event; + }, + enableSend: true, + }); + + const client = new TestClient(options); + client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); + + const [, hint] = sendEvent.mock.calls[0]; + expect(hint).toEqual({ attachments: [{ filename: 'another.file', data: 'more text' }] }); + }); + + test('gets passed through to `beforeSend` and can be further mutated', () => { expect.assertions(1); const options = getDefaultTestClientOptions({ @@ -61,6 +80,32 @@ describe('Hint', () => { }); }); + test('gets passed through to `beforeSendTransaction` and can be further mutated', () => { + expect.assertions(1); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + beforeSendTransaction: (event, hint) => { + hint.attachments = [...(hint.attachments || []), { filename: 'another.file', data: 'more text' }]; + return event; + }, + }); + + const client = new TestClient(options); + client.captureEvent( + { transaction: '/dogs/are/great', type: 'transaction' }, + { attachments: [{ filename: 'some-file.txt', data: 'Hello' }] }, + ); + + const [, hint] = sendEvent.mock.calls[0]; + expect(hint).toEqual({ + attachments: [ + { filename: 'some-file.txt', data: 'Hello' }, + { filename: 'another.file', data: 'more text' }, + ], + }); + }); + test('can be mutated by an integration via event processor', () => { expect.assertions(1); @@ -76,7 +121,7 @@ describe('Hint', () => { expect(hint?.attachments).toEqual([{ filename: 'integration.file', data: 'great content!' }]); }); - test('get copied from scope to hint', () => { + test('gets copied from scope to hint', () => { expect.assertions(1); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); diff --git a/packages/nextjs/test/index.client.test.ts b/packages/nextjs/test/index.client.test.ts index 699b2fac1f0a..8b61ef7ff1ba 100644 --- a/packages/nextjs/test/index.client.test.ts +++ b/packages/nextjs/test/index.client.test.ts @@ -93,7 +93,7 @@ describe('Client init()', () => { expect(transportSend).not.toHaveBeenCalled(); expect(captureEvent.mock.results[0].value).toBeUndefined(); - expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned null, will not send event.'); + expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned `null`, will not send event.'); }); describe('integrations', () => { diff --git a/packages/nextjs/test/index.server.test.ts b/packages/nextjs/test/index.server.test.ts index 08d4178deb5b..d432b61eb16e 100644 --- a/packages/nextjs/test/index.server.test.ts +++ b/packages/nextjs/test/index.server.test.ts @@ -111,7 +111,7 @@ describe('Server init()', () => { await SentryNode.flush(); expect(transportSend).not.toHaveBeenCalled(); - expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned null, will not send event.'); + expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned `null`, will not send event.'); }); it("initializes both global hub and domain hub when there's an active domain", () => {