From 7c6bc9756ff00ce03e4408da306bc72191fc5302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 3 Sep 2018 14:58:15 +0200 Subject: [PATCH] feat: Breadcrumbs hint implementation --- .../browser/src/integrations/breadcrumbs.ts | 86 ++++++++++++------- packages/browser/src/integrations/helpers.ts | 14 ++- packages/core/src/base.ts | 14 ++- packages/core/src/interfaces.ts | 14 +-- packages/core/test/lib/base.test.ts | 27 ++++-- packages/hub/src/hub.ts | 13 +-- packages/node/src/integrations/console.ts | 18 ++-- packages/node/src/integrations/http.ts | 25 +++--- packages/types/src/index.ts | 5 ++ 9 files changed, 143 insertions(+), 73 deletions(-) diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index 41ab7379bb54..b4d17cc2c6fc 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -31,15 +31,20 @@ export interface SentryWrappedXMLHttpRequest extends XMLHttpRequest { function addSentryBreadcrumb(serializedData: string): void { // There's always something that can go wrong with deserialization... try { - const data: { [key: string]: any } = deserialize(serializedData); - const exception = data.exception && data.exception.values && data.exception.values[0]; - - getCurrentHub().addBreadcrumb({ - category: 'sentry', - event_id: data.event_id, - level: data.level || Severity.fromString('error'), - message: exception ? `${exception.type ? `${exception.type}: ` : ''}${exception.value}` : data.message, - }); + const event: { [key: string]: any } = deserialize(serializedData); + const exception = event.exception && event.exception.values && event.exception.values[0]; + + getCurrentHub().addBreadcrumb( + { + category: 'sentry', + event_id: event.event_id, + level: event.level || Severity.fromString('error'), + message: exception ? `${exception.type ? `${exception.type}: ` : ''}${exception.value}` : event.message, + }, + { + event, + }, + ); } catch (_oO) { logger.error('Error while adding sentry type breadcrumb'); } @@ -98,17 +103,20 @@ export class Breadcrumbs implements Integration { } // What is wrong with you TypeScript... - const crumb = ({ + const breadcrumbData = ({ category: 'beacon', data, type: 'http', } as any) as { [key: string]: any }; if (!result) { - crumb.level = Severity.Error; + breadcrumbData.level = Severity.Error; } - getCurrentHub().addBreadcrumb(crumb); + getCurrentHub().addBreadcrumb(breadcrumbData, { + input: args, + result, + }); return result; }; @@ -153,7 +161,10 @@ export class Breadcrumbs implements Integration { } } - getCurrentHub().addBreadcrumb(breadcrumbData); + getCurrentHub().addBreadcrumb(breadcrumbData, { + input: args, + level, + }); // this fails for some browsers. :( if (originalConsoleLevel) { @@ -223,20 +234,32 @@ export class Breadcrumbs implements Integration { .apply(global, args) .then((response: Response) => { fetchData.status_code = response.status; - getCurrentHub().addBreadcrumb({ - category: 'fetch', - data: fetchData, - type: 'http', - }); + getCurrentHub().addBreadcrumb( + { + category: 'fetch', + data: fetchData, + type: 'http', + }, + { + input: args, + response, + }, + ); return response; }) .catch((error: Error) => { - getCurrentHub().addBreadcrumb({ - category: 'fetch', - data: fetchData, - level: Severity.Error, - type: 'http', - }); + getCurrentHub().addBreadcrumb( + { + category: 'fetch', + data: fetchData, + level: Severity.Error, + type: 'http', + }, + { + error, + input: args, + }, + ); throw error; }); @@ -385,11 +408,16 @@ export class Breadcrumbs implements Integration { } catch (e) { /* do nothing */ } - getCurrentHub().addBreadcrumb({ - category: 'xhr', - data: xhr.__sentry_xhr__, - type: 'http', - }); + getCurrentHub().addBreadcrumb( + { + category: 'xhr', + data: xhr.__sentry_xhr__, + type: 'http', + }, + { + xhr, + }, + ); } } diff --git a/packages/browser/src/integrations/helpers.ts b/packages/browser/src/integrations/helpers.ts index 2617461c301b..c0bc907b476c 100644 --- a/packages/browser/src/integrations/helpers.ts +++ b/packages/browser/src/integrations/helpers.ts @@ -135,10 +135,16 @@ export function breadcrumbEventHandler(eventName: string): (event: Event) => voi target = ''; } - getCurrentHub().addBreadcrumb({ - category: `ui.${eventName}`, // e.g. ui.click, ui.input - message: target, - }); + getCurrentHub().addBreadcrumb( + { + category: `ui.${eventName}`, // e.g. ui.click, ui.input + message: target, + }, + { + event, + name: eventName, + }, + ); }; } diff --git a/packages/core/src/base.ts b/packages/core/src/base.ts index 54b2e3b65227..d4a7fd8e3d25 100644 --- a/packages/core/src/base.ts +++ b/packages/core/src/base.ts @@ -1,5 +1,13 @@ import { Scope } from '@sentry/hub'; -import { Breadcrumb, SentryEvent, SentryEventHint, SentryResponse, Severity, Status } from '@sentry/types'; +import { + Breadcrumb, + SentryBreadcrumbHint, + SentryEvent, + SentryEventHint, + SentryResponse, + Severity, + Status, +} from '@sentry/types'; import { uuid4 } from '@sentry/utils/misc'; import { truncate } from '@sentry/utils/string'; import { Dsn } from './dsn'; @@ -144,7 +152,7 @@ export abstract class BaseClient implement /** * @inheritDoc */ - public async addBreadcrumb(breadcrumb: Breadcrumb, scope?: Scope): Promise { + public async addBreadcrumb(breadcrumb: Breadcrumb, hint?: SentryBreadcrumbHint, scope?: Scope): Promise { const { beforeBreadcrumb, maxBreadcrumbs = DEFAULT_BREADCRUMBS } = this.getOptions(); if (maxBreadcrumbs <= 0) { @@ -153,7 +161,7 @@ export abstract class BaseClient implement const timestamp = new Date().getTime() / 1000; const mergedBreadcrumb = { timestamp, ...breadcrumb }; - const finalBreadcrumb = beforeBreadcrumb ? beforeBreadcrumb(mergedBreadcrumb) : mergedBreadcrumb; + const finalBreadcrumb = beforeBreadcrumb ? beforeBreadcrumb(mergedBreadcrumb, hint) : mergedBreadcrumb; if (finalBreadcrumb === null) { return; diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 718a1eabcd68..85ee121b72b6 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -3,6 +3,7 @@ import { Breadcrumb, Integration, Repo, + SentryBreadcrumbHint, SentryEvent, SentryEventHint, SentryResponse, @@ -99,7 +100,7 @@ export interface Options { * Returning null will case the event to be dropped. * * @param event The error or message event generated by the SDK. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. * @returns A new event that will be sent | null. */ beforeSend?(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null; @@ -115,7 +116,7 @@ export interface Options { * @param breadcrumb The breadcrumb as created by the SDK. * @returns The breadcrumb that will be added | null. */ - beforeBreadcrumb?(breadcrumb: Breadcrumb): Breadcrumb | null; + beforeBreadcrumb?(breadcrumb: Breadcrumb, hint?: SentryBreadcrumbHint): Breadcrumb | null; } /** @@ -150,7 +151,7 @@ export interface Client { * Captures an exception event and sends it to Sentry. * * @param exception An exception-like object. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. * @param scope An optional scope containing event metadata. * @returns SentryResponse status and event */ @@ -161,7 +162,7 @@ export interface Client { * * @param message The message to send to Sentry. * @param level Define the level of the message. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. * @param scope An optional scope containing event metadata. * @returns SentryResponse status and event */ @@ -171,7 +172,7 @@ export interface Client { * Captures a manually created event and sends it to Sentry. * * @param event The event to send to Sentry. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. * @param scope An optional scope containing event metadata. * @returns SentryResponse status and event */ @@ -185,9 +186,10 @@ export interface Client { * of breadcrumbs, use {@link Options.maxBreadcrumbs}. * * @param breadcrumb The breadcrumb to record. + * @param hint May contain additional information about the original breadcrumb. * @param scope An optional scope to store this breadcrumb in. */ - addBreadcrumb(breadcrumb: Breadcrumb, scope?: Scope): void; + addBreadcrumb(breadcrumb: Breadcrumb, hint?: SentryBreadcrumbHint, scope?: Scope): void; /** Returns the current Dsn. */ getDsn(): Dsn | undefined; diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 4de0651d2d6a..02eaf10b865f 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -84,7 +84,7 @@ describe('BaseClient', () => { const client = new TestClient({}); const scope = new Scope(); scope.addBreadcrumb({ message: 'hello' }, 100); - await client.addBreadcrumb({ message: 'world' }, scope); + await client.addBreadcrumb({ message: 'world' }, undefined, scope); expect(scope.getBreadcrumbs()[1].message).toBe('world'); }); @@ -92,7 +92,7 @@ describe('BaseClient', () => { const client = new TestClient({}); const scope = new Scope(); scope.addBreadcrumb({ message: 'hello' }, 100); - await client.addBreadcrumb({ message: 'world' }, scope); + await client.addBreadcrumb({ message: 'world' }, undefined, scope); expect(scope.getBreadcrumbs()[1].timestamp).toBeGreaterThan(1); }); @@ -100,7 +100,7 @@ describe('BaseClient', () => { const client = new TestClient({ maxBreadcrumbs: 1 }); const scope = new Scope(); scope.addBreadcrumb({ message: 'hello' }, 100); - await client.addBreadcrumb({ message: 'world' }, scope); + await client.addBreadcrumb({ message: 'world' }, undefined, scope); expect(scope.getBreadcrumbs().length).toBe(1); expect(scope.getBreadcrumbs()[0].message).toBe('world'); }); @@ -109,8 +109,8 @@ describe('BaseClient', () => { const client = new TestClient({}); const scope = new Scope(); await Promise.all([ - client.addBreadcrumb({ message: 'hello' }, scope), - client.addBreadcrumb({ message: 'world' }, scope), + client.addBreadcrumb({ message: 'hello' }, undefined, scope), + client.addBreadcrumb({ message: 'world' }, undefined, scope), ]); expect(scope.getBreadcrumbs()).toHaveLength(2); }); @@ -119,7 +119,7 @@ describe('BaseClient', () => { const beforeBreadcrumb = jest.fn(breadcrumb => breadcrumb); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); - await client.addBreadcrumb({ message: 'hello' }, scope); + await client.addBreadcrumb({ message: 'hello' }, undefined, scope); expect(scope.getBreadcrumbs()[0].message).toBe('hello'); }); @@ -127,17 +127,26 @@ describe('BaseClient', () => { const beforeBreadcrumb = jest.fn(() => ({ message: 'changed' })); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); - await client.addBreadcrumb({ message: 'hello' }, scope); + await client.addBreadcrumb({ message: 'hello' }, undefined, scope); expect(scope.getBreadcrumbs()[0].message).toBe('changed'); }); - test('calls shouldAddBreadcrumb and discards the breadcrumb', async () => { + test('calls beforeBreadcrumb and discards the breadcrumb when returned null', async () => { const beforeBreadcrumb = jest.fn(() => null); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); - await client.addBreadcrumb({ message: 'hello' }, scope); + await client.addBreadcrumb({ message: 'hello' }, undefined, scope); expect(scope.getBreadcrumbs().length).toBe(0); }); + + test('calls beforeBreadcrumb gets an access to a hint as a second argument', async () => { + const beforeBreadcrumb = jest.fn((breadcrumb, hint) => ({ ...breadcrumb, data: hint.data })); + const client = new TestClient({ beforeBreadcrumb }); + const scope = new Scope(); + await client.addBreadcrumb({ message: 'hello' }, { data: 'someRandomThing' }, scope); + expect(scope.getBreadcrumbs()[0].message).toBe('hello'); + expect(scope.getBreadcrumbs()[0].data).toBe('someRandomThing'); + }); }); describe('captures', () => { diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 793fe2cb6329..d1d220613c5c 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -1,4 +1,4 @@ -import { Breadcrumb, SentryEvent, SentryEventHint, Severity } from '@sentry/types'; +import { Breadcrumb, SentryBreadcrumbHint, SentryEvent, SentryEventHint, Severity } from '@sentry/types'; import { uuid4 } from '@sentry/utils/misc'; import { Layer } from './interfaces'; import { Scope } from './scope'; @@ -168,7 +168,7 @@ export class Hub { * Captures an exception event and sends it to Sentry. * * @param exception An exception-like object. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. * @returns The generated eventId. */ public captureException(exception: any, hint?: SentryEventHint): string { @@ -185,7 +185,7 @@ export class Hub { * * @param message The message to send to Sentry. * @param level Define the level of the message. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. * @returns The generated eventId. */ public captureMessage(message: string, level?: Severity, hint?: SentryEventHint): string { @@ -201,7 +201,7 @@ export class Hub { * Captures a manually created event and sends it to Sentry. * * @param event The event to send to Sentry. - * @param hint May contain additional informartion about the original exception. + * @param hint May contain additional information about the original exception. */ public captureEvent(event: SentryEvent, hint?: SentryEventHint): string { const eventId = (this._lastEventId = uuid4()); @@ -228,9 +228,10 @@ export class Hub { * user's actions prior to an error or crash. * * @param breadcrumb The breadcrumb to record. + * @param hint May contain additional information about the original breadcrumb. */ - public addBreadcrumb(breadcrumb: Breadcrumb): void { - this.invokeClient('addBreadcrumb', breadcrumb); + public addBreadcrumb(breadcrumb: Breadcrumb, hint?: SentryBreadcrumbHint): void { + this.invokeClient('addBreadcrumb', breadcrumb, { ...hint }); } /** diff --git a/packages/node/src/integrations/console.ts b/packages/node/src/integrations/console.ts index 4e2eee0961d4..9b779ef1d6ad 100644 --- a/packages/node/src/integrations/console.ts +++ b/packages/node/src/integrations/console.ts @@ -1,4 +1,4 @@ -import { addBreadcrumb } from '@sentry/minimal'; +import { getCurrentHub } from '@sentry/hub'; import { Integration, Severity } from '@sentry/types'; import { fill } from '@sentry/utils/object'; import { format } from 'util'; @@ -55,11 +55,17 @@ function consoleWrapper(originalModule: any): any { } return function(): any { - addBreadcrumb({ - category: 'console', - level: sentryLevel, - message: format.apply(undefined, arguments), - }); + getCurrentHub().addBreadcrumb( + { + category: 'console', + level: sentryLevel, + message: format.apply(undefined, arguments), + }, + { + input: [...arguments], + level, + }, + ); originalConsoleLevel.apply(originalModule, arguments); }; diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 8ff60964df64..ff34a82f8b66 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,4 +1,3 @@ -import { addBreadcrumb } from '@sentry/minimal'; import { Integration } from '@sentry/types'; import { fill } from '@sentry/utils/object'; import { ClientRequest, ClientRequestArgs, ServerResponse } from 'http'; @@ -120,16 +119,22 @@ function emitWrapper(origEmit: EventListener): (event: string, response: ServerR const isNotSentryRequest = dsn && this.__ravenBreadcrumbUrl && !this.__ravenBreadcrumbUrl.includes(dsn.host); if (isInterestingEvent && isNotSentryRequest) { - addBreadcrumb({ - category: 'http', - data: { - method: this.method, - status_code: response.statusCode, - - url: this.__ravenBreadcrumbUrl, + getCurrentHub().addBreadcrumb( + { + category: 'http', + data: { + method: this.method, + status_code: response.statusCode, + url: this.__ravenBreadcrumbUrl, + }, + type: 'http', }, - type: 'http', - }); + { + event, + request: this, + response, + }, + ); } return origEmit.apply(this, arguments); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 76acb9d14e03..70d65d523f5a 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -290,3 +290,8 @@ export interface SentryEventHint { originalException?: Error | null; data?: any; } + +/** JSDoc */ +export interface SentryBreadcrumbHint { + [key: string]: any; +}