Repository navigation
feat: Add a shared file data engine - #2082
Merged
Merged
Conversation
Contributor
|
@launchdarkly/js-sdk-common size report |
Contributor
|
@launchdarkly/js-client-sdk size report |
Contributor
|
@launchdarkly/js-client-sdk-common size report |
joker23
approved these changes
Oct 8, 2026
Adds FileReloader, FileDirectoryWatcher, mergeDocuments, parseDocumentByExtension, and the FileDataPolicy interface under packages/shared/sdk-server/src/data_sources/filedata, with their tests. Nothing uses the engine yet. The file data source and the FDv2 file data initializer move onto it in the following change.
A direct watch on a configured file that reported an error was dropped and set up again, but no reload ran. The file can have changed while its watch was failing, and when the path is a link to a file in another directory the directory watch does not see that change. An error from a direct file watch is now handled like a change event: the watch is renewed and the callback runs, as a directory watch already does when it is replaced.
When a watch could not be set up for every directory, the setup returned before the catch-up that reloads after a watch gap, so a directory whose watch was created in that pass got no reload while another directory still could not be watched, and the next retry skipped it because it was watched by then. A file written into the directory before its watch was in place was never loaded. A path in a directory that never exists is a valid configuration, so the catch-up now runs for the directories watched in each pass, and the retry continues for the rest.
kinyoklion
force-pushed
the
rlamb/overrides-node-filedata-engine
branch
from
October 8, 2026 22:17
eaefbbf to
60a16b3
Compare
The bound on how long a stream of change notifications can postpone a reload was measured with the wall clock, which a clock adjustment can move. It now uses monotonicNow from the common package, which reads a monotonic clock where the runtime has one.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 07851e0. Configure here.
…quests A direct watch on a configured file follows the path to an inode when it is set up. When a directory event named the file, or the file's metadata changed through another entry such as a link swap, the existing direct watch was kept, so after a retarget it stayed on the old target and edits to the new one were invisible to it and to the directory watch alike. The direct watch is now set up again in both cases. A request to notify that arrived while a metadata read was in flight was only applied to the read that ran afterwards, which compared against the metadata the first read had already recorded and so found no change. The request now applies to the read in flight as well, and during the first observation, which has nothing to compare to, it counts as a change.
kinyoklion
added a commit
that referenced
this pull request
Oct 9, 2026
) ## Summary This change moves the existing file data source and the FDv2 file data initializer onto the shared file data engine added in #2082, so that they and the file-based override source that a later change adds share one reading, reloading, and merging implementation. How a source turns document data into flags stays with that source, in an explicit policy. The previous loader, `FileLoader`, is removed. ### The injected translation The engine takes a `FileDataPolicy`, one per source; #2082 describes its members. Each policy is defined in its source's file, so the differences are visible in one place per source. `fileDataSourcePolicy` in `FileDataSource.ts`: - parser chosen by file extension; a YAML file without a configured parser fails with the existing message - a `flagValues` entry becomes a flag that is on and serves the value by fallthrough; its version starts at 1 and increments only when the value changes between loads, using the flag the last successful load produced for the same key - a flag or segment entry is keyed by its own `key` property, as the source has always stored it, so two entries with the same `key` property are duplicates whatever their map keys - a key that appears more than once fails the load with `found duplicate key: "<key>"` - a configured file that does not exist fails the load `fileDataInitializerPolicy` in `fileDataInitilizerFDv2.ts`: - parser chosen by file extension - every `flagValues` entry gets version 1; the initializer runs once and has no previous load - a flag or segment entry is keyed by its map key, as the initializer has always merged it - the last definition of a duplicate key wins, across files and between `flags` and `flagValues` in one file - a configured file that does not exist fails the load The later change that adds the override source supplies `fileOverrideSourcePolicy`: extension-based parsing with validation of the document shape, a `flagValues` entry expanded to an off flag, the configured `fail` or `ignore` duplicate handling, and a missing file that contributes nothing. ### Robustness the existing file data source gains The observable data semantics of the file data source and the FDv2 initializer are unchanged: the same documents produce the same flags, versions, and duplicate handling; the same errors reach the error handler; and the same messages are logged where they still apply. The unmodified baseline tests pass, and new tests pin the extension-based detection, the no-parser error text, and the documents the sources accepted before. With automatic updates on, the file data source now behaves differently in these ways: - A filesystem error during a reload (for example a file removed while it is being read) cannot become an unhandled promise rejection. Every failure is reported through the error handler. - The directories that contain the files are watched instead of the files themselves, so a rename-based write, as editors, deploy tools, and mounted ConfigMaps perform, is detected, and so is a file deleted and created again. - A watched directory that is deleted does not leave a dead watch behind. After each event from a watch, and after each failed load, the watcher checks that the directory still exists. A directory that no longer exists has its watch closed at once, which also ends the stream of events that Windows delivers for a deleted directory, and the watch is set up again through the existing one-second retry once the directory exists. When it is, one reload runs so that changes made in the meantime are picked up. An `error` event from the Node `FSWatcher` is routed to the same replacement instead of surfacing as an uncaught exception. - Change notifications settle for 100 ms before a reload, coalescing the burst of events one edit produces. A stream of notifications that never settles cannot postpone a reload without end: a reload runs no later than one second after the first notification of the burst. The previous per-file timestamp check is replaced by a comparison of the loaded content, so an unchanged file still does not reinitialize the store. - A failed load, for example a file read while it was being written, is retried after one second. The last good data stays in effect and the internal version baseline is intact, so the next successful load continues the version sequence. - A file that is missing at start is a load failure reported through the error handler, as before, and the retry loads it when it appears. - A read failure is now logged at error level as `Error loading files: <error>`; before it reached the error handler only. Processing failures keep `Error processing files: <error>`. Only an automatic retry can be a repeat: a failure that repeats on retry is logged at debug level and not reported to the error handler again, while a failure after a change notification is always reported, even when it matches the previous one. Consecutive retry failures of the same kind on the same path count as repeats regardless of their message, so a file that is written slowly, and fails to parse at a different position on each retry, is reported once. - An exception while the merged result is translated into store data, for example a flag entry that is not an object, is reported through the error handler as `Error processing files: <error>` and retried, as the old `_processFileData` catch did. Nothing from that load is remembered as the last good result, and the reload promise never rejects, so a bad entry can neither raise an unhandled rejection at startup nor stop later reloads. Without automatic updates the source loads once, as before. The FDv2 initializer loads once, with no watching, debouncing, or retry. ### Tests The baseline test files pass with one implementation-detail change in `FileDataSource.test.ts`: the tests that registered or fired a mocked `Filesystem.watch` per file path now do so on the files' directory, because the directory is what is watched. New tests cover each robustness change on the file data source, the two legacy policies, and the format-detection pins. The Node package gains a test over a real directory that is deleted and created again, and tests for the `FSWatcher` error routing and the `getFileStats` failure mapping. [SDK-3251](https://launchdarkly.atlassian.net/browse/SDK-3251) [SDK-3251]: https://launchdarkly.atlassian.net/browse/SDK-3251?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **Moves the file data source and FDv2 file data initializer onto the shared `filedata` engine** (`FileReloader`, `FileDirectoryWatcher`), removes `FileLoader`, and keeps per-source semantics in explicit `FileDataPolicy` implementations (`fileDataSourcePolicy`, `fileDataInitializerPolicy`). > > For **`FileDataSource` with `autoUpdate`**, watching and reloading change materially: parent **directories** are watched (plus direct watches on configured paths for symlinks), changes are **debounced** and **retried** on failure while last-good data and flag versions stay intact, deleted directories get watches torn down and re-established, and failures route through the error handler without unhandled rejections. **`FileDataInitializerFDv2`** uses the same loader path for a single startup read (no watch/retry). **`autoUpdate` docs** in `FileDataSourceOptions` describe the new behavior. > > **Tests** add policy/format/robustness coverage in `sdk-server` and integration-style Node tests (real dirs, symlinks, ConfigMap-style link swaps); existing `FileDataSource.test.ts` expectations shift to directory-level watch callbacks. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8dda675. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is the first of the preliminary changes for the flag overrides feature described by the OVERRIDE specification. It adds a shared file data engine to the server SDK common package, so that the existing file data source, the FDv2 file data initializer, and the file-based override source that a later change adds share one reading, reloading, and merging implementation. How a source turns document data into flags stays with that source, in an explicit policy that the engine takes as input.
This change adds the engine and nothing else. No source uses it yet; the existing sources keep their current loader until #2049 moves them onto the engine. The split keeps each change reviewable on its own: this one is the engine and its tests, and #2049 is the two sources' policies and the behavior they gain.
The shared engine
packages/shared/sdk-server/src/data_sources/filedata/holds what every file-based source needs:FileReloader: runs one load at a time; waits for a burst of change notifications to settle before reloading, and reloads no later than one second after the first notification of a burst that does not settle; retries a failed load after a short delay; keeps the last good data by not applying a failed load and leaves its internal state intact; skips applying content identical to the last applied content; and reports every read, parse, and merge failure through anonFailurecallback as a typedLoadFailure(kindread,parse, ormerge, the original error, the path, and whether the failure repeats an earlier one). Its load never rejects, so no filesystem error can become an unhandled rejection.FileDirectoryWatcher: watches the directories that contain the files, so a file replaced by a rename, deleted and recreated, or created after start is detected. An event that names an entry which is not one of the configured files causes a reload only when the metadata of a configured file in that directory changed, read through any symbolic link, so a mounted ConfigMap or Secret update (a link swap that never names the file) is detected while a busy sibling file costs no reload; when the platform does not report which entry changed, every event in the directory counts. Each configured file is also watched directly, so a path that is a symbolic link to a file in another directory is followed; that watch is set up again after each event from it, after a directory event that names the file, and after a change to the file's metadata seen through another entry, so it survives the file being replaced and follows a link that is retargeted. A directory that cannot be watched is retried. A watched directory that is deleted has its watch closed and set up again once the directory exists; a watch that reports an error is replaced the same way.mergeDocuments: combines the documents of the configured files in order and lets the policy resolve duplicate keys.parseDocumentByExtension: the format is chosen by file extension only, which is the rule the file data sources already used.The injected policy
The engine takes a
FileDataPolicy, one per source, with five members:parseDocument(path, data),makeFlagWithValue(key, value, previous),entryKey(mapKey, entry)(the key under which aflagsorsegmentsentry is stored and checked for duplicates),resolveDuplicateKey(category, key)(returnskeepFirstorkeepLast, or throws to fail the load), andmissingFile(failorskip). The policies of the file data source and the FDv2 initializer are defined in #2049, each in its source's file, so the differences between the sources are visible in one place per source. The override source supplies its own policy in a later change.Tests
The engine's tests cover the parsers, the merge, the reloader, and the watcher, including a deleted and recreated directory, a flood of events for a deleted directory, a watch error, the maximum debounce delay, and the repeat key.
MockFilesystemandtestPolicyare test helpers that the tests of #2049 also use.The platform additions the engine relies on,
Filesystem.getFileStatsand watch error reporting (#2062) and the changed entry name in the watch callback (#2068), have merged.SDK-3251
Note
Overview
Adds a new shared file data engine under
packages/shared/sdk-server/src/data_sources/filedata/so file-based flag sources can share one read/reload/merge path. Nothing in the SDK wires it up yet; existing sources keep their loaders until a follow-up.FileReloaderloads configured paths in order, runs one reload at a time, debounces change triggers (with a max delay cap), retries failures, optionally skips unchanged content, and reports typedLoadFailures viaonFailurewhile keeping the last good state when a load fails.FileDirectoryWatcherwatches each file’s parent directory (deduped) plus a direct watch per path. It filters noisy directory events using file metadata (for ConfigMap-style link swaps), renews watches after errors/replaces, retries missing directories, and exposesverify()after failed loads.mergeDocumentsmerges parsed documents using an injectedFileDataPolicy(parse,flagValuesexpansion, duplicate keys, missing-file behavior).parseDocumentByExtensionpicks JSON vs YAML by extension.The PR adds broad unit tests (
MockFilesystem,testPolicy) for parsing, merge, reloader, and watcher edge cases (deleted dirs, watch floods, in-flight metadata checks, strict ENOENT on file watches).Reviewed by Cursor Bugbot for commit 1784d77. Bugbot is set up for automated code reviews on this repo. Configure here.