From 22eb773fcdfcbdb9656127bd6dcf59abd087e76e Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 13 Nov 2026 19:29:00 +0600 Subject: [PATCH 1/2] [FSSDK-13432] Don't treat the first invalid datafile from datafileManager as fatal Previously, if the first datafile emitted by the datafileManager was invalid while the ProjectConfigManager was starting, it was treated as a fatal init error. Now startup fails only after the datafileManager has emitted MAX_INVALID_DATAFILE_EMIT (3) invalid datafiles before a valid one. Also make jsonSchemaValidator and datafileManager private. --- .../project_config_manager.spec.ts | 76 ++++++++++++++++--- lib/project_config/project_config_manager.ts | 19 +++-- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/lib/project_config/project_config_manager.spec.ts b/lib/project_config/project_config_manager.spec.ts index 8fb63dc98..409ce12eb 100644 --- a/lib/project_config/project_config_manager.spec.ts +++ b/lib/project_config/project_config_manager.spec.ts @@ -1,5 +1,5 @@ /** - * Copyright 2019-2020, 2022, 2024, Optimizely + * Copyright 2019-2020, 2022, 2024, 2026, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,8 @@ import { resolvablePromise } from '../utils/promise/resolvablePromise'; import { getMockDatafileManager } from '../tests/mock/mock_datafile_manager'; import { wait } from '../tests/testUtils'; +import { MAX_INVALID_DATAFILE_EMIT } from './project_config_manager'; + describe('ProjectConfigManagerImpl', () => { describe('a logger is passed in the constructor', () => { it('should set name on the logger passed into the constructor', () => { @@ -201,14 +203,14 @@ describe('ProjectConfigManagerImpl', () => { describe('when datafile is invalid', () => { it('should reject onRunning() with the same error if datafileManager.onRunning() rejects', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.reject(new Error('test error')) }); - const manager = new ProjectConfigManagerImpl({ datafile: JSON.stringify({}), datafileManager }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); manager.start(); await expect(manager.onRunning()).rejects.toThrow('DatafileManager failed to start, reason: test error'); }); it('should resolve onRunning() if datafileManager.onUpdate() is fired and should update config', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); - const manager = new ProjectConfigManagerImpl({ datafile: JSON.stringify({}), datafileManager }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); manager.start(); datafileManager.pushUpdate(JSON.stringify(testData.getTestProjectConfig())); await expect(manager.onRunning()).resolves.not.toThrow(); @@ -217,7 +219,7 @@ describe('ProjectConfigManagerImpl', () => { it('should resolve onRunning(), update config and call onUpdate listeners if datafileManager.onUpdate() is fired', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); - const manager = new ProjectConfigManagerImpl({ datafile: JSON.stringify({}), datafileManager }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); manager.start(); const listener = vi.fn(); manager.onUpdate(listener); @@ -230,20 +232,52 @@ describe('ProjectConfigManagerImpl', () => { it('should return undefined from getConfig() before onRunning() resolves', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); - const manager = new ProjectConfigManagerImpl({ datafile: JSON.stringify({}), datafileManager }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); manager.start(); expect(manager.getConfig()).toBeUndefined(); }); it('should return the correct config from getConfig() after onRunning() resolves', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); - const manager = new ProjectConfigManagerImpl({ datafile: JSON.stringify({}), datafileManager }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); manager.start(); datafileManager.pushUpdate(JSON.stringify(testData.getTestProjectConfig())); await expect(manager.onRunning()).resolves.not.toThrow(); expect(manager.getConfig()).toEqual(createProjectConfig(JSON.stringify(testData.getTestProjectConfig()))); }); + + it('should reject onRunning() and onTerminated() if datafileManger emits invalid datafile \ + MAX_INVALID_DATAFILE_EMIT times before running', async () => { + const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); + manager.start(); + + for(let i = 0; i < MAX_INVALID_DATAFILE_EMIT; i++) { + // emit invalid datafile + datafileManager.pushUpdate('ab'); + } + + await expect(manager.onRunning()).rejects.toThrow(); + await expect(manager.onTerminated()).rejects.toThrow(); + }); + + it('should resolve onRunning and update config when datafile manger emits valid datafile before \ + emitting inbalid datafile MAX_INVALID_DATAFILE_EMIT times', async () => { + const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); + const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); + manager.start(); + + for(let i = 0; i < MAX_INVALID_DATAFILE_EMIT - 1; i++) { + // emit invalid datafile + datafileManager.pushUpdate('ab'); + } + + datafileManager.pushUpdate(JSON.stringify(testData.getTestProjectConfig())); + + await expect(manager.onRunning()).resolves.not.toThrow(); + expect(manager.getConfig()).toEqual(createProjectConfig(JSON.stringify(testData.getTestProjectConfig()))); + }); }); }); @@ -255,16 +289,40 @@ describe('ProjectConfigManagerImpl', () => { await expect(manager.onRunning()).rejects.toThrow('DatafileManager failed to start, reason: test error'); }); - it('should reject onRunning() and onTerminated if datafileManager emits an invalid datafile in the first onUpdate', async () => { + it('should reject onRunning() and onTerminated() if datafileManger emits invalid datafile \ + MAX_INVALID_DATAFILE_EMIT times before running', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); const manager = new ProjectConfigManagerImpl({ datafileManager }); manager.start(); - datafileManager.pushUpdate('foo'); + + for(let i = 0; i < MAX_INVALID_DATAFILE_EMIT; i++) { + // emit invalid datafile + datafileManager.pushUpdate('ab'); + } + await expect(manager.onRunning()).rejects.toThrow(); await expect(manager.onTerminated()).rejects.toThrow(); }); - it('should resolve onRunning(), update config and call onUpdate listeners if datafileManager.onUpdate() is fired', async () => { + it('should resolve onRunning and update config when datafile manger emits valid datafile before \ + emitting inbalid datafile MAX_INVALID_DATAFILE_EMIT times', async () => { + const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); + const manager = new ProjectConfigManagerImpl({ datafileManager }); + manager.start(); + + for(let i = 0; i < MAX_INVALID_DATAFILE_EMIT - 1; i++) { + // emit invalid datafile + datafileManager.pushUpdate('ab'); + } + + datafileManager.pushUpdate(JSON.stringify(testData.getTestProjectConfig())); + + await expect(manager.onRunning()).resolves.not.toThrow(); + expect(manager.getConfig()).toEqual(createProjectConfig(JSON.stringify(testData.getTestProjectConfig()))); + }); + + it('should resolve onRunning(), update config and call onUpdate listeners if datafileManager.onUpdate() is fired \ + with valid datafile on the first call', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); const manager = new ProjectConfigManagerImpl({ datafileManager }); manager.start(); diff --git a/lib/project_config/project_config_manager.ts b/lib/project_config/project_config_manager.ts index 0985c5c3a..0c11aa31d 100644 --- a/lib/project_config/project_config_manager.ts +++ b/lib/project_config/project_config_manager.ts @@ -1,5 +1,5 @@ /** - * Copyright 2019-2022, 2024, Optimizely + * Copyright 2019-2022, 2024, 2026, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,13 +56,16 @@ export interface ProjectConfigManager extends Service { export const LOGGER_NAME = 'ProjectConfigManager'; +export const MAX_INVALID_DATAFILE_EMIT = 3; + export class ProjectConfigManagerImpl extends BaseService implements ProjectConfigManager { private datafile?: string; private projectConfig?: ProjectConfig; private optimizelyConfig?: OptimizelyConfig; - public jsonSchemaValidator?: Transformer; - public datafileManager?: DatafileManager; + private jsonSchemaValidator?: Transformer; + private datafileManager?: DatafileManager; private eventEmitter: EventEmitter<{ update: ProjectConfig }> = new EventEmitter(); + private datafileManagerInvalidEmit = 0; constructor(config: ProjectConfigManagerConfig) { super(); @@ -170,14 +173,18 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf } catch (err) { this.logger?.error(err); + if (!fromConfig) { + this.datafileManagerInvalidEmit++; + } + // if the state is starting and no datafileManager is provided, we cannot recover. // If the state is starting and the datafileManager has emitted a datafile, // that means a datafile was not provided in config or an invalid datafile was provided, // otherwise the state would have already been set to running synchronously. - // If the first datafile emitted by the datafileManager is invalid, + // If MAX_INVALID_DATAFILE_EMIT number of datafiles emitted by the datafileManager is invalid, // we consider this to be an initialization error as well. - const fatalError = (this.isStarting() && !this.datafileManager) || - (this.isStarting() && !fromConfig); + const fatalError = this.isStarting() + && (!this.datafileManager || this.datafileManagerInvalidEmit >= MAX_INVALID_DATAFILE_EMIT); if (fatalError) { this.handleInitError(new Error(GOT_INVALID_DATAFILE)); } From de974e5f2a5ba8cc2f74301cd8fd55611cdf608e Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 13 Nov 2026 23:30:06 +0600 Subject: [PATCH 2/2] Fix typo in test description for datafile manager Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- lib/project_config/project_config_manager.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/project_config/project_config_manager.spec.ts b/lib/project_config/project_config_manager.spec.ts index 409ce12eb..8fd998ab4 100644 --- a/lib/project_config/project_config_manager.spec.ts +++ b/lib/project_config/project_config_manager.spec.ts @@ -262,8 +262,8 @@ describe('ProjectConfigManagerImpl', () => { await expect(manager.onTerminated()).rejects.toThrow(); }); - it('should resolve onRunning and update config when datafile manger emits valid datafile before \ - emitting inbalid datafile MAX_INVALID_DATAFILE_EMIT times', async () => { + it('should resolve onRunning and update config when datafileManager emits valid datafile before \ + emitting invalid datafile MAX_INVALID_DATAFILE_EMIT times', async () => { const datafileManager = getMockDatafileManager({ onRunning: Promise.resolve() }); const manager = new ProjectConfigManagerImpl({ datafile: 'ab', datafileManager }); manager.start();