Skip to content

Commit

Permalink
feat: improve user callback execution (#2006)
Browse files Browse the repository at this point in the history 
* feat: improve user callback execution

* fix: error message for callback parameter validation
  • Loading branch information
@saikumarrs
saikumarrs authored Jan 20, 2025
1 parent 30a5d43 commit bc050e6
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 58 deletions.
9 changes: 5 additions & 4 deletions packages/analytics-js-common/src/constants/loggerContexts.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const API_SUFFIX = 'API';
const CAPABILITIES_MANAGER = 'CapabilitiesManager';
const CONFIG_MANAGER = 'ConfigManager';
const EVENT_MANAGER = 'EventManager';
Expand All @@ -6,14 +7,13 @@ const USER_SESSION_MANAGER = 'UserSessionManager';
const ERROR_HANDLER = 'ErrorHandler';
const PLUGIN_ENGINE = 'PluginEngine';
const STORE_MANAGER = 'StoreManager';
const READY_API = 'readyApi';
const LOAD_CONFIGURATION = 'LoadConfiguration';
const READY_API = `Ready${API_SUFFIX}`;
const LOAD_API = `Load${API_SUFFIX}`;
const EVENT_REPOSITORY = 'EventRepository';
const EXTERNAL_SRC_LOADER = 'ExternalSrcLoader';
const HTTP_CLIENT = 'HttpClient';
const RSA = 'RudderStackAnalytics';
const ANALYTICS_CORE = 'AnalyticsCore';

export {
CAPABILITIES_MANAGER,
CONFIG_MANAGER,
Expand All @@ -24,10 +24,11 @@ export {
PLUGIN_ENGINE,
STORE_MANAGER,
READY_API,
LOAD_CONFIGURATION,
LOAD_API,
EVENT_REPOSITORY,
EXTERNAL_SRC_LOADER,
HTTP_CLIENT,
RSA,
ANALYTICS_CORE,
API_SUFFIX,
};
91 changes: 90 additions & 1 deletion packages/analytics-js/__tests__/components/core/Analytics.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,33 @@ describe('Core - Analytics', () => {
describe('load', () => {
const sampleDataPlaneUrl = 'https://www.dummy.url';
it('should load the analytics script with the given options', () => {
state.loadOptions.value.logLevel = 'WARN';

const startLifecycleSpy = jest.spyOn(analytics, 'startLifecycle');
const setMinLogLevelSpy = jest.spyOn(analytics.logger, 'setMinLogLevel');

analytics.load(dummyWriteKey, sampleDataPlaneUrl, { logLevel: 'ERROR' });

expect(state.lifecycle.status.value).toBe('browserCapabilitiesReady');
expect(startLifecycleSpy).toHaveBeenCalledTimes(1);
expect(setMinLogLevelSpy).toHaveBeenCalledWith('ERROR');
// Once in load API and then in config manager
expect(setMinLogLevelSpy).toHaveBeenCalledTimes(2);
expect(setMinLogLevelSpy).toHaveBeenNthCalledWith(1, 'ERROR');
expect(setExposedGlobal).toHaveBeenCalledWith('state', state, dummyWriteKey);
});

it('should set the log level if it is not configured', () => {
state.loadOptions.value.logLevel = undefined;
const setMinLogLevelSpy = jest.spyOn(analytics.logger, 'setMinLogLevel');

analytics.load(dummyWriteKey, sampleDataPlaneUrl);

expect(state.lifecycle.status.value).toBe('browserCapabilitiesReady');
// Once in load API and then in config manager
expect(setMinLogLevelSpy).toHaveBeenCalledTimes(2);
expect(setMinLogLevelSpy).toHaveBeenNthCalledWith(1, 'ERROR');
});

it('should not load if the write key is invalid', () => {
const startLifecycleSpy = jest.spyOn(analytics, 'startLifecycle');
const errorSpy = jest.spyOn(analytics.logger, 'error');
Expand Down Expand Up @@ -233,6 +251,32 @@ describe('Core - Analytics', () => {
analyticsInstance: undefined,
});
});

it('should log an error if the onLoaded callback is not a function', () => {
const errorSpy = jest.spyOn(analytics.logger, 'error');
// @ts-expect-error testing invalid callback
state.loadOptions.value.onLoaded = true;

analytics.onInitialized();

expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy).toHaveBeenCalledWith('LoadAPI:: The provided callback parameter is not a function.');
});

it('should log an error if the onLoaded callback throws an error', () => {
const errorSpy = jest.spyOn(analytics.logger, 'error');
state.loadOptions.value.onLoaded = () => {
throw new Error('Test error');
};

analytics.onInitialized();

expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy).toHaveBeenCalledWith(
'LoadAPI:: The callback threw an exception',
new Error('Test error'),
);
});
});

describe('onDestinationsReady', () => {
Expand All @@ -259,6 +303,22 @@ describe('Core - Analytics', () => {
expect(callback).toHaveBeenCalledTimes(2);
});

it('should log an error if a ready callback throws an error', () => {
const errorSpy = jest.spyOn(analytics.logger, 'error');
const callback = () => {
throw new Error('Test error');
};
state.eventBuffer.readyCallbacksArray.value = [callback, jest.fn()];

analytics.onReady();

expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy).toHaveBeenCalledWith(
'ReadyAPI:: The callback threw an exception',
new Error('Test error'),
);
});

it('should ignore calls with no function callback', () => {
const leaveBreadcrumbSpy = jest.spyOn(analytics.errorHandler, 'leaveBreadcrumb');
const errorSpy = jest.spyOn(analytics.logger, 'error');
Expand Down Expand Up @@ -309,6 +369,35 @@ describe('Core - Analytics', () => {
analyticsInstance: undefined,
});
});

it('should log an error if the provided callback is not a function', () => {
state.lifecycle.loaded.value = true;

const errorSpy = jest.spyOn(analytics.logger, 'error');
// @ts-expect-error testing invalid callback
analytics.ready(true);

expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy).toHaveBeenCalledWith('ReadyAPI:: The provided callback parameter is not a function.');
});

it('should log an error if the provided callback throws an error', () => {
state.lifecycle.loaded.value = true;
state.lifecycle.status.value = 'readyExecuted';

const errorSpy = jest.spyOn(analytics.logger, 'error');
const callback = () => {
throw new Error('Test error');
};

analytics.ready(callback);

expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy).toHaveBeenCalledWith(
'ReadyAPI:: The callback threw an exception',
new Error('Test error'),
);
});
});

describe('page', () => {
Expand Down
59 changes: 41 additions & 18 deletions packages/analytics-js/__tests__/components/eventRepository/EventRepository.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ import type {
DestinationConfig,
} from '@rudderstack/analytics-js-common/types/Destination';
import type { RudderEvent } from '@rudderstack/analytics-js-common/types/Event';
import type { IErrorHandler } from '@rudderstack/analytics-js-common/types/ErrorHandler';
import { defaultErrorHandler } from '@rudderstack/analytics-js-common/__mocks__/ErrorHandler';
import { defaultLogger } from '@rudderstack/analytics-js-common/__mocks__/Logger';
import { EventRepository } from '../../../src/components/eventRepository';
import { state, resetState } from '../../../src/state';
import { PluginsManager } from '../../../src/components/pluginsManager';
import { defaultPluginEngine } from '../../../src/services/PluginEngine';
import { defaultErrorHandler } from '../../../src/services/ErrorHandler';
import { defaultLogger } from '../../../src/services/Logger';
import { StoreManager } from '../../../src/services/StoreManager';

describe('EventRepository', () => {
Expand Down Expand Up @@ -94,6 +93,7 @@ describe('EventRepository', () => {
defaultPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);
const spy = jest.spyOn(defaultPluginsManager, 'invokeSingle');
eventRepository.init();
Expand All @@ -105,7 +105,7 @@ describe('EventRepository', () => {
expect.objectContaining({}),
defaultStoreManager,
defaultErrorHandler,
undefined,
defaultLogger,
);
expect(spy).nthCalledWith(
2,
Expand All @@ -115,7 +115,7 @@ describe('EventRepository', () => {
expect.objectContaining({}),
defaultStoreManager,
defaultErrorHandler,
undefined,
defaultLogger,
);
expect(spy).nthCalledWith(
3,
Expand All @@ -125,7 +125,7 @@ describe('EventRepository', () => {
defaultStoreManager,
undefined,
defaultErrorHandler,
undefined,
defaultLogger,
);
spy.mockRestore();
});
Expand All @@ -135,6 +135,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

eventRepository.init();
Expand All @@ -149,6 +150,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

state.nativeDestinations.activeDestinations.value = [
Expand Down Expand Up @@ -180,6 +182,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

state.nativeDestinations.activeDestinations.value = activeDestinationsWithHybridMode;
Expand All @@ -196,6 +199,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

state.nativeDestinations.activeDestinations.value = activeDestinationsWithHybridMode;
Expand All @@ -219,6 +223,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

state.nativeDestinations.activeDestinations.value = activeDestinationsWithHybridMode;
Expand All @@ -241,6 +246,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

eventRepository.init();
Expand All @@ -258,7 +264,7 @@ describe('EventRepository', () => {
integrations: { All: true },
},
defaultErrorHandler,
undefined,
defaultLogger,
);
expect(invokeSingleSpy).nthCalledWith(
2,
Expand All @@ -267,7 +273,7 @@ describe('EventRepository', () => {
mockDestinationsEventsQueue,
testEvent,
defaultErrorHandler,
undefined,
defaultLogger,
);

invokeSingleSpy.mockRestore();
Expand All @@ -278,6 +284,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

eventRepository.init();
Expand All @@ -292,29 +299,42 @@ describe('EventRepository', () => {
});
});

it('should handle error if event callback function throws', () => {
const mockErrorHandler = {
onError: jest.fn(),
} as unknown as IErrorHandler;

it('should log an error if the event callback function is not a function', () => {
const eventRepository = new EventRepository(
mockPluginsManager,
defaultStoreManager,
mockErrorHandler,
defaultErrorHandler,
defaultLogger,
);

eventRepository.init();

eventRepository.enqueue(testEvent, 'invalid-callback' as any);

expect(defaultLogger.error).toHaveBeenCalledTimes(1);
expect(defaultLogger.error).toHaveBeenCalledWith(
'TrackAPI:: The provided callback parameter is not a function.',
);
});

it('should handle error if event callback function throws', () => {
const eventRepository = new EventRepository(
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

eventRepository.init();
const mockEventCallback = jest.fn(() => {
throw new Error('test error');
});
eventRepository.enqueue(testEvent, mockEventCallback);

expect(mockErrorHandler.onError).toBeCalledTimes(1);
expect(mockErrorHandler.onError).toBeCalledWith(
expect(defaultLogger.error).toHaveBeenCalledTimes(1);
expect(defaultLogger.error).toHaveBeenCalledWith(
'TrackAPI:: The callback threw an exception',
new Error('test error'),
'EventRepository',
'API Callback Invocation Failed',
);
});

Expand All @@ -323,6 +343,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

state.consents.preConsent.value = {
Expand All @@ -346,6 +367,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);
eventRepository.init();

Expand All @@ -358,6 +380,7 @@ describe('EventRepository', () => {
mockPluginsManager,
defaultStoreManager,
defaultErrorHandler,
defaultLogger,
);

state.consents.postConsent.value.discardPreConsentEvents = true;
Expand Down
Loading

0 comments on commit bc050e6

Please sign in to comment.