ref(browser): Extract private methods from LinkedErrors (#4295)

Move out private methods into their own functions as they do not need access to class properties. This helps save on bundle size. Co-authored-by: Armin Ronacher <armin.ronacher@active-4.com>
getsentry · Dec 15, 2021 · 21ea870 · 21ea870
1 parent 4464f31
commit 21ea870
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 71 deletions.
diff --git 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<LinkedErrorsOptions> = {}) {
     this._key = options.key || DEFAULT_KEY;
     this._limit = options.limit || DEFAULT_LIMIT;
   }
@@ -43,36 +48,31 @@ 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;
-      }
-      return event;
+      return getCurrentHub().getIntegration(LinkedErrors) ? _handler(this._key, this._limit, event, hint) : 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
+ */
+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;
   }
+  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
+ */
+export 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]);
 }
diff --git a/packages/browser/test/unit/integrations/linkederrors.test.ts b/packages/browser/test/unit/integrations/linkederrors.test.ts
@@ -1,55 +1,29 @@
 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');
+    it('should bail out if event does not contain exception', () => {
       const event = {
         message: 'foo',
       };
-      const result = linkedErrors._handler(event);
-      expect(spy.called).toBe(false);
+      const result = LinkedErrorsModule._handler('cause', 5, event);
       expect(result).toEqual(event);
     });
 
     it('should bail out if event contains exception, but no hint', () => {
-      const spy = stub(linkedErrors, '_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(result).toEqual(event);
     });
 
-    it('should call walkErrorTree if event contains exception and hint with originalException', () => {
-      const spy = stub(linkedErrors, '_walkErrorTree').callsFake(() => []);
-      const event = {
-        exception: {
-          values: [],
-        },
-        message: 'foo',
-      };
-      const hint = {
-        originalException: new Error('originalException'),
-      };
-      linkedErrors._handler(event, hint);
-      expect(spy.calledOnce).toBe(true);
-    });
-
     it('should recursively walk error to find linked exceptions and assign them to the event', async () => {
       const three: ExtendedError = new SyntaxError('three');
 
@@ -62,7 +36,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 +55,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 +66,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,20 +84,17 @@ 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');
       one.cause = two;
       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);