fix: error filtering of non-errors

packages/analytics-js-plugins/__tests__/errorReporting/utils.test.ts

@@ -12,6 +12,7 @@ import {
   getBugsnagErrorEvent,
   getErrorDeliveryPayload,
   getConfigForPayloadCreation,
+  isAllowedToBeNotified,
 } from '../../src/errorReporting/utils';
 
 jest.mock('@rudderstack/analytics-js-common/utilities/uuId', () => ({
@@ -115,6 +116,14 @@ describe('Error Reporting utilities', () => {
         expect(isRudderSDKError(event)).toBe(expectedValue);
       },
     );
+
+    it('should return false if error message contains value that is not allowed to be notified', () => {
+      const event = {
+        message: 'The request failed',
+      };
+
+      expect(isRudderSDKError(event)).toBe(false);
+    });
   });
 
   describe('getErrorContext', () => {
@@ -548,4 +557,21 @@ describe('Error Reporting utilities', () => {
       });
     });
   });
+
+  describe('isAllowedToBeNotified', () => {
+    it('should return true for Error argument value', () => {
+      const result = isAllowedToBeNotified('dummy error');
+      expect(result).toBeTruthy();
+    });
+
+    it('should return false for Error argument value', () => {
+      const result = isAllowedToBeNotified('The request failed');
+      expect(result).toBeFalsy();
+    });
+
+    it('should return false for Error argument value', () => {
+      const result = isAllowedToBeNotified('unhandledException handler received a non-error');
+      expect(result).toBeFalsy();
+    });
+  });
 });

packages/analytics-js-plugins/src/errorReporting/event/event.ts

@@ -73,14 +73,10 @@ const normaliseError = (
   let error;
   let internalFrames = 0;
 
-  const createAndLogInputError = (reason: string) => {
+  const logInputError = (reason: string) => {
     const verb = component === 'error cause' ? 'was' : 'received';
     if (logger) logger.warn(`${component} ${verb} a non-error: "${reason}"`);
-    const err = new Error(
-      `${component} ${verb} a non-error. See "${component}" tab for more detail.`,
-    );
-    err.name = 'InvalidError';
-    return err;
+    return undefined;
   };
 
   // In some cases:
@@ -95,8 +91,7 @@ const normaliseError = (
     if (isError(maybeError)) {
       error = maybeError;
     } else {
-      error = createAndLogInputError(typeof maybeError);
-      internalFrames += 2;
+      error = logInputError(typeof maybeError);
     }
   } else {
     switch (typeof maybeError) {
@@ -107,8 +102,7 @@ const normaliseError = (
         internalFrames += 1;
         break;
       case 'function':
-        error = createAndLogInputError('function');
-        internalFrames += 2;
+        error = logInputError('function');
         break;
       case 'object':
         if (maybeError !== null && isError(maybeError)) {
@@ -118,17 +112,15 @@ const normaliseError = (
           error.name = maybeError.name || maybeError.errorClass;
           internalFrames += 1;
         } else {
-          error = createAndLogInputError(maybeError === null ? 'null' : 'unsupported object');
-          internalFrames += 2;
+          error = logInputError(maybeError === null ? 'null' : 'unsupported object');
         }
         break;
       default:
-        error = createAndLogInputError('nothing');
-        internalFrames += 2;
+        error = logInputError('nothing');
     }
   }
 
-  if (!hasStack(error)) {
+  if (error && !hasStack(error)) {
     // in IE10/11 a new Error() doesn't have a stacktrace until you throw it, so try that here
     try {
       throw error;
@@ -137,7 +129,7 @@ const normaliseError = (
         error = e;
         // if the error only got a stacktrace after we threw it here, we know it
         // will only have one extra internal frame from this function, regardless
-        // of whether it went through createAndLogInputError() or not
+        // of whether it went through logInputError() or not
         internalFrames = 1;
       }
     }
@@ -166,6 +158,9 @@ class ErrorFormat implements IErrorFormat {
       component,
       logger,
     );
+    if (!error) {
+      return undefined;
+    }
     let event;
     try {
       const stacktrace = getStacktrace(

packages/analytics-js-plugins/src/errorReporting/index.ts

@@ -84,7 +84,7 @@ const ErrorReporting = (): ExtensionPlugin => ({
         );
 
         // filter errors
-        if (!isRudderSDKError(errorPayload.errors[0])) {
+        if (!errorPayload || !isRudderSDKError(errorPayload.errors[0])) {
           return;
         }
 

packages/analytics-js-plugins/src/errorReporting/utils.ts

@@ -17,6 +17,7 @@ import type {
 import { generateUUID } from '@rudderstack/analytics-js-common/utilities/uuId';
 import { METRICS_PAYLOAD_VERSION } from '@rudderstack/analytics-js-common/constants/metrics';
 import { stringifyWithoutCircular } from '@rudderstack/analytics-js-common/utilities/json';
+import { ERROR_MESSAGES_TO_BE_FILTERED } from '@rudderstack/analytics-js-common/constants/errors';
 import {
   APP_STATE_EXCLUDE_KEYS,
   DEV_HOSTS,
@@ -142,8 +143,25 @@ const getBugsnagErrorEvent = (
   ],
 });
 
+/**
+ * A function to determine whether the error should be promoted to notify or not
+ * @param {Error} error
+ * @returns
+ */
+const isAllowedToBeNotified = (errorMessage: string) =>
+  !ERROR_MESSAGES_TO_BE_FILTERED.some(e => errorMessage.includes(e));
+
+/**
+ * A function to determine if the error is from Rudder SDK
+ * @param {Error} event
+ * @returns
+ */
 const isRudderSDKError = (event: any) => {
   const errorOrigin = event.stacktrace?.[0]?.file;
+  const errorMessage = event.message;
+  if (errorMessage && typeof errorMessage === 'string' && !isAllowedToBeNotified(errorMessage)) {
+    return false;
+  }
 
   if (!errorOrigin || typeof errorOrigin !== 'string') {
     return false;
@@ -188,4 +206,5 @@ export {
   isRudderSDKError,
   getErrorDeliveryPayload,
   getErrorContext,
+  isAllowedToBeNotified,
 };

packages/analytics-js/__tests__/components/configManager/commonUtil.test.ts

@@ -110,7 +110,7 @@ describe('Config Manager Common Utilities', () => {
 
       updateReportingState(mockSourceConfig, mockLogger);
 
-      // expect(state.reporting.isErrorReportingEnabled.value).toBe(true); // TODO: uncomment this line when error reporting is enabled
+      expect(state.reporting.isErrorReportingEnabled.value).toBe(true);
       expect(state.reporting.isMetricsReportingEnabled.value).toBe(true);
       expect(mockLogger.warn).not.toHaveBeenCalled();
     });
@@ -133,7 +133,7 @@ describe('Config Manager Common Utilities', () => {
 
       updateReportingState(mockSourceConfig, mockLogger);
 
-      // expect(state.reporting.isErrorReportingEnabled.value).toBe(true); // TODO: uncomment this line when error reporting is enabled
+      expect(state.reporting.isErrorReportingEnabled.value).toBe(true);
       expect(state.reporting.isMetricsReportingEnabled.value).toBe(true);
       expect(mockLogger.warn).not.toHaveBeenCalled();
     });

packages/analytics-js/__tests__/services/ErrorHandler/processError.test.ts

@@ -1,7 +1,6 @@
 import {
   processError,
   getNormalizedErrorForUnhandledError,
-  isAllowedToBeNotified,
 } from '../../../src/services/ErrorHandler/processError';
 
 jest.mock('../../../src/components/utilities/event', () => {
@@ -95,7 +94,7 @@ describe('ErrorHandler - getNormalizedErrorForUnhandledError', () => {
     expect(normalizedError).toBeUndefined();
   });
 
-  it.skip('should return error instance for Event argument value with SDK script target', () => {
+  it('should return error instance for Event argument value with SDK script target', () => {
     const event = new Event('dummyError');
     const targetElement = document.createElement('script');
     targetElement.dataset.loader = 'RS_JS_SDK';
@@ -116,30 +115,3 @@ describe('ErrorHandler - getNormalizedErrorForUnhandledError', () => {
     );
   });
 });
-
-describe('ErrorHandler - isAllowedToBeNotified', () => {
-  it('should return true for Error argument value', () => {
-    const result = isAllowedToBeNotified(new Error('dummy error'));
-    expect(result).toBeTruthy();
-  });
-
-  it('should return true for Error argument value', () => {
-    const result = isAllowedToBeNotified(new Error('The request failed'));
-    expect(result).toBeFalsy();
-  });
-
-  it('should return true for ErrorEvent argument value', () => {
-    const result = isAllowedToBeNotified(new ErrorEvent('dummy error'));
-    expect(result).toBeTruthy();
-  });
-
-  it('should return true for PromiseRejectionEvent argument value', () => {
-    const result = isAllowedToBeNotified(new PromiseRejectionEvent('dummy error'));
-    expect(result).toBeTruthy();
-  });
-
-  it('should return true for PromiseRejectionEvent argument value', () => {
-    const result = isAllowedToBeNotified(new PromiseRejectionEvent('The request failed'));
-    expect(result).toBeFalsy();
-  });
-});

packages/analytics-js/src/components/configManager/util/commonUtil.ts

@@ -73,10 +73,8 @@ const getSDKUrl = (): string | undefined => {
  * @param logger Logger instance
  */
 const updateReportingState = (res: SourceConfigResponse): void => {
-  state.reporting.isErrorReportingEnabled.value = false;
-  // TODO: Enable this once the error reporting is tested properly
-  // state.reporting.isErrorReportingEnabled.value =
-  //   isErrorReportingEnabled(res.source.config) && !isSDKRunningInChromeExtension();
+  state.reporting.isErrorReportingEnabled.value =
+    isErrorReportingEnabled(res.source.config) && !isSDKRunningInChromeExtension();
   state.reporting.isMetricsReportingEnabled.value = isMetricsReportingEnabled(res.source.config);
 };
 

packages/analytics-js/src/services/ErrorHandler/ErrorHandler.ts

@@ -21,11 +21,7 @@ import {
 import { state } from '../../state';
 import { defaultPluginEngine } from '../PluginEngine';
 import { defaultLogger } from '../Logger';
-import {
-  isAllowedToBeNotified,
-  getNormalizedErrorForUnhandledError,
-  processError,
-} from './processError';
+import { getNormalizedErrorForUnhandledError, processError } from './processError';
 
 /**
  * A service to handle errors
@@ -204,7 +200,7 @@ class ErrorHandler implements IErrorHandler {
    * @param {Error} error Error instance from handled error
    */
   notifyError(error: SDKError, errorState: ErrorState) {
-    if (this.pluginEngine && this.httpClient && isAllowedToBeNotified(error)) {
+    if (this.pluginEngine && this.httpClient) {
       try {
         this.pluginEngine.invokeSingle(
           'errorReporting.notify',

packages/analytics-js/src/services/ErrorHandler/processError.ts

@@ -1,7 +1,6 @@
 import { stringifyWithoutCircular } from '@rudderstack/analytics-js-common/utilities/json';
 import { isString } from '@rudderstack/analytics-js-common/utilities/checks';
 import type { ErrorTarget, SDKError } from '@rudderstack/analytics-js-common/types/ErrorHandler';
-import { ERROR_MESSAGES_TO_BE_FILTERED } from '../../constants/errors';
 import { LOAD_ORIGIN } from './constant';
 
 /**
@@ -39,44 +38,29 @@
       return error;
     }
     // TODO: remove this block once all device mode integrations start using the v3 script loader module (TS)
-    // if (error instanceof Event) {
-    //   const eventTarget = error.target as ErrorTarget;
-    //   // Discard all the non-script loading errors
-    //   if (eventTarget && eventTarget.localName !== 'script') {
-    //     return undefined;
-    //   }
-    //   // Discard script errors that are not originated at SDK or from native SDKs
-    //   if (
-    //     eventTarget?.dataset &&
-    //     (eventTarget.dataset.loader !== LOAD_ORIGIN ||
-    //       eventTarget.dataset.isnonnativesdk !== 'true')
-    //   ) {
-    //     return undefined;
-    //   }
-    //   const errorMessage = `Error in loading a third-party script from URL ${eventTarget?.src} with ID ${eventTarget?.id}.`;
-    //   return Object.create(error, {
-    //     message: { value: errorMessage },
-    //   });
-    // }
-    return undefined;
+    if (error instanceof Event) {
+      const eventTarget = error.target as ErrorTarget;
+      // Discard all the non-script loading errors
+      if (eventTarget && eventTarget.localName !== 'script') {
+        return undefined;
+      }
+      // Discard script errors that are not originated at SDK or from native SDKs
+      if (
+        eventTarget?.dataset &&
+        (eventTarget.dataset.loader !== LOAD_ORIGIN ||
+          eventTarget.dataset.isnonnativesdk !== 'true')
+      ) {
+        return undefined;
+      }
+      const errorMessage = `Error in loading a third-party script from URL ${eventTarget?.src} with ID ${eventTarget?.id}.`;
+      return Object.create(error, {
+        message: { value: errorMessage },
+      });
+    }
+    return error;
   } catch (e) {
     return e;
   }
 };
 
-/**
- * A function to determine whether the error should be promoted to notify or not
- * @param {Error} error
- * @returns
- */
-const isAllowedToBeNotified = (error: SDKError) => {
-  if ((error instanceof Error || error instanceof ErrorEvent) && error.message) {
-    return !ERROR_MESSAGES_TO_BE_FILTERED.some(e => error.message.includes(e));
-  }
-  if (error instanceof PromiseRejectionEvent && typeof error.reason === 'string') {
-    return !ERROR_MESSAGES_TO_BE_FILTERED.some(e => error.reason.includes(e));
-  }
-  return true;
-};
-
-export { processError, isAllowedToBeNotified, getNormalizedErrorForUnhandledError };
+export { processError, getNormalizedErrorForUnhandledError };

packages/analytics-js/src/services/HttpClient/xhr/xhrRequestHandler.ts

@@ -8,8 +8,8 @@ import type {
 } from '@rudderstack/analytics-js-common/types/HttpClient';
 import type { ILogger } from '@rudderstack/analytics-js-common/types/Logger';
 import { getMutatedError } from '@rudderstack/analytics-js-common/utilities/errors';
+import { FAILED_REQUEST_ERR_MSG_PREFIX } from '@rudderstack/analytics-js-common/constants/errors';
 import { DEFAULT_XHR_TIMEOUT_MS } from '../../../constants/timeouts';
-import { FAILED_REQUEST_ERR_MSG_PREFIX } from '../../../constants/errors';
 import {
   XHR_PAYLOAD_PREP_ERROR,
   XHR_DELIVERY_ERROR,