diff --git a/lib/event_processor/forwarding_event_processor.ts b/lib/event_processor/forwarding_event_processor.ts index d516afe7c..67899bddb 100644 --- a/lib/event_processor/forwarding_event_processor.ts +++ b/lib/event_processor/forwarding_event_processor.ts @@ -23,7 +23,7 @@ import { buildLogEvent } from './event_builder/log_event'; import { BaseService, ServiceState } from '../service'; import { EventEmitter } from '../utils/event_emitter/event_emitter'; import { Consumer, Fn } from '../utils/type'; -import { SERVICE_STOPPED_BEFORE_IT_WAS_STARTED } from 'error_message'; +import { SERVICE_STOPPED_BEFORE_RUNNING } from 'error_message'; import { OptimizelyError } from '../error/optimizly_error'; class ForwardingEventProcessor extends BaseService implements EventProcessor { private dispatcher: EventDispatcher; @@ -56,7 +56,7 @@ class ForwardingEventProcessor extends BaseService implements EventProcessor { } if (this.isNew()) { - this.startPromise.reject(new OptimizelyError(SERVICE_STOPPED_BEFORE_IT_WAS_STARTED)); + this.startPromise.reject(new OptimizelyError(SERVICE_STOPPED_BEFORE_RUNNING)); } this.state = ServiceState.Terminated; diff --git a/lib/message/error_message.ts b/lib/message/error_message.ts index 5b12a5f68..66bf469f7 100644 --- a/lib/message/error_message.ts +++ b/lib/message/error_message.ts @@ -95,7 +95,7 @@ export const DATAFILE_MANAGER_STOPPED = 'Datafile manager stopped before it coul export const FAILED_TO_FETCH_DATAFILE = 'Failed to fetch datafile'; export const NO_SDKKEY_OR_DATAFILE = 'At least one of sdkKey or datafile must be provided'; export const RETRY_CANCELLED = 'Retry cancelled'; -export const SERVICE_STOPPED_BEFORE_IT_WAS_STARTED = 'Service stopped before it was started'; +export const SERVICE_STOPPED_BEFORE_RUNNING = 'Service stopped before running'; export const ONLY_POST_REQUESTS_ARE_SUPPORTED = 'Only POST requests are supported'; export const SEND_BEACON_FAILED = 'sendBeacon failed'; export const FAILED_TO_DISPATCH_EVENTS = 'Failed to dispatch events' diff --git a/lib/optimizely/index.tests.js b/lib/optimizely/index.tests.js index f2a739a04..c4efb2c67 100644 --- a/lib/optimizely/index.tests.js +++ b/lib/optimizely/index.tests.js @@ -37,26 +37,15 @@ import { FEATURE_NOT_ENABLED_FOR_USER, INVALID_CLIENT_ENGINE, INVALID_DEFAULT_DECIDE_OPTIONS, - INVALID_OBJECT, NOT_ACTIVATING_USER, - USER_HAS_NO_FORCED_VARIATION, - USER_HAS_NO_FORCED_VARIATION_FOR_EXPERIMENT, - USER_MAPPED_TO_FORCED_VARIATION, - USER_RECEIVED_DEFAULT_VARIABLE_VALUE, VALID_USER_PROFILE_SERVICE, - VARIATION_REMOVED_FOR_USER, } from 'log_message'; import { - EXPERIMENT_KEY_NOT_IN_DATAFILE, - INVALID_ATTRIBUTES, NOT_TRACKING_USER, EVENT_KEY_NOT_FOUND, INVALID_EXPERIMENT_KEY, - INVALID_INPUT_FORMAT, - NO_VARIATION_FOR_EXPERIMENT_KEY, - USER_NOT_IN_FORCED_VARIATION, - INSTANCE_CLOSED, ONREADY_TIMEOUT, + SERVICE_STOPPED_BEFORE_RUNNING } from 'error_message'; import { @@ -77,6 +66,7 @@ import { } from '../core/decision_service'; import { USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP } from '../core/bucketer'; +import { resolvablePromise } from '../utils/promise/resolvablePromise'; var LOG_LEVEL = enums.LOG_LEVEL; var DECISION_SOURCES = enums.DECISION_SOURCES; @@ -9253,10 +9243,10 @@ describe('lib/optimizely', function() { }); }); - it('returns a promise that fulfills with a successful result object', function() { - return optlyInstance.close().then(function(result) { - assert.deepEqual(result, { success: true }); - }); + it('returns a promise that resolves', function() { + return optlyInstance.close().then().catch(() => { + assert.fail(); + }) }); }); @@ -9291,13 +9281,11 @@ describe('lib/optimizely', function() { }); }); - it('returns a promise that fulfills with an unsuccessful result object', function() { - return optlyInstance.close().then(function(result) { - // assert.deepEqual(result, { - // success: false, - // reason: 'Error: Failed to stop', - // }); - assert.isFalse(result.success); + it('returns a promise that rejects', function() { + return optlyInstance.close().then(() => { + assert.fail('promnise should reject') + }).catch(() => { + }); }); }); @@ -9465,7 +9453,7 @@ describe('lib/optimizely', function() { var readyPromise = optlyInstance.onReady(); clock.tick(300001); return readyPromise.then(() => { - return Promise.reject(new Error(PROMISE_SHOULD_NOT_HAVE_RESOLVED)); + return Promise.reject(new Error('PROMISE_SHOULD_NOT_HAVE_RESOLVED')); }, (err) => { assert.equal(err.baseMessage, ONREADY_TIMEOUT); assert.deepEqual(err.params, [ 30000 ]); @@ -9487,18 +9475,25 @@ describe('lib/optimizely', function() { eventProcessor, }); var readyPromise = optlyInstance.onReady({ timeout: 100 }); + optlyInstance.close(); + return readyPromise.then(() => { - return Promise.reject(new Error(PROMISE_SHOULD_NOT_HAVE_RESOLVED)); + return Promise.reject(new Error('PROMISE_SHOULD_NOT_HAVE_RESOLVED')); }, (err) => { - assert.equal(err.baseMessage, INSTANCE_CLOSED); + assert.equal(err.baseMessage, SERVICE_STOPPED_BEFORE_RUNNING); }); }); it('can be called several times with different timeout values and the returned promises behave correctly', function() { + const onRunning = resolvablePromise(); + optlyInstance = new Optimizely({ clientEngine: 'node-sdk', - projectConfigManager: getMockProjectConfigManager(), + projectConfigManager: getMockProjectConfigManager({ + onRunning: onRunning.promise, + }), + eventProcessor, jsonSchemaValidator: jsonSchemaValidator, logger: createdLogger, @@ -9512,16 +9507,16 @@ describe('lib/optimizely', function() { var readyPromise3 = optlyInstance.onReady({ timeout: 300 }); clock.tick(101); return readyPromise1 - .then(function() { + .catch(function() { clock.tick(100); return readyPromise2; }) - .then(function() { + .catch(function() { // readyPromise3 has not resolved yet because only 201 ms have elapsed. // Calling close on the instance should resolve readyPromise3 - optlyInstance.close(); + optlyInstance.close().catch(() => {}); return readyPromise3; - }); + }).catch(() => {}); }); it('clears the timeout when the project config manager ready promise fulfills', function() { diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 416b4f3c8..9ca0c6089 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -22,7 +22,7 @@ import { OdpManager } from '../odp/odp_manager'; import { VuidManager } from '../vuid/vuid_manager'; import { OdpEvent } from '../odp/event_manager/odp_event'; import { OptimizelySegmentOption } from '../odp/segment_manager/optimizely_segment_option'; -import { BaseService } from '../service'; +import { BaseService, ServiceState } from '../service'; import { UserAttributes, @@ -43,7 +43,7 @@ import { ProjectConfigManager } from '../project_config/project_config_manager'; import { createDecisionService, DecisionService, DecisionObj } from '../core/decision_service'; import { buildLogEvent } from '../event_processor/event_builder/log_event'; import { buildImpressionEvent, buildConversionEvent } from '../event_processor/event_builder/user_event'; -import fns from '../utils/fns'; +import { isSafeInteger } from '../utils/fns'; import { validate } from '../utils/attributes_validator'; import * as eventTagsValidator from '../utils/event_tags_validator'; import * as projectConfig from '../project_config/project_config'; @@ -77,7 +77,8 @@ import { NOT_TRACKING_USER, VARIABLE_REQUESTED_WITH_WRONG_TYPE, ONREADY_TIMEOUT, - INSTANCE_CLOSED + INSTANCE_CLOSED, + SERVICE_STOPPED_BEFORE_RUNNING } from 'error_message'; import { @@ -132,18 +133,13 @@ export type OptimizelyOptions = { disposable?: boolean; } -export default class Optimizely implements Client { - private disposeOnUpdate?: Fn; - private readyPromise: Promise; - // readyTimeout is specified as any to make this work in both browser & Node - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private readyTimeouts: { [key: string]: { readyTimeout: any; onClose: () => void } }; - private nextReadyTimeoutId: number; +export default class Optimizely extends BaseService implements Client { + private cleanupTasks: Map = new Map(); + private nextCleanupTaskId = 0; private clientEngine: string; private clientVersion: string; private errorNotifier?: ErrorNotifier; private errorReporter: ErrorReporter; - protected logger?: LoggerFacade; private projectConfigManager: ProjectConfigManager; private decisionService: DecisionService; private eventProcessor?: EventProcessor; @@ -153,6 +149,8 @@ export default class Optimizely implements Client { private vuidManager?: VuidManager; constructor(config: OptimizelyOptions) { + super(); + let clientEngine = config.clientEngine; if (!clientEngine) { config.logger?.info(INVALID_CLIENT_ENGINE, clientEngine); @@ -193,7 +191,8 @@ export default class Optimizely implements Client { }); this.defaultDecideOptions = defaultDecideOptions; - this.disposeOnUpdate = this.projectConfigManager.onUpdate((configObj: projectConfig.ProjectConfig) => { + this.projectConfigManager = config.projectConfigManager; + this.projectConfigManager.onUpdate((configObj: projectConfig.ProjectConfig) => { this.logger?.info( UPDATED_OPTIMIZELY_CONFIG, configObj.revision, @@ -205,9 +204,14 @@ export default class Optimizely implements Client { this.updateOdpSettings(); }); - this.projectConfigManager.start(); - const projectConfigManagerRunningPromise = this.projectConfigManager.onRunning(); + this.eventProcessor = config.eventProcessor; + this.eventProcessor?.onDispatch((event) => { + this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.LOG_EVENT, event); + }); + this.odpManager = config.odpManager; + + let userProfileService: UserProfileService | null = null; if (config.userProfileService) { try { @@ -228,34 +232,39 @@ export default class Optimizely implements Client { this.notificationCenter = createNotificationCenter({ logger: this.logger, errorNotifier: this.errorNotifier }); - this.eventProcessor = config.eventProcessor; + this.start(); + } - this.eventProcessor?.start(); - const eventProcessorRunningPromise = this.eventProcessor ? this.eventProcessor.onRunning() : - Promise.resolve(undefined); + start(): void { + if (!this.isNew()) { + return; + } - this.eventProcessor?.onDispatch((event) => { - this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.LOG_EVENT, event); - }); + super.start(); + this.state = ServiceState.Starting; + this.projectConfigManager.start(); + this.eventProcessor?.start(); this.odpManager?.start(); - this.readyPromise = Promise.all([ - projectConfigManagerRunningPromise, - eventProcessorRunningPromise, - config.odpManager ? config.odpManager.onRunning() : Promise.resolve(), - config.vuidManager ? config.vuidManager.initialize() : Promise.resolve(), - ]); + Promise.all([ + this.projectConfigManager.onRunning(), + this.eventProcessor ? this.eventProcessor.onRunning() : Promise.resolve(), + this.odpManager ? this.odpManager.onRunning() : Promise.resolve(), + this.vuidManager ? this.vuidManager.initialize() : Promise.resolve(), + ]).then(() => { + this.state = ServiceState.Running; + this.startPromise.resolve(); - this.readyPromise.then(() => { const vuid = this.vuidManager?.getVuid(); if (vuid) { this.odpManager?.setVuid(vuid); } + }).catch((err) => { + this.state = ServiceState.Failed; + this.errorReporter.report(err); + this.startPromise.reject(err); }); - - this.readyTimeouts = {}; - this.nextReadyTimeoutId = 0; } /** @@ -1220,62 +1229,47 @@ export default class Optimizely implements Client { * above) are complete. If there are no in-flight event dispatcher requests and * no queued events waiting to be sent, returns an immediately-fulfilled Promise. * - * Returned Promises are fulfilled with result objects containing these - * properties: - * - success (boolean): true if the event dispatcher signaled completion of - * all in-flight and final requests, or if there were no - * queued events and no in-flight requests. false if an - * unexpected error was encountered during the close - * process. - * - reason (string=): If success is false, this is a string property with - * an explanatory message. * * NOTE: After close is called, this instance is no longer usable - any events * generated will no longer be sent to the event dispatcher. * * @return {Promise} */ - close(): Promise<{ success: boolean; reason?: string }> { - try { - this.projectConfigManager.stop(); - this.eventProcessor?.stop(); - this.odpManager?.stop(); - this.notificationCenter.clearAllNotificationListeners(); - - const eventProcessorStoppedPromise = this.eventProcessor ? this.eventProcessor.onTerminated() : - Promise.resolve(); - - if (this.disposeOnUpdate) { - this.disposeOnUpdate(); - this.disposeOnUpdate = undefined; - } + close(): Promise { + this.stop(); + return this.onTerminated(); + } - Object.keys(this.readyTimeouts).forEach((readyTimeoutId: string) => { - const readyTimeoutRecord = this.readyTimeouts[readyTimeoutId]; - clearTimeout(readyTimeoutRecord.readyTimeout); - readyTimeoutRecord.onClose(); - }); - this.readyTimeouts = {}; - return eventProcessorStoppedPromise.then( - function() { - return { - success: true, - }; - }, - function(err) { - return { - success: false, - reason: String(err), - }; - } - ); - } catch (err) { - this.errorReporter.report(err); - return Promise.resolve({ - success: false, - reason: String(err), - }); + stop(): void { + if (this.isDone()) { + return; } + + if (!this.isRunning()) { + this.startPromise.reject(new OptimizelyError(SERVICE_STOPPED_BEFORE_RUNNING)); + } + + this.state = ServiceState.Stopping; + + this.projectConfigManager.stop(); + this.eventProcessor?.stop(); + this.odpManager?.stop(); + this.notificationCenter.clearAllNotificationListeners(); + + this.cleanupTasks.forEach((onClose) => onClose()); + + Promise.all([ + this.projectConfigManager.onTerminated(), + this.eventProcessor ? this.eventProcessor.onTerminated() : Promise.resolve(), + this.odpManager ? this.odpManager.onTerminated() : Promise.resolve(), + ]).then(() => { + this.state = ServiceState.Terminated; + this.stopPromise.resolve() + }).catch((err) => { + this.errorReporter.report(err); + this.state = ServiceState.Failed; + this.stopPromise.reject(err); + }); } /** @@ -1312,35 +1306,30 @@ export default class Optimizely implements Client { timeoutValue = options.timeout; } } - if (!fns.isSafeInteger(timeoutValue)) { + if (!isSafeInteger(timeoutValue)) { timeoutValue = DEFAULT_ONREADY_TIMEOUT; } const timeoutPromise = resolvablePromise(); - const timeoutId = this.nextReadyTimeoutId++; + const cleanupTaskId = this.nextCleanupTaskId++; const onReadyTimeout = () => { - delete this.readyTimeouts[timeoutId]; + this.cleanupTasks.delete(cleanupTaskId); timeoutPromise.reject(new OptimizelyError(ONREADY_TIMEOUT, timeoutValue)); }; const readyTimeout = setTimeout(onReadyTimeout, timeoutValue); - const onClose = function() { - timeoutPromise.reject(new OptimizelyError(INSTANCE_CLOSED)); - }; - - this.readyTimeouts[timeoutId] = { - readyTimeout: readyTimeout, - onClose: onClose, - }; - - this.readyPromise.then(() => { + + this.cleanupTasks.set(cleanupTaskId, () => { clearTimeout(readyTimeout); - delete this.readyTimeouts[timeoutId]; + timeoutPromise.reject(new OptimizelyError(INSTANCE_CLOSED)); }); - return Promise.race([this.readyPromise, timeoutPromise]); + return Promise.race([this.onRunning().then(() => { + clearTimeout(readyTimeout); + this.cleanupTasks.delete(cleanupTaskId); + }), timeoutPromise]); } //============ decide ============// @@ -1363,12 +1352,19 @@ export default class Optimizely implements Client { return null; } - return new OptimizelyUserContext({ + const userContext = new OptimizelyUserContext({ optimizely: this, userId: userIdentifier, attributes, - shouldIdentifyUser: true, }); + + this.onRunning().then(() => { + if (this.odpManager && this.isOdpIntegrated()) { + this.odpManager.identifyUser(userIdentifier); + } + }).catch(() => {}); + + return userContext; } /** @@ -1387,7 +1383,6 @@ export default class Optimizely implements Client { optimizely: this, userId, attributes, - shouldIdentifyUser: false, }); } @@ -1668,17 +1663,6 @@ export default class Optimizely implements Client { return this.getProjectConfig()?.odpIntegrationConfig?.integrated ?? false; } - /** - * Identifies user with ODP server in a fire-and-forget manner. - * Should be called only after the instance is ready - * @param {string} userId - */ - public identifyUser(userId: string): void { - if (this.odpManager && this.isOdpIntegrated()) { - this.odpManager.identifyUser(userId); - } - } - /** * Fetches list of qualified segments from ODP for a particular userId. * @param {string} userId diff --git a/lib/optimizely_user_context/index.ts b/lib/optimizely_user_context/index.ts index b2a524a5e..46fa103f4 100644 --- a/lib/optimizely_user_context/index.ts +++ b/lib/optimizely_user_context/index.ts @@ -30,7 +30,6 @@ interface OptimizelyUserContextConfig { optimizely: Optimizely; userId: string; attributes?: UserAttributes; - shouldIdentifyUser?: boolean; } export interface IOptimizelyUserContext { @@ -57,25 +56,11 @@ export default class OptimizelyUserContext implements IOptimizelyUserContext { private forcedDecisionsMap: { [key: string]: { [key: string]: OptimizelyForcedDecision } }; private _qualifiedSegments: string[] | null = null; - constructor({ optimizely, userId, attributes, shouldIdentifyUser = true }: OptimizelyUserContextConfig) { + constructor({ optimizely, userId, attributes }: OptimizelyUserContextConfig) { this.optimizely = optimizely; this.userId = userId; this.attributes = { ...attributes } ?? {}; this.forcedDecisionsMap = {}; - - if (shouldIdentifyUser) { - this.optimizely.onReady().then(() => { - this.identifyUser(); - }).catch(() => {}); - } - } - - /** - * On user context instantiation, fire event to attempt to identify user to ODP. - * Note: This fails if ODP is not enabled. - */ - private identifyUser(): void { - this.optimizely.identifyUser(this.userId); } /** @@ -235,7 +220,6 @@ export default class OptimizelyUserContext implements IOptimizelyUserContext { private cloneUserContext(): OptimizelyUserContext { const userContext = new OptimizelyUserContext({ - shouldIdentifyUser: false, optimizely: this.getOptimizely(), userId: this.getUserId(), attributes: this.getAttributes(), diff --git a/lib/service.ts b/lib/service.ts index 03e23ee67..b024ef510 100644 --- a/lib/service.ts +++ b/lib/service.ts @@ -66,7 +66,7 @@ export abstract class BaseService implements Service { this.startPromise = resolvablePromise(); this.stopPromise = resolvablePromise(); this.startupLogs = startupLogs; - + // avoid unhandled promise rejection this.startPromise.promise.catch(() => {}); this.stopPromise.promise.catch(() => {}); diff --git a/lib/shared_types.ts b/lib/shared_types.ts index fe62e6471..0cb41e6d0 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -319,7 +319,7 @@ export interface Client { ): { [variableKey: string]: unknown } | null; getOptimizelyConfig(): OptimizelyConfig | null; onReady(options?: { timeout?: number }): Promise; - close(): Promise<{ success: boolean; reason?: string }>; + close(): Promise; sendOdpEvent(action: string, type?: string, identifiers?: Map, data?: Map): void; getProjectConfig(): ProjectConfig | null; isOdpIntegrated(): boolean; diff --git a/lib/utils/fns/index.ts b/lib/utils/fns/index.ts index 3a427f5dc..7d9506756 100644 --- a/lib/utils/fns/index.ts +++ b/lib/utils/fns/index.ts @@ -21,7 +21,7 @@ export function currentTimestamp(): number { return Math.round(new Date().getTime()); } -function isSafeInteger(number: unknown): boolean { +export function isSafeInteger(number: unknown): boolean { return typeof number == 'number' && Math.abs(number) <= MAX_SAFE_INTEGER_LIMIT; }