Skip to content

Commit

Permalink
feat: warn users on missing plugins (#1691)
Browse files Browse the repository at this point in the history
* feat: warn users on missing plugins

* feat: structure the log messages

* feat: add data plane events state

* refactor: merge plugin filtering and configuration checking logic

* refactor: clean up

* test: add tests for updateDataPlaneEventsStateFromLoadOptions

* fix: use types in signal creation

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix: add missing import

* fix: add tests and refactor code

* test: update descriptions as per coderabbitai suggestions

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
  • Loading branch information
saikumarrs and coderabbitai[bot] committed Apr 25, 2024
1 parent bb7e1df commit c57cf82
Show file tree
Hide file tree
Showing 14 changed files with 517 additions and 94 deletions.
6 changes: 6 additions & 0 deletions packages/analytics-js-common/src/types/ApplicationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ export type MetricsState = {
triggered: Signal<number>;
};

export type DataPlaneEventsState = {
eventsQueuePluginName: Signal<PluginName | undefined>;
readonly deliveryEnabled: Signal<boolean>;
};

export type NativeDestinationsState = {
configuredDestinations: Signal<Destination[]>;
activeDestinations: Signal<Destination[]>;
Expand Down Expand Up @@ -167,6 +172,7 @@ export interface ApplicationState {
session: SessionState;
source: SourceConfigState;
storage: StorageState;
dataPlaneEvents: DataPlaneEventsState;
}

export type DebouncedFunction = (...args: any[]) => void;
4 changes: 2 additions & 2 deletions packages/analytics-js/.size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports = [
{
name: 'Core - NPM',
path: 'dist/npm/modern/umd/index.js',
limit: '23 KiB',
limit: '23.5 KiB',
},
{
name: 'Core Legacy - CDN',
Expand All @@ -26,6 +26,6 @@ module.exports = [
{
name: 'Core - CDN',
path: 'dist/cdn/modern/iife/rsa.min.js',
limit: '23 KiB',
limit: '23.5 KiB',
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
updateStorageStateFromLoadOptions,
updateConsentsStateFromLoadOptions,
updateConsentsState,
updateDataPlaneEventsStateFromLoadOptions,
} from '../../../src/components/configManager/util/commonUtil';
import { state, resetState } from '../../../src/state';

Expand Down Expand Up @@ -528,4 +529,49 @@ describe('Config Manager Common Utilities', () => {
expect(state.consents.resolutionStrategy.value).toBe('and'); // default value
});
});

describe('updateDataPlaneEventsStateFromLoadOptions', () => {
beforeEach(() => {
resetState();
});

it('should not set the events queue plugin name if events delivery is disabled', () => {
state.dataPlaneEvents.deliveryEnabled.value = false;

updateDataPlaneEventsStateFromLoadOptions(mockLogger);

expect(state.dataPlaneEvents.eventsQueuePluginName.value).toBeUndefined();
});

it('should set the events queue plugin name to XhrQueue by default', () => {
updateDataPlaneEventsStateFromLoadOptions(mockLogger);

expect(state.dataPlaneEvents.eventsQueuePluginName.value).toMatch('XhrQueue');
});

it('should set the events queue plugin name to BeaconQueue if beacon transport is selected', () => {
state.loadOptions.value.useBeacon = true;

// Force set the beacon availability
state.capabilities.isBeaconAvailable.value = true;

updateDataPlaneEventsStateFromLoadOptions(mockLogger);

expect(state.dataPlaneEvents.eventsQueuePluginName.value).toMatch('BeaconQueue');
});

it('should set the events queue plugin name to XhrQueue if beacon transport is selected but not available', () => {
state.loadOptions.value.useBeacon = true;

// Force set the beacon availability to false
state.capabilities.isBeaconAvailable.value = false;

updateDataPlaneEventsStateFromLoadOptions(mockLogger);

expect(state.dataPlaneEvents.eventsQueuePluginName.value).toMatch('XhrQueue');
expect(mockLogger.warn).toHaveBeenCalledWith(
'ConfigManager:: The Beacon API is not supported by your browser. The events will be sent using XHR instead.',
);
});
});
});
14 changes: 10 additions & 4 deletions packages/analytics-js/__tests__/components/core/Analytics.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { IPluginsManager } from '@rudderstack/analytics-js-common/types/PluginsManager';
import { IEventManager } from '@rudderstack/analytics-js/components/eventManager/types';
import { IUserSessionManager } from '@rudderstack/analytics-js/components/userSessionManager/types';
import { IStoreManager } from '@rudderstack/analytics-js-common/types/Store';
import type { IPluginsManager } from '@rudderstack/analytics-js-common/types/PluginsManager';
import type { IEventManager } from '@rudderstack/analytics-js/components/eventManager/types';
import type { IUserSessionManager } from '@rudderstack/analytics-js/components/userSessionManager/types';
import type { IStoreManager } from '@rudderstack/analytics-js-common/types/Store';
import { USER_SESSION_STORAGE_KEYS } from '@rudderstack/analytics-js/components/userSessionManager/constants';
import { batch } from '@preact/signals-core';
import {
entriesWithMixStorage,
entriesWithOnlyCookieStorage,
Expand Down Expand Up @@ -50,6 +51,11 @@ describe('Core - Analytics', () => {

describe('startLifecycle', () => {
it('should call expected methods in different state status', () => {
batch(() => {
state.lifecycle.writeKey.value = dummyWriteKey;
state.lifecycle.dataPlaneUrl.value = 'https://dummy.dataplane.url';
});

analytics.startLifecycle();
const onMountedSpy = jest.spyOn(analytics, 'onMounted');
const loadConfigSpy = jest.spyOn(analytics, 'loadConfig');
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
import { PluginsManager } from '../../../src/components/pluginsManager';
import { defaultErrorHandler } from '../../../src/services/ErrorHandler';
import { defaultLogger } from '../../../src/services/Logger';
import { defaultPluginEngine } from '../../../src/services/PluginEngine';
import { state, resetState } from '../../../src/state';
import { defaultOptionalPluginsList } from '../../../src/components/pluginsManager/defaultPluginsList';

let pluginsManager: PluginsManager;

describe('PluginsManager', () => {
beforeAll(() => {
defaultLogger.warn = jest.fn();
defaultLogger.error = jest.fn();
});

afterAll(() => {
jest.clearAllMocks();
});

describe('getPluginsToLoadBasedOnConfig', () => {
beforeEach(() => {
resetState();

pluginsManager = new PluginsManager(defaultPluginEngine, defaultErrorHandler, defaultLogger);

state.plugins.pluginsToLoadFromConfig.value = defaultOptionalPluginsList;
});

it('should return empty array if plugins list is set to undefined in the state', () => {
state.plugins.pluginsToLoadFromConfig.value = undefined;

expect(pluginsManager.getPluginsToLoadBasedOnConfig()).toEqual([]);
});

it('should return the default optional plugins if no plugins were configured in the state', () => {
// All other plugins require some state variables to be set which by default are not set
expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
['ExternalAnonymousId', 'GoogleLinker'].sort(),
);
});

it('should not filter the data plane queue plugin if it is automatically configured', () => {
state.dataPlaneEvents.eventsQueuePluginName.value = 'XhrQueue';

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
['XhrQueue', 'ExternalAnonymousId', 'GoogleLinker'].sort(),
);
});

it('should add the data plane queue plugin if it is not configured through the plugins input', () => {
state.plugins.pluginsToLoadFromConfig.value = [];
state.dataPlaneEvents.eventsQueuePluginName.value = 'XhrQueue';

expect(pluginsManager.getPluginsToLoadBasedOnConfig()).toEqual(['XhrQueue']);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Data plane events delivery is enabled, but 'XhrQueue' plugin was not configured to load. So, the plugin will be loaded automatically.",
);
});

it('should not filter the error reporting plugins if it is configured to load by default', () => {
state.reporting.errorReportingProviderPluginName.value = 'Bugsnag';

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
['ErrorReporting', 'Bugsnag', 'ExternalAnonymousId', 'GoogleLinker'].sort(),
);
});

it('should filter the error reporting plugins if they are not configured through the plugins input', () => {
state.reporting.errorReportingProviderPluginName.value = 'Bugsnag';
state.plugins.pluginsToLoadFromConfig.value = [];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual([]);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Error reporting is enabled, but ['ErrorReporting', 'Bugsnag'] plugins were not configured to load. Ignore if this was intentional. Otherwise, consider adding them to the 'plugins' load API option.",
);
});

it('should log a warning even if the error reporting plugins were partially not configured', () => {
state.reporting.errorReportingProviderPluginName.value = 'Bugsnag';
// Only ErrorReporting is configured
state.plugins.pluginsToLoadFromConfig.value = ['ErrorReporting'];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(['ErrorReporting']);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Error reporting is enabled, but 'Bugsnag' plugin was not configured to load. Ignore if this was intentional. Otherwise, consider adding it to the 'plugins' load API option.",
);
});

it('should not filter the device mode destination plugins if they are configured', () => {
// Non-empty array
state.nativeDestinations.configuredDestinations.value = [
{
config: {
connectionMode: 'device',
},
},
];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
[
'DeviceModeDestinations',
'NativeDestinationQueue',
'ExternalAnonymousId',
'GoogleLinker',
].sort(),
);
});

it('should filter the device mode destination plugins if they are not configured through the plugins input', () => {
// Non-empty array
state.nativeDestinations.configuredDestinations.value = [
{
config: {
connectionMode: 'device',
},
},
];
state.plugins.pluginsToLoadFromConfig.value = [];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual([]);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Device mode destinations are connected to the source, but ['DeviceModeDestinations', 'NativeDestinationQueue'] plugins were not configured to load. Ignore if this was intentional. Otherwise, consider adding them to the 'plugins' load API option.",
);
});

it('should log a warning even if the device mode destination plugins were partially not configured', () => {
// Non-empty array
state.nativeDestinations.configuredDestinations.value = [
{
config: {
connectionMode: 'device',
},
},
];
// Only DeviceModeDestinations is configured
state.plugins.pluginsToLoadFromConfig.value = ['DeviceModeDestinations'];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual([
'DeviceModeDestinations',
]);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Device mode destinations are connected to the source, but 'NativeDestinationQueue' plugin was not configured to load. Ignore if this was intentional. Otherwise, consider adding it to the 'plugins' load API option.",
);
});

it('should not filter device mode transformation plugin if it is configured to load by default', () => {
// At least one device mode destination is configured
state.nativeDestinations.configuredDestinations.value = [
{
config: {
connectionMode: 'device',
},
},
{
config: {
connectionMode: 'device',
},
shouldApplyDeviceModeTransformation: true,
},
];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
[
'DeviceModeTransformation',
'DeviceModeDestinations',
'NativeDestinationQueue',
'ExternalAnonymousId',
'GoogleLinker',
].sort(),
);
});

it('should filter device mode transformation plugin if it is not configured through the plugins input', () => {
// At least one device mode destination is configured
state.nativeDestinations.configuredDestinations.value = [
{
config: {
connectionMode: 'device',
},
shouldApplyDeviceModeTransformation: true,
},
];
state.plugins.pluginsToLoadFromConfig.value = [
'DeviceModeDestinations',
'NativeDestinationQueue',
];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
['DeviceModeDestinations', 'NativeDestinationQueue'].sort(),
);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Device mode transformations are enabled for at least one destination, but 'DeviceModeTransformation' plugin was not configured to load. Ignore if this was intentional. Otherwise, consider adding it to the 'plugins' load API option.",
);
});

it('should not filter storage encryption plugin if it is configured to load by default', () => {
state.storage.encryptionPluginName.value = 'StorageEncryption';

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
['StorageEncryption', 'ExternalAnonymousId', 'GoogleLinker'].sort(),
);
});

it('should filter storage encryption plugin if it is not configured through the plugins input', () => {
state.storage.encryptionPluginName.value = 'StorageEncryption';
state.plugins.pluginsToLoadFromConfig.value = [];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual([]);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Storage encryption is enabled, but 'StorageEncryption' plugin was not configured to load. Ignore if this was intentional. Otherwise, consider adding it to the 'plugins' load API option.",
);
});

it('should not filter storage migrator plugin if it is configured to load by default', () => {
state.storage.migrate.value = true;

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual(
['StorageMigrator', 'ExternalAnonymousId', 'GoogleLinker'].sort(),
);
});

it('should filter storage migrator plugin if it is not configured through the plugins input', () => {
state.storage.migrate.value = true;
state.plugins.pluginsToLoadFromConfig.value = [];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual([]);

// Expect a warning for user not explicitly configuring it
expect(defaultLogger.warn).toHaveBeenCalledTimes(1);
expect(defaultLogger.warn).toHaveBeenCalledWith(
"PluginsManager:: Storage migration is enabled, but 'StorageMigrator' plugin was not configured to load. Ignore if this was intentional. Otherwise, consider adding it to the 'plugins' load API option.",
);
});

it('should not log any warning if logger is not supplied', () => {
pluginsManager = new PluginsManager(defaultPluginEngine, defaultErrorHandler);

// Checking only for the migration plugin
state.storage.migrate.value = true;
state.plugins.pluginsToLoadFromConfig.value = [];

expect(pluginsManager.getPluginsToLoadBasedOnConfig().sort()).toEqual([]);
expect(defaultLogger.warn).not.toHaveBeenCalled();
});
});
});

0 comments on commit c57cf82

Please sign in to comment.