feat: make error handler and logger mandatory in all components (#2007)

* feat: improve user callback execution * feat: make error handler and logger mandatory in all components
rudderlabs · Jan 21, 2025 · 63c7f23 · 63c7f23
1 parent bc050e6
commit 63c7f23
Show file tree

Hide file tree

Showing 55 changed files with 1,026 additions and 754 deletions.
diff --git a/packages/analytics-js-common/__mocks__/HttpClient.ts b/packages/analytics-js-common/__mocks__/HttpClient.ts
@@ -1,16 +1,17 @@
-import type { IErrorHandler } from '../src/types/ErrorHandler';
-import type { IHttpClient } from '../src/types/HttpClient';
-import type { ILogger } from '../src/types/Logger';
+import type { IErrorHandler } from '@rudderstack/analytics-js-common/types/ErrorHandler';
+import type { IHttpClient } from '@rudderstack/analytics-js-common/types/HttpClient';
+import type { ILogger } from '@rudderstack/analytics-js-common/types/Logger';
+import { defaultLogger } from './Logger';
 
 class HttpClient implements IHttpClient {
   errorHandler?: IErrorHandler;
-  logger?: ILogger;
+  logger: ILogger = defaultLogger;
   hasErrorHandler = false;
-  init = jest.fn();
   getData = jest.fn();
   getAsyncData = jest.fn();
   setAuthHeader = jest.fn();
   resetAuthHeader = jest.fn();
+  init = jest.fn();
 }
 
 const defaultHttpClient = new HttpClient();

diff --git a/packages/analytics-js-common/__mocks__/Store.ts b/packages/analytics-js-common/__mocks__/Store.ts
@@ -1,23 +1,33 @@
-import type { IStore, IStoreConfig } from '../src/types/Store';
+import type { IPluginsManager } from '@rudderstack/analytics-js-common/types/PluginsManager';
+import type { IStorage, IStore, IStoreConfig } from '../src/types/Store';
 import { defaultInMemoryStorage, defaultLocalStorage } from './Storage';
+import { defaultPluginsManager } from './PluginsManager';
+import { defaultLogger } from './Logger';
+import { defaultErrorHandler } from './ErrorHandler';
 
 // Mock all the methods of the Store class
 
 class Store implements IStore {
-  constructor(config: IStoreConfig, engine?: any) {
+  constructor(config: IStoreConfig, engine: IStorage, pluginsManager: IPluginsManager) {
     this.id = config.id;
     this.name = config.name;
     this.isEncrypted = config.isEncrypted ?? false;
     this.validKeys = config.validKeys ?? {};
     this.engine = engine ?? defaultLocalStorage;
     this.originalEngine = this.engine;
+    this.errorHandler = config.errorHandler;
+    this.logger = config.logger;
+    this.pluginsManager = pluginsManager;
   }
   id = 'test';
   name = 'test';
   isEncrypted = false;
   validKeys: Record<string, string>;
-  engine = defaultLocalStorage;
-  originalEngine = defaultLocalStorage;
+  engine: IStorage = defaultLocalStorage;
+  originalEngine: IStorage = defaultLocalStorage;
+  errorHandler;
+  logger;
+  pluginsManager;
   createValidKey = (key: string) => {
     return [this.name, this.id, key].join('.');
   };
@@ -51,6 +61,10 @@ class Store implements IStore {
   getOriginalEngine = () => this.originalEngine;
 }
 
-const defaultStore = new Store({ id: 'test', name: 'test' });
+const defaultStore = new Store(
+  { id: 'test', name: 'test', errorHandler: defaultErrorHandler, logger: defaultLogger },
+  defaultLocalStorage,
+  defaultPluginsManager,
+);
 
 export { Store, defaultStore };
diff --git a/packages/analytics-js-common/__mocks__/StoreManager.ts b/packages/analytics-js-common/__mocks__/StoreManager.ts
@@ -1,4 +1,5 @@
 import type { IStoreConfig, IStoreManager } from '../src/types/Store';
+import { defaultPluginsManager } from './PluginsManager';
 import { defaultCookieStorage, defaultInMemoryStorage, defaultLocalStorage } from './Storage';
 import { defaultStore, Store } from './Store';
 
@@ -21,7 +22,7 @@ class StoreManager implements IStoreManager {
         break;
     }
 
-    return new Store(config, storageEngine);
+    return new Store(config, storageEngine, defaultPluginsManager);
   };
   getStore = jest.fn(() => defaultStore);
   initializeStorageState = jest.fn();

diff --git a/packages/analytics-js-common/__tests__/utilities/errors.test.ts b/packages/analytics-js-common/__tests__/utilities/errors.test.ts
@@ -1,18 +1,83 @@
-import { dispatchErrorEvent } from '../../src/utilities/errors';
+import { dispatchErrorEvent, getStacktrace } from '../../src/utilities/errors';
 
 describe('Errors - utilities', () => {
   describe('dispatchErrorEvent', () => {
+    const dispatchEventMock = jest.fn();
+    const originalDispatchEvent = globalThis.dispatchEvent;
+
+    beforeEach(() => {
+      globalThis.dispatchEvent = dispatchEventMock;
+    });
+
+    afterEach(() => {
+      globalThis.dispatchEvent = originalDispatchEvent;
+    });
+
     it('should dispatch an error event', () => {
-      const dispatchEvent = jest.fn();
-      const originalDispatchEvent = globalThis.dispatchEvent;
+      const error = new Error('Test error');
+
+      dispatchErrorEvent(error);
 
-      globalThis.dispatchEvent = dispatchEvent;
+      expect(dispatchEventMock).toHaveBeenCalledWith(new ErrorEvent('error', { error }));
+      expect((error.stack as string).endsWith('[SDK DISPATCHED ERROR]')).toBeTruthy();
+    });
+
+    it('should decorate stacktrace before dispatching error event', () => {
       const error = new Error('Test error');
+      // @ts-expect-error need to set the value for testing
+      error.stacktrace = error.stack;
+      delete error.stack;
+
       dispatchErrorEvent(error);
-      expect(dispatchEvent).toHaveBeenCalledWith(new ErrorEvent('error', { error }));
 
-      // Cleanup
-      globalThis.dispatchEvent = originalDispatchEvent;
+      // @ts-expect-error need to check the stacktrace property
+      expect((error.stacktrace as string).endsWith('[SDK DISPATCHED ERROR]')).toBeTruthy();
+    });
+
+    it('should decorate opera sourceloc before dispatching error event', () => {
+      const error = new Error('Test error');
+      // @ts-expect-error need to set the value for testing
+      error['opera#sourceloc'] = error.stack;
+      delete error.stack;
+
+      dispatchErrorEvent(error);
+
+      // @ts-expect-error need to check the opera sourceloc property
+      expect((error['opera#sourceloc'] as string).endsWith('[SDK DISPATCHED ERROR]')).toBeTruthy();
+    });
+  });
+
+  describe('getStacktrace', () => {
+    it('should return stack if it is a string', () => {
+      const error = new Error('Test error');
+      expect(getStacktrace(error)).toBe(error.stack);
+    });
+
+    it('should return stacktrace if it is a string', () => {
+      const error = new Error('Test error');
+      // @ts-expect-error need to set the value for testing
+      error.stacktrace = error.stack;
+      delete error.stack;
+
+      // @ts-expect-error need to check the stacktrace property
+      expect(getStacktrace(error)).toBe(error.stacktrace);
+    });
+
+    it('should return opera sourceloc if it is a string', () => {
+      const error = new Error('Test error');
+      // @ts-expect-error need to set the value for testing
+      error['opera#sourceloc'] = error.stack;
+      delete error.stack;
+
+      // @ts-expect-error need to check the opera sourceloc property
+      expect(getStacktrace(error)).toBe(error['opera#sourceloc']);
+    });
+
+    it('should return undefined if none of the properties are strings', () => {
+      const error = new Error('Test error');
+      delete error.stack;
+
+      expect(getStacktrace(error)).toBeUndefined();
     });
   });
 });
diff --git a/packages/analytics-js-common/src/services/ExternalSrcLoader/ExternalSrcLoader.ts b/packages/analytics-js-common/src/services/ExternalSrcLoader/ExternalSrcLoader.ts
@@ -10,20 +10,18 @@ import { jsFileLoader } from './jsFileLoader';
  * Service to load external resources/files
  */
 class ExternalSrcLoader implements IExternalSrcLoader {
-  errorHandler?: IErrorHandler;
-  logger?: ILogger;
-  hasErrorHandler = false;
+  errorHandler: IErrorHandler;
+  logger: ILogger;
   timeout: number;
 
   constructor(
-    errorHandler?: IErrorHandler,
-    logger?: ILogger,
+    errorHandler: IErrorHandler,
+    logger: ILogger,
     timeout = DEFAULT_EXT_SRC_LOAD_TIMEOUT_MS,
   ) {
     this.errorHandler = errorHandler;
     this.logger = logger;
     this.timeout = timeout;
-    this.hasErrorHandler = Boolean(this.errorHandler);
     this.onError = this.onError.bind(this);
   }
 
@@ -52,11 +50,7 @@ class ExternalSrcLoader implements IExternalSrcLoader {
    * Handle errors
    */
   onError(error: unknown) {
-    if (this.hasErrorHandler) {
-      this.errorHandler?.onError(error, EXTERNAL_SRC_LOADER);
-    } else {
-      throw error;
-    }
+    this.errorHandler.onError(error, EXTERNAL_SRC_LOADER);
   }
 }
 

diff --git a/packages/analytics-js-common/src/services/ExternalSrcLoader/types.ts b/packages/analytics-js-common/src/services/ExternalSrcLoader/types.ts
@@ -11,8 +11,8 @@ export interface IExternalSourceLoadConfig {
 }
 
 export interface IExternalSrcLoader {
-  errorHandler?: IErrorHandler;
-  logger?: ILogger;
+  errorHandler: IErrorHandler;
+  logger: ILogger;
   timeout: number;
   loadJSFile(config: IExternalSourceLoadConfig): void;
 }
diff --git a/packages/analytics-js-common/src/types/HttpClient.ts b/packages/analytics-js-common/src/types/HttpClient.ts
@@ -56,14 +56,13 @@ export type HTTPClientMethod =
 
 export interface IHttpClient {
   errorHandler?: IErrorHandler;
-  logger?: ILogger;
+  logger: ILogger;
   basicAuthHeader?: string;
-  hasErrorHandler: boolean;
-  init: (errorHandler: IErrorHandler) => void;
   getData<T = any>(
     config: IRequestConfig,
   ): Promise<{ data: T | string | undefined; details?: ResponseDetails }>;
   getAsyncData<T = any>(config: IAsyncRequestConfig<T>): void;
   setAuthHeader(value: string, noBto?: boolean): void;
   resetAuthHeader(): void;
+  init(errorHandler: IErrorHandler): void;
 }
diff --git a/packages/analytics-js-common/src/types/Store.ts b/packages/analytics-js-common/src/types/Store.ts
@@ -12,16 +12,16 @@ export interface IStoreConfig {
   isEncrypted?: boolean;
   validKeys?: Record<string, string>;
   noCompoundKey?: boolean;
-  errorHandler?: IErrorHandler;
-  logger?: ILogger;
+  errorHandler: IErrorHandler;
+  logger: ILogger;
   type?: StorageType;
 }
 
 export interface IStoreManager {
   stores?: Record<StoreId, IStore>;
   isInitialized?: boolean;
-  errorHandler?: IErrorHandler;
-  logger?: ILogger;
+  errorHandler: IErrorHandler;
+  logger: ILogger;
   init(): void;
   initializeStorageState(): void;
   setStore(storeConfig: IStoreConfig): IStore;
@@ -37,9 +37,9 @@ export interface IStore {
   originalEngine: IStorage;
   noKeyValidation?: boolean;
   noCompoundKey?: boolean;
-  errorHandler?: IErrorHandler;
-  logger?: ILogger;
-  pluginsManager?: IPluginsManager;
+  errorHandler: IErrorHandler;
+  logger: ILogger;
+  pluginsManager: IPluginsManager;
   createValidKey(key: string): string | undefined;
   swapQueueStoreToInMemoryEngine(): void;
   set(key: string, value: any): void;

diff --git a/packages/analytics-js-common/src/utilities/errors.ts b/packages/analytics-js-common/src/utilities/errors.ts
@@ -1,7 +1,19 @@
 import { isTypeOfError } from './checks';
 import { stringifyWithoutCircular } from './json';
 
-const MANUAL_ERROR_IDENTIFIER = '[MANUAL ERROR]';
+const MANUAL_ERROR_IDENTIFIER = '[SDK DISPATCHED ERROR]';
+
+const getStacktrace = (err: any): string | undefined => {
+  const { stack, stacktrace } = err;
+  const operaSourceloc = err['opera#sourceloc'];
+
+  const stackString = stack ?? stacktrace ?? operaSourceloc;
+
+  if (!!stackString && typeof stackString === 'string') {
+    return stackString;
+  }
+  return undefined;
+};
 
 /**
  * Get mutated error with issue prepended to error message
@@ -21,21 +33,32 @@ const getMutatedError = (err: any, issue: string): Error => {
 
 const dispatchErrorEvent = (error: any) => {
   if (isTypeOfError(error)) {
-    error.stack = `${error.stack ?? ''}\n${MANUAL_ERROR_IDENTIFIER}`;
-  }
-  (globalThis as typeof window).dispatchEvent(new ErrorEvent('error', { error }));
-};
-
-const getStacktrace = (err: any): string | undefined => {
-  const { stack, stacktrace } = err;
-  const operaSourceloc = err['opera#sourceloc'];
+    const errStack = getStacktrace(error);
+    if (errStack) {
+      const { stack, stacktrace } = error;
+      const operaSourceloc = error['opera#sourceloc'];
 
-  const stackString = stack ?? stacktrace ?? operaSourceloc;
-
-  if (!!stackString && typeof stackString === 'string') {
-    return stackString;
+      switch (errStack) {
+        case stack:
+          // eslint-disable-next-line no-param-reassign
+          error.stack = `${stack}\n${MANUAL_ERROR_IDENTIFIER}`;
+          break;
+        case stacktrace:
+          // eslint-disable-next-line no-param-reassign
+          error.stacktrace = `${stacktrace}\n${MANUAL_ERROR_IDENTIFIER}`;
+          break;
+        case operaSourceloc:
+        default:
+          // eslint-disable-next-line no-param-reassign
+          error['opera#sourceloc'] = `${operaSourceloc}\n${MANUAL_ERROR_IDENTIFIER}`;
+          break;
+      }
+    }
   }
-  return undefined;
+
+  (globalThis as typeof window).dispatchEvent(
+    new ErrorEvent('error', { error, bubbles: true, cancelable: true, composed: true }),
+  );
 };
 
 export { getMutatedError, dispatchErrorEvent, MANUAL_ERROR_IDENTIFIER, getStacktrace };
diff --git a/packages/analytics-js-plugins/__tests__/ketchConsentManager/utils.test.ts b/packages/analytics-js-plugins/__tests__/ketchConsentManager/utils.test.ts
@@ -6,7 +6,6 @@ import {
   getConsentData,
   getKetchConsentData,
 } from '../../src/ketchConsentManager/utils';
-import { defaultPluginsManager } from '@rudderstack/analytics-js-common/__mocks__/PluginsManager';
 
 describe('KetchConsentManager - Utils', () => {
   beforeEach(() => {

diff --git a/packages/analytics-js-plugins/src/iubendaConsentManager/utils.ts b/packages/analytics-js-plugins/src/iubendaConsentManager/utils.ts
@@ -51,6 +51,8 @@ const getIubendaConsentData = (
       id: IUBENDA_CONSENT_MANAGER_PLUGIN,
       name: IUBENDA_CONSENT_MANAGER_PLUGIN,
       type: COOKIE_STORAGE,
+      errorHandler: storeManager?.errorHandler,
+      logger: storeManager?.logger,
     });
     rawConsentCookieData = dataStore?.engine.getItem(getIubendaCookieName(logger));
   } catch (err) {

diff --git a/packages/analytics-js-plugins/src/ketchConsentManager/utils.ts b/packages/analytics-js-plugins/src/ketchConsentManager/utils.ts
@@ -25,6 +25,8 @@ const getKetchConsentData = (
       id: KETCH_CONSENT_MANAGER_PLUGIN,
       name: KETCH_CONSENT_MANAGER_PLUGIN,
       type: COOKIE_STORAGE,
+      errorHandler: storeManager?.errorHandler,
+      logger: storeManager?.logger,
     });
     rawConsentCookieData = dataStore?.engine.getItem(KETCH_CONSENT_COOKIE_NAME_V1);
   } catch (err) {

diff --git a/packages/analytics-js-plugins/src/utilities/retryQueue/RetryQueue.ts b/packages/analytics-js-plugins/src/utilities/retryQueue/RetryQueue.ts
@@ -134,6 +134,8 @@ class RetryQueue implements IQueue<QueueItemData> {
       name: this.name,
       validKeys: QueueStatuses,
       type: storageType,
+      errorHandler: this.storeManager.errorHandler,
+      logger: this.storeManager.logger,
     });
     this.setDefaultQueueEntries();
 
@@ -583,6 +585,8 @@ class RetryQueue implements IQueue<QueueItemData> {
       name: this.name,
       validKeys: QueueStatuses,
       type: LOCAL_STORAGE,
+      errorHandler: this.storeManager.errorHandler,
+      logger: this.storeManager.logger,
     });
     const our = {
       queue: (this.getStorageEntry(QueueStatuses.QUEUE) ?? []) as QueueItem[],
@@ -770,6 +774,8 @@ class RetryQueue implements IQueue<QueueItemData> {
               name,
               validKeys: QueueStatuses,
               type: LOCAL_STORAGE,
+              errorHandler: this.storeManager.errorHandler,
+              logger: this.storeManager.logger,
             }),
           );
         }