fix: Check if value is error object in extraErrorData integration (#1817)

* check if value is error object, also use getOwnPropertyNames
* use isError method

Author: github-felipe-caputo

packages/core/src/integrations/extraerrordata.ts

@@ -28,20 +28,22 @@ export class ExtraErrorData implements Integration {
   public setupOnce(): void {
     addGlobalEventProcessor(async (event: SentryEvent, hint?: SentryEventHint) => {
       const self = getCurrentHub().getIntegration(ExtraErrorData);
-
-      if (!self || !hint || !hint.originalException) {
+      if (!self) {
         return event;
       }
-
-      return this.enhanceEventWithErrorData(event, hint.originalException);
+      return self.enhanceEventWithErrorData(event, hint);
     });
   }
 
   /**
    * Attaches extracted information from the Error object to extra field in the SentryEvent
    */
-  public enhanceEventWithErrorData(event: SentryEvent, error: Error): SentryEvent {
-    const errorData = this.extractErrorData(error);
+  public enhanceEventWithErrorData(event: SentryEvent, hint?: SentryEventHint): SentryEvent {
+    if (!hint || !hint.originalException || !isError(hint.originalException)) {
+      return event;
+    }
+
+    const errorData = this.extractErrorData(hint.originalException);
 
     if (errorData) {
       return {
@@ -64,7 +66,7 @@ export class ExtraErrorData implements Integration {
     try {
       const nativeKeys = ['name', 'message', 'stack', 'line', 'column', 'fileName', 'lineNumber', 'columnNumber'];
       const name = error.name || error.constructor.name;
-      const errorKeys = Object.keys(error).filter(key => nativeKeys.indexOf(key) === -1);
+      const errorKeys = Object.getOwnPropertyNames(error).filter(key => nativeKeys.indexOf(key) === -1);
 
       if (errorKeys.length) {
         const extraErrorInfo: { [key: string]: unknown } = {};

packages/core/test/lib/integrations/extraerrordata.test.ts

@@ -21,7 +21,9 @@ describe('ExtraErrorData()', () => {
     error.baz = 42;
     error.foo = 'bar';
 
-    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, error);
+    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, {
+      originalException: error,
+    });
 
     expect(enhancedEvent.extra).toEqual({
       TypeError: {
@@ -35,7 +37,9 @@ describe('ExtraErrorData()', () => {
     const error = new TypeError('foo') as ExtendedError;
     error.cause = new SyntaxError('bar');
 
-    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, error);
+    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, {
+      originalException: error,
+    });
 
     expect(enhancedEvent.extra).toEqual({
       TypeError: {
@@ -53,7 +57,9 @@ describe('ExtraErrorData()', () => {
     const error = new TypeError('foo') as ExtendedError;
     error.baz = 42;
 
-    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, error);
+    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, {
+      originalException: error,
+    });
 
     expect(enhancedEvent.extra).toEqual({
       TypeError: {
@@ -62,4 +68,28 @@ describe('ExtraErrorData()', () => {
       foo: 42,
     });
   });
+
+  it('should return event if originalException is not an Error object', () => {
+    const error = 'error message, not object';
+
+    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, {
+      originalException: error,
+    });
+
+    expect(enhancedEvent).toEqual(event);
+  });
+
+  it('should return event if there is no SentryEventHint', () => {
+    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event);
+
+    expect(enhancedEvent).toEqual(event);
+  });
+
+  it('should return event if there is no originalException', () => {
+    const enhancedEvent = extraErrorData.enhanceEventWithErrorData(event, {
+      notOriginalException: 'fooled you',
+    });
+
+    expect(enhancedEvent).toEqual(event);
+  });
 });