Skip to content

Commit

Permalink
test: address ai bot review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
saikumarrs committed Jan 10, 2025
1 parent 0ecb545 commit 8451e77
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -604,16 +604,16 @@ describe('Error Reporting utilities', () => {
});

describe('isAllowedToBeNotified', () => {
it('should return true if the error is allowed to be notified', () => {
const result = isAllowedToBeNotified({ message: 'dummy error' } as unknown as Exception);
expect(result).toBeTruthy();
});
const testCases = [
['dummy error', true, 'should allow generic errors'],
['The request failed', false, 'should not allow request failures'],
['', true, 'should allow empty messages'],
['Network request failed', true, 'should allow network errors'],
];

it('should return false if the error is not allowed to be notified', () => {
const result = isAllowedToBeNotified({
message: 'The request failed',
} as unknown as Exception);
expect(result).toBeFalsy();
test.each(testCases)('%s -> %s (%s)', (message, expected, testName) => {
const result = isAllowedToBeNotified({ message } as unknown as Exception);
expect(result).toBe(expected);
});
});

Expand Down
24 changes: 10 additions & 14 deletions packages/analytics-js/src/components/core/Analytics.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
import { ExternalSrcLoader } from '@rudderstack/analytics-js-common/services/ExternalSrcLoader';
import { batch, effect } from '@preact/signals-core';
import { isDefined, isFunction, isNull } from '@rudderstack/analytics-js-common/utilities/checks';
import { isFunction, isNull } from '@rudderstack/analytics-js-common/utilities/checks';
import type { IHttpClient } from '@rudderstack/analytics-js-common/types/HttpClient';
import { clone } from 'ramda';
import type { ILogger } from '@rudderstack/analytics-js-common/types/Logger';
Expand Down Expand Up @@ -69,6 +69,7 @@ import {
import type { IAnalytics } from './IAnalytics';
import { getConsentManagementData, getValidPostConsentOptions } from '../utilities/consent';
import { dispatchSDKEvent, isDataPlaneUrlValid, isWriteKeyValid } from './utilities';
import { safelyInvokeCallback } from '../utilities/callbacks';

/*
* Analytics class with lifecycle based on state ad user triggered events
Expand Down Expand Up @@ -322,19 +323,14 @@ class Analytics implements IAnalytics {

// Execute onLoaded callback if provided in load options
const onLoadedCallbackFn = state.loadOptions.value.onLoaded;
if (isDefined(onLoadedCallbackFn)) {
if (isFunction(onLoadedCallbackFn)) {
// TODO: we need to avoid passing the window object to the callback function
// as this will prevent us from supporting multiple SDK instances in the same page
try {
onLoadedCallbackFn((globalThis as typeof window).rudderanalytics);
} catch (err) {
this.logger.error(CALLBACK_INVOKE_ERROR(LOAD_API), err);
}
} else {
this.logger.error(INVALID_CALLBACK_FN_ERROR(LOAD_API));
}
}
// TODO: we need to avoid passing the window object to the callback function
// as this will prevent us from supporting multiple SDK instances in the same page
safelyInvokeCallback(
onLoadedCallbackFn,
[(globalThis as typeof window).rudderanalytics],
LOAD_API,
this.logger,
);

// Set lifecycle state
batch(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import type { ApiCallback } from '@rudderstack/analytics-js-common/types/EventAp
import { isHybridModeDestination } from '@rudderstack/analytics-js-common/utilities/destinations';
import { API_SUFFIX } from '@rudderstack/analytics-js-common/constants/loggerContexts';
import type { Destination } from '@rudderstack/analytics-js-common/types/Destination';
import { isDefined, isFunction } from '@rudderstack/analytics-js-common/utilities/checks';
import { CALLBACK_INVOKE_ERROR, INVALID_CALLBACK_FN_ERROR } from '../../constants/logMessages';
import { state } from '../../state';
import type { IEventRepository } from './types';
import {
Expand All @@ -20,6 +18,7 @@ import {
DMT_EXT_POINT_PREFIX,
} from './constants';
import { getFinalEvent, shouldBufferEventsForPreConsent } from './utils';
import { safelyInvokeCallback } from '../utilities/callbacks';

/**
* Event repository class responsible for queuing events for further processing and delivery
Expand Down Expand Up @@ -170,20 +169,8 @@ class EventRepository implements IEventRepository {
);

// Invoke the callback if it exists
if (isDefined(callback)) {
const apiName = `${event.type.charAt(0).toUpperCase()}${event.type.slice(1)}${API_SUFFIX}`;
if (isFunction(callback)) {
try {
// Using the event sent to the data plane queue here
// to ensure the mutated (if any) event is sent to the callback
callback(dpQEvent);
} catch (error) {
this.logger.error(CALLBACK_INVOKE_ERROR(apiName), error);
}
} else {
this.logger.error(INVALID_CALLBACK_FN_ERROR(apiName));
}
}
const apiName = `${event.type.charAt(0).toUpperCase()}${event.type.slice(1)}${API_SUFFIX}`;
safelyInvokeCallback(callback, [dpQEvent], apiName, this.logger);
}
}

Expand Down
26 changes: 26 additions & 0 deletions packages/analytics-js/src/components/utilities/callbacks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import type { ILogger } from '@rudderstack/analytics-js-common/types/Logger';
import { isDefined, isFunction } from '@rudderstack/analytics-js-common/utilities/checks';
import { CALLBACK_INVOKE_ERROR, INVALID_CALLBACK_FN_ERROR } from '../../constants/logMessages';

const safelyInvokeCallback = (
callback: unknown,
args: unknown[],
apiName: string,
logger: ILogger,
): void => {
if (!isDefined(callback)) {
return;
}

if (isFunction(callback)) {
try {
callback(...args);
} catch (error) {
logger.error(CALLBACK_INVOKE_ERROR(apiName), error);
}
} else {
logger.error(INVALID_CALLBACK_FN_ERROR(apiName));
}
};

export { safelyInvokeCallback };

0 comments on commit 8451e77

Please sign in to comment.