[fix] Unify browser and node linked errors, correct order, fix limit …

…handling (#1704)
getsentry · Oct 30, 2018 · 418ae83 · 418ae83
1 parent c69749d
commit 418ae83
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 61 deletions.
diff --git a/packages/browser/test/integrations/linkederrors.test.ts b/packages/browser/test/integrations/linkederrors.test.ts
@@ -102,7 +102,6 @@ describe('LinkedErrors', () => {
         originalException,
       });
 
-      // It shouldn't include root exception, as it's already processed in the event by the main error handler
       expect(result!.exception!.values!.length).equal(3);
       expect(result!.exception!.values![0].type).equal('SyntaxError');
       expect(result!.exception!.values![0].value).equal('three');
@@ -120,28 +119,24 @@ describe('LinkedErrors', () => {
         limit: 2,
       });
 
-      const three: ExtendedError = new SyntaxError('three');
-
-      const two: ExtendedError = new TypeError('two');
-      two.cause = three;
-
       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 originalException = one;
       const backend = new BrowserBackend({});
-      const event = await backend.eventFromException(originalException);
+      const event = await backend.eventFromException(one);
       const result = linkedErrors.handler(event, {
-        originalException,
+        originalException: one,
       });
 
-      // It shouldn't include root exception, as it's already processed in the event by the main error handler
       expect(result!.exception!.values!.length).equal(2);
-      expect(result!.exception!.values![0].type).equal('Error');
-      expect(result!.exception!.values![0].value).equal('one');
+      expect(result!.exception!.values![0].type).equal('TypeError');
+      expect(result!.exception!.values![0].value).equal('two');
       expect(result!.exception!.values![0].stacktrace).to.have.property('frames');
-      expect(result!.exception!.values![1].type).equal('TypeError');
-      expect(result!.exception!.values![1].value).equal('two');
+      expect(result!.exception!.values![1].type).equal('Error');
+      expect(result!.exception!.values![1].value).equal('one');
       expect(result!.exception!.values![1].stacktrace).to.have.property('frames');
     });
   });

diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts
@@ -62,7 +62,7 @@ export class LinkedErrors implements Integration {
       return event;
     }
     const linkedErrors = await this.walkErrorTree(hint.originalException, this.key);
-    event.exception.values = [...event.exception.values, ...linkedErrors];
+    event.exception.values = [...linkedErrors, ...event.exception.values];
     return event;
   }
 
@@ -74,10 +74,10 @@ export class LinkedErrors implements Integration {
     key: string,
     stack: SentryException[] = [],
   ): Promise<SentryException[]> {
-    if (!(error[key] instanceof Error) || stack.length >= this.limit) {
+    if (!(error[key] instanceof Error) || stack.length + 1 >= this.limit) {
       return stack;
     }
     const exception = await getExceptionFromError(error[key]);
-    return this.walkErrorTree(error[key], key, [...stack, exception]);
+    return this.walkErrorTree(error[key], key, [exception, ...stack]);
   }
 }
diff --git a/packages/node/test/integrations/linkederrors.test.ts b/packages/node/test/integrations/linkederrors.test.ts
@@ -1,3 +1,4 @@
+import { NodeBackend } from '../../src';
 import { LinkedErrors } from '../../src/integrations/linkederrors';
 
 let linkedErrors: LinkedErrors;
@@ -24,12 +25,9 @@ describe('LinkedErrors', () => {
 
     it('should bail out if event contains exception, but no hint', async () => {
       const spy = jest.spyOn(linkedErrors, 'walkErrorTree');
-      const event = {
-        exception: {
-          values: [],
-        },
-        message: 'foo',
-      };
+      const one = new Error('originalException');
+      const backend = new NodeBackend({});
+      const event = await backend.eventFromException(one);
       const result = await linkedErrors.handler(event);
       expect(spy.mock.calls.length).toEqual(0);
       expect(result).toEqual(event);
@@ -42,79 +40,92 @@ describe('LinkedErrors', () => {
             resolve([]);
           }),
       );
-      const event = {
-        exception: {
-          values: [],
-        },
-        message: 'foo',
-      };
-      const hint = {
-        originalException: new Error('originalException'),
-      };
-      await linkedErrors.handler(event, hint);
+      const one = new Error('originalException');
+      const backend = new NodeBackend({});
+      const event = await backend.eventFromException(one);
+      await linkedErrors.handler(event, {
+        originalException: one,
+      });
       expect(spy.mock.calls.length).toEqual(1);
     });
 
     it('should recursively walk error to find linked exceptions and assign them to the event', async () => {
-      const event = {
-        exception: {
-          values: [],
-        },
-        message: 'foo',
-      };
-
       const one: ExtendedError = new Error('one');
       const two: ExtendedError = new TypeError('two');
       const three: ExtendedError = new SyntaxError('three');
-
-      const originalException = one;
       one.cause = two;
       two.cause = three;
 
+      const backend = new NodeBackend({});
+      const event = await backend.eventFromException(one);
       const result = await linkedErrors.handler(event, {
-        originalException,
+        originalException: one,
       });
 
-      // It shouldn't include root exception, as it's already processed in the event by the main error handler
-      expect(result!.exception!.values!.length).toEqual(2);
-      expect(result!.exception!.values![0].type).toEqual('TypeError');
-      expect(result!.exception!.values![0].value).toEqual('two');
+      expect(result!.exception!.values!.length).toEqual(3);
+      expect(result!.exception!.values![0].type).toEqual('SyntaxError');
+      expect(result!.exception!.values![0].value).toEqual('three');
       expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames');
-      expect(result!.exception!.values![1].type).toEqual('SyntaxError');
-      expect(result!.exception!.values![1].value).toEqual('three');
+      expect(result!.exception!.values![1].type).toEqual('TypeError');
+      expect(result!.exception!.values![1].value).toEqual('two');
       expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames');
+      expect(result!.exception!.values![2].type).toEqual('Error');
+      expect(result!.exception!.values![2].value).toEqual('one');
+      expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames');
     });
 
     it('should allow to change walk key', async () => {
       linkedErrors = new LinkedErrors({
         key: 'reason',
       });
-      const event = {
-        exception: {
-          values: [],
-        },
-        message: 'foo',
-      };
 
       const one: ExtendedError = new Error('one');
       const two: ExtendedError = new TypeError('two');
       const three: ExtendedError = new SyntaxError('three');
-
-      const originalException = one;
       one.reason = two;
       two.reason = three;
 
+      const backend = new NodeBackend({});
+      const event = await backend.eventFromException(one);
+      const result = await linkedErrors.handler(event, {
+        originalException: one,
+      });
+
+      expect(result!.exception!.values!.length).toEqual(3);
+      expect(result!.exception!.values![0].type).toEqual('SyntaxError');
+      expect(result!.exception!.values![0].value).toEqual('three');
+      expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames');
+      expect(result!.exception!.values![1].type).toEqual('TypeError');
+      expect(result!.exception!.values![1].value).toEqual('two');
+      expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames');
+      expect(result!.exception!.values![2].type).toEqual('Error');
+      expect(result!.exception!.values![2].value).toEqual('one');
+      expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames');
+    });
+
+    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 NodeBackend({});
+      const event = await backend.eventFromException(one);
       const result = await linkedErrors.handler(event, {
-        originalException,
+        originalException: one,
       });
 
-      // It shouldn't include root exception, as it's already processed in the event by the main error handler
       expect(result!.exception!.values!.length).toEqual(2);
       expect(result!.exception!.values![0].type).toEqual('TypeError');
       expect(result!.exception!.values![0].value).toEqual('two');
       expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames');
-      expect(result!.exception!.values![1].type).toEqual('SyntaxError');
-      expect(result!.exception!.values![1].value).toEqual('three');
+      expect(result!.exception!.values![1].type).toEqual('Error');
+      expect(result!.exception!.values![1].value).toEqual('one');
       expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames');
     });
   });