Skip to content

[FSSDK-13432] Don't treat the first invalid datafile from datafileManager as fatal #1188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Raju Ahmed (raju-opti) merged 2 commits into from
Nov 13, 2026
Merged

[FSSDK-13432] Don't treat the first invalid datafile from datafileManager as fatal #1188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22eb773
[FSSDK-13432] Don't treat the first invalid datafile from datafileMan…
raju-opti Nov 13, 2026
de974e5
Fix typo in test description for datafile manager
raju-opti Nov 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 67 additions & 9 deletions lib/project_config/project_config_manager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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 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();

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())));
});
});
});

Expand All @@ -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();
Expand Down
19 changes: 13 additions & 6 deletions lib/project_config/project_config_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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<unknown, boolean>;
public datafileManager?: DatafileManager;
private jsonSchemaValidator?: Transformer<unknown, boolean>;
private datafileManager?: DatafileManager;
private eventEmitter: EventEmitter<{ update: ProjectConfig }> = new EventEmitter();
private datafileManagerInvalidEmit = 0;

constructor(config: ProjectConfigManagerConfig) {
super();
Expand Down Expand Up @@ -170,14 +173,18 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf
} catch (err) {
this.logger?.error(err);

if (!fromConfig) {
this.datafileManagerInvalidEmit++;
Comment thread
raju-opti marked this conversation as resolved.
Resolved
}

// 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);
Comment thread
raju-opti marked this conversation as resolved.
Resolved
if (fatalError) {
this.handleInitError(new Error(GOT_INVALID_DATAFILE));
}
Expand Down
Loading