Skip to content

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

Merged
Raju Ahmed (raju-opti) merged 2 commits into
masterfrom
raju/invalid-datafile-retry
Nov 13, 2026
Merged

Raju Ahmed (raju-opti) merged 2 commits into
masterfrom
raju/invalid-datafile-retry

Conversation

@raju-opti

Copy link
Copy Markdown
Contributor

Summary

  • ProjectConfigManagerImpl no longer fails to start on the first invalid datafile from the datafileManager. Startup now fails only after MAX_INVALID_DATAFILE_EMIT (3) invalid datafiles arrive before a valid one. A valid datafile within that limit starts the manager as usual.
  • An invalid datafile with no datafileManager is still a fatal init error, as before.
  • jsonSchemaValidator and datafileManager are now private.

A single bad response, such as a truncated or corrupted payload, used to stop the SDK from starting even though the next poll would have returned a valid datafile.

Test plan

  • Updated project_config_manager.spec.ts, both with and without an initial (invalid) datafile in config:
    • onRunning() and onTerminated() reject after MAX_INVALID_DATAFILE_EMIT invalid datafiles.
    • onRunning() resolves and the config is updated when a valid datafile arrives after MAX_INVALID_DATAFILE_EMIT - 1 invalid ones.
  • The "invalid datafile in config" tests now use 'ab' instead of JSON.stringify({}).
  • npx vitest run lib/project_config/project_config_manager.spec.ts: 47 passed. tsc --noEmit is clean.

Issues

  • FSSDK-13432

…ager 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.
@coveralls

Coveralls (coveralls) commented Nov 13, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 77.492% (-0.02%) from 77.513% — raju/invalid-datafile-retry into master

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical startup hangs remain: polling can stop after an invalid response or repeatedly receive 304 responses without advancing the validation retry limit.

2 open issues
What changed in this PR

This PR makes SDK startup tolerate transient invalid datafiles from the datafile manager instead of failing on the first invalid emission.

Changes:

  • Fail startup after three invalid manager emissions, while allowing a valid datafile before that threshold to complete startup.
  • Make the schema validator and datafile manager fields private.
  • Update tests for retry-limit failure and recovery, with and without an invalid initial datafile.
File Description
lib/​project_config/​project_config_manager.ts Adds the invalid-emission threshold and makes internal fields private.
lib/​project_config/​project_config_manager.spec.ts Tests threshold failure and recovery; updates invalid-datafile fixtures.

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

An active review nit remains for naming and spelling typos in lib/project_config/project_config_manager.spec.ts.

1 open issue
2 resolved since last review

Review effort: Balanced

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@raju-opti
Raju Ahmed (raju-opti) merged commit 77957f4 into master Nov 13, 2026
20 of 27 checks passed
@raju-opti
Raju Ahmed (raju-opti) deleted the raju/invalid-datafile-retry branch November 13, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants