From 7972e082ae954b2ead09e86c0cb2162ee19d86e4 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 14 Dec 2021 21:32:04 -0500 Subject: [PATCH 1/3] ref(browser): Extract private methods from LinkedErrors Move out private methods into their own functions as they do not need access to class properties. This helps save on bundle size. --- .../browser/src/integrations/linkederrors.ts | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts index dad3b2759fa9..bab338497e1b 100644 --- a/packages/browser/src/integrations/linkederrors.ts +++ b/packages/browser/src/integrations/linkederrors.ts @@ -8,6 +8,11 @@ import { computeStackTrace } from '../tracekit'; const DEFAULT_KEY = 'cause'; const DEFAULT_LIMIT = 5; +interface LinkedErrorsOptions { + key: string; + limit: number; +} + /** Adds SDK info to an event. */ export class LinkedErrors implements Integration { /** @@ -23,17 +28,17 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - private readonly _key: string; + private readonly _key: LinkedErrorsOptions['key']; /** * @inheritDoc */ - private readonly _limit: number; + private readonly _limit: LinkedErrorsOptions['limit']; /** * @inheritDoc */ - public constructor(options: { key?: string; limit?: number } = {}) { + public constructor(options: Partial = {}) { this._key = options.key || DEFAULT_KEY; this._limit = options.limit || DEFAULT_LIMIT; } @@ -43,36 +48,34 @@ export class LinkedErrors implements Integration { */ public setupOnce(): void { addGlobalEventProcessor((event: Event, hint?: EventHint) => { - const self = getCurrentHub().getIntegration(LinkedErrors); - if (self) { - const handler = self._handler && self._handler.bind(self); - return typeof handler === 'function' ? handler(event, hint) : event; + if (getCurrentHub().getIntegration(LinkedErrors)) { + return typeof handler === 'function' ? handler(this._key, this._limit, event, hint) : event; } return event; }); } +} - /** - * @inheritDoc - */ - private _handler(event: Event, hint?: EventHint): Event | null { - if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) { - return event; - } - const linkedErrors = this._walkErrorTree(hint.originalException as ExtendedError, this._key); - event.exception.values = [...linkedErrors, ...event.exception.values]; +/** + * @inheritDoc + */ +function handler(key: string, limit: number, event: Event, hint?: EventHint): Event | null { + if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) { return event; } + const linkedErrors = _walkErrorTree(limit, hint.originalException as ExtendedError, key); + event.exception.values = [...linkedErrors, ...event.exception.values]; + return event; +} - /** - * @inheritDoc - */ - private _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): Exception[] { - if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) { - return stack; - } - const stacktrace = computeStackTrace(error[key]); - const exception = exceptionFromStacktrace(stacktrace); - return this._walkErrorTree(error[key], key, [exception, ...stack]); +/** + * JSDOC + */ +function _walkErrorTree(limit: number, error: ExtendedError, key: string, stack: Exception[] = []): Exception[] { + if (!isInstanceOf(error[key], Error) || stack.length + 1 >= limit) { + return stack; } + const stacktrace = computeStackTrace(error[key]); + const exception = exceptionFromStacktrace(stacktrace); + return _walkErrorTree(limit, error[key], key, [exception, ...stack]); } From b26227f2fa45cef0761cd1e49ed3ed73e1f85687 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 11:44:48 -0500 Subject: [PATCH 2/3] fix tests --- .../browser/src/integrations/linkederrors.ts | 9 ++-- .../unit/integrations/linkederrors.test.ts | 46 +++++++------------ 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts index bab338497e1b..0a97d11cd051 100644 --- a/packages/browser/src/integrations/linkederrors.ts +++ b/packages/browser/src/integrations/linkederrors.ts @@ -48,10 +48,7 @@ export class LinkedErrors implements Integration { */ public setupOnce(): void { addGlobalEventProcessor((event: Event, hint?: EventHint) => { - if (getCurrentHub().getIntegration(LinkedErrors)) { - return typeof handler === 'function' ? handler(this._key, this._limit, event, hint) : event; - } - return event; + return getCurrentHub().getIntegration(LinkedErrors) ? _handler(this._key, this._limit, event, hint) : event; }); } } @@ -59,7 +56,7 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ -function handler(key: string, limit: number, event: Event, hint?: EventHint): Event | null { +export function _handler(key: string, limit: number, event: Event, hint?: EventHint): Event | null { if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) { return event; } @@ -71,7 +68,7 @@ function handler(key: string, limit: number, event: Event, hint?: EventHint): Ev /** * JSDOC */ -function _walkErrorTree(limit: number, error: ExtendedError, key: string, stack: Exception[] = []): Exception[] { +export function _walkErrorTree(limit: number, error: ExtendedError, key: string, stack: Exception[] = []): Exception[] { if (!isInstanceOf(error[key], Error) || stack.length + 1 >= limit) { return stack; } diff --git a/packages/browser/test/unit/integrations/linkederrors.test.ts b/packages/browser/test/unit/integrations/linkederrors.test.ts index 15a448a47570..17e09f0ff101 100644 --- a/packages/browser/test/unit/integrations/linkederrors.test.ts +++ b/packages/browser/test/unit/integrations/linkederrors.test.ts @@ -1,42 +1,35 @@ import { ExtendedError } from '@sentry/types'; -import { stub } from 'sinon'; import { BrowserBackend } from '../../../src/backend'; -import { LinkedErrors } from '../../../src/integrations/linkederrors'; - -let linkedErrors: any; +import * as LinkedErrorsModule from '../../../src/integrations/linkederrors'; describe('LinkedErrors', () => { - beforeEach(() => { - linkedErrors = new LinkedErrors(); - }); - describe('handler', () => { it('should bail out if event doesnt contain exception', () => { - const spy = stub(linkedErrors, '_walkErrorTree'); + const spy = jest.spyOn(LinkedErrorsModule, '_walkErrorTree'); const event = { message: 'foo', }; - const result = linkedErrors._handler(event); - expect(spy.called).toBe(false); + const result = LinkedErrorsModule._handler('cause', 5, event); + expect(spy).toHaveBeenCalledTimes(0); expect(result).toEqual(event); }); it('should bail out if event contains exception, but no hint', () => { - const spy = stub(linkedErrors, '_walkErrorTree'); + const spy = jest.spyOn(LinkedErrorsModule, '_walkErrorTree'); const event = { exception: { values: [], }, message: 'foo', }; - const result = linkedErrors._handler(event); - expect(spy.called).toBe(false); + const result = LinkedErrorsModule._handler('cause', 5, event); + expect(spy).toHaveBeenCalledTimes(0); expect(result).toEqual(event); }); it('should call walkErrorTree if event contains exception and hint with originalException', () => { - const spy = stub(linkedErrors, '_walkErrorTree').callsFake(() => []); + const spy = jest.spyOn(LinkedErrorsModule, '_walkErrorTree').mockImplementation(() => []); const event = { exception: { values: [], @@ -46,8 +39,8 @@ describe('LinkedErrors', () => { const hint = { originalException: new Error('originalException'), }; - linkedErrors._handler(event, hint); - expect(spy.calledOnce).toBe(true); + LinkedErrorsModule._handler('cause', 5, event, hint); + expect(spy).toHaveBeenCalledTimes(1); }); it('should recursively walk error to find linked exceptions and assign them to the event', async () => { @@ -62,7 +55,7 @@ describe('LinkedErrors', () => { const originalException = one; const backend = new BrowserBackend({}); return backend.eventFromException(originalException).then(event => { - const result = linkedErrors._handler(event, { + const result = LinkedErrorsModule._handler('cause', 5, event, { originalException, }); @@ -81,10 +74,6 @@ describe('LinkedErrors', () => { }); it('should allow to change walk key', async () => { - linkedErrors = new LinkedErrors({ - key: 'reason', - }); - const three: ExtendedError = new SyntaxError('three'); const two: ExtendedError = new TypeError('two'); @@ -96,7 +85,7 @@ describe('LinkedErrors', () => { const originalException = one; const backend = new BrowserBackend({}); return backend.eventFromException(originalException).then(event => { - const result = linkedErrors._handler(event, { + const result = LinkedErrorsModule._handler('reason', 5, event, { originalException, }); @@ -114,10 +103,6 @@ describe('LinkedErrors', () => { }); it('should allow to change stack size limit', async () => { - linkedErrors = new LinkedErrors({ - limit: 2, - }); - const one: ExtendedError = new Error('one'); const two: ExtendedError = new TypeError('two'); const three: ExtendedError = new SyntaxError('three'); @@ -125,9 +110,10 @@ describe('LinkedErrors', () => { two.cause = three; const backend = new BrowserBackend({}); - return backend.eventFromException(one).then(event => { - const result = linkedErrors._handler(event, { - originalException: one, + const originalException = one; + return backend.eventFromException(originalException).then(event => { + const result = LinkedErrorsModule._handler('cause', 2, event, { + originalException, }); expect(result.exception.values.length).toBe(2); From bb64e4121eedf08c25d9fd1b184265f4c916186b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 15 Dec 2021 12:08:30 -0500 Subject: [PATCH 3/3] change tests --- .../unit/integrations/linkederrors.test.ts | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/packages/browser/test/unit/integrations/linkederrors.test.ts b/packages/browser/test/unit/integrations/linkederrors.test.ts index 17e09f0ff101..1531aa6f77ed 100644 --- a/packages/browser/test/unit/integrations/linkederrors.test.ts +++ b/packages/browser/test/unit/integrations/linkederrors.test.ts @@ -5,18 +5,15 @@ import * as LinkedErrorsModule from '../../../src/integrations/linkederrors'; describe('LinkedErrors', () => { describe('handler', () => { - it('should bail out if event doesnt contain exception', () => { - const spy = jest.spyOn(LinkedErrorsModule, '_walkErrorTree'); + it('should bail out if event does not contain exception', () => { const event = { message: 'foo', }; const result = LinkedErrorsModule._handler('cause', 5, event); - expect(spy).toHaveBeenCalledTimes(0); expect(result).toEqual(event); }); it('should bail out if event contains exception, but no hint', () => { - const spy = jest.spyOn(LinkedErrorsModule, '_walkErrorTree'); const event = { exception: { values: [], @@ -24,25 +21,9 @@ describe('LinkedErrors', () => { message: 'foo', }; const result = LinkedErrorsModule._handler('cause', 5, event); - expect(spy).toHaveBeenCalledTimes(0); expect(result).toEqual(event); }); - it('should call walkErrorTree if event contains exception and hint with originalException', () => { - const spy = jest.spyOn(LinkedErrorsModule, '_walkErrorTree').mockImplementation(() => []); - const event = { - exception: { - values: [], - }, - message: 'foo', - }; - const hint = { - originalException: new Error('originalException'), - }; - LinkedErrorsModule._handler('cause', 5, event, hint); - expect(spy).toHaveBeenCalledTimes(1); - }); - it('should recursively walk error to find linked exceptions and assign them to the event', async () => { const three: ExtendedError = new SyntaxError('three');