diff --git a/packages/sdk/server-node/__tests__/LDClientNode.fileDataSource.test.ts b/packages/sdk/server-node/__tests__/LDClientNode.fileDataSource.test.ts index f99d40f71c..913eec2a60 100644 --- a/packages/sdk/server-node/__tests__/LDClientNode.fileDataSource.test.ts +++ b/packages/sdk/server-node/__tests__/LDClientNode.fileDataSource.test.ts @@ -1,8 +1,13 @@ -import { mkdir, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { parse as parseYaml } from 'yaml'; -import { integrations } from '@launchdarkly/js-server-sdk-common'; +import { integrations, sleep } from '@launchdarkly/js-server-sdk-common'; import LDClientNode from '../src/LDClientNode'; +import { rewriteInPlace, sampleValues } from './inPlaceWrites'; +import waitFor from './waitFor'; const flag1Key = 'flag1'; const flag2Key = 'flag2'; @@ -178,3 +183,98 @@ describe('When using a file data source', () => { client.close(); }); }); + +describe('given a file data source that reloads a file rewritten in place', () => { + let directory: string; + + beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'ld-file-data-')); + }); + + afterEach(async () => { + await rm(directory, { recursive: true, force: true }); + }); + + const flagsDocument = (value: string) => JSON.stringify({ flagValues: { flag1: value } }); + + // Starts a client whose data file serves 'a', runs the rewrites while it samples what the client + // serves, and returns what it served until it served the final value, and the errors it logged. + const sampleRewrites = async ( + extension: string, + rewrites: (path: string) => Promise, + finalValue: string, + ) => { + const path = join(directory, `flags.${extension}`); + await writeFile(path, flagsDocument('a')); // JSON is also YAML + const errors: string[] = []; + const fds = new integrations.FileDataSourceFactory({ + paths: [path], + autoUpdate: true, + yamlParser: parseYaml, + }); + const client = new LDClientNode('sdk-key', { + updateProcessor: fds.getFactory(), + sendEvents: false, + diagnosticOptOut: true, + logger: { + error: (message: string) => errors.push(message), + warn: () => {}, + info: () => {}, + debug: () => {}, + }, + }); + try { + await client.waitForInitialization({ timeout: 10 }); + expect(await client.variation('flag1', { key: 'user1' }, 'default')).toEqual('a'); + + const sampler = sampleValues(() => client.variation('flag1', { key: 'user1' }, 'default')); + try { + await rewrites(path); + // Waits for the sampler itself to see the final value, so it has recorded everything that + // the client served before it. + await waitFor(() => sampler.served.includes(finalValue), 10000); + } finally { + await sampler.stop(); + } + return { served: sampler.served, errors }; + } finally { + client.close(); + } + }; + + it.each(['json', 'yaml'])('never serves the emptied %s file', async (extension) => { + // The file is empty for 300 ms, longer than the settle window after a change, so a reload + // reads it empty. The client must keep serving the old value until the new one is written. + const { served, errors } = await sampleRewrites( + extension, + (path) => rewriteInPlace(path, flagsDocument('b'), 300), + 'b', + ); + + expect(served).toEqual(['a', 'b']); + // The file was read while it was empty, which is not an error to report. + expect(errors).toEqual([]); + }); + + it.each(['json', 'yaml'])( + 'never serves the %s file emptied by two in-place rewrites in a row', + async (extension) => { + // The first rewrite's write and the second's truncation fall in one settle window, so the + // load after them finds the file empty again. + const { served, errors } = await sampleRewrites( + extension, + async (path) => { + await rewriteInPlace(path, flagsDocument('b'), 300); + await sleep(20); + await rewriteInPlace(path, flagsDocument('c'), 300); + }, + 'c', + ); + + expect(served).not.toContain('default'); + expect(served[0]).toEqual('a'); + expect(served[served.length - 1]).toEqual('c'); + expect(errors).toEqual([]); + }, + ); +}); diff --git a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts index 4d2dc3989a..da7909a394 100644 --- a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts +++ b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts @@ -1,25 +1,30 @@ +import { promises as fsPromises } from 'node:fs'; import { mkdtemp, rm, unlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { parse as parseYaml } from 'yaml'; -import { FileOverrideSourceOptions, sleep } from '@launchdarkly/js-server-sdk-common'; +import { FileOverrideSourceOptions, LDLogger, sleep } from '@launchdarkly/js-server-sdk-common'; import LDClientNode from '../src/LDClientNode'; +import { rewriteInPlace, sampleValues } from './inPlaceWrites'; import waitFor from './waitFor'; const user = { key: 'user-key' }; +const silentLogger: LDLogger = { error: () => {}, warn: () => {}, info: () => {}, debug: () => {} }; + // The client never obtains data from LaunchDarkly: its only initializer reads a file that does // not exist. Overrides are served from the override layer alone. function makeClient( directory: string, overrides: Omit, + logger: LDLogger = silentLogger, ): LDClientNode { return new LDClientNode('sdk-key-file-overrides', { sendEvents: false, diagnosticOptOut: true, - logger: { error: () => {}, warn: () => {}, info: () => {}, debug: () => {} }, + logger, dataSystem: { dataSource: { dataSourceOptionsType: 'custom', @@ -133,4 +138,137 @@ describe('given a temporary directory of override files', () => { 10000, ); }); + + describe.each([ + ['json', 'watching'], + ['json', 'polling'], + ['yaml', 'watching'], + ['yaml', 'polling'], + ] as const)('given a %s file in %s mode', (extension, changeDetection) => { + let polls: number[]; + + beforeEach(() => { + polls = []; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + // Records when the poller examines the file, so that a test can start a rewrite just before a + // poll. + const recordPolls = (path: string) => { + const realStat = fsPromises.stat; + jest.spyOn(fsPromises, 'stat').mockImplementation(((statPath: any, ...rest: any[]) => { + if (statPath === path) { + polls.push(Date.now()); + } + return (realStat as any).call(fsPromises, statPath, ...rest); + }) as any); + }; + + let path: string; + let errors: string[]; + + // Starts a client whose override file serves 'a'. + const start = async () => { + path = join(directory, `overrides.${extension}`); + await writeFile(path, document('a')); // JSON is also YAML + errors = []; + recordPolls(path); + client = makeClient( + directory, + { paths: [path], changeDetection, pollInterval: 1, yamlParser: parseYaml }, + { ...silentLogger, error: (message: string) => errors.push(message) }, + ); + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'a', + 10000, + ); + }; + + // Waits for the next poll and returns when it happened. + const nextPoll = async () => { + const count = polls.length; + await waitFor(() => polls.length > count, 5000, 1); + return polls[polls.length - 1]; + }; + + // Samples what the client serves while the rewrites run, until it serves the final value. + const sampleWhile = async (rewrite: () => Promise, finalValue: string) => { + const sampler = sampleValues(() => client!.variation('overridden-flag', user, 'default')); + try { + await rewrite(); + // Waits for the sampler itself to see the final value, so it has recorded everything that + // the client served before it. + await waitFor(() => sampler.served.includes(finalValue), 10000); + } finally { + await sampler.stop(); + } + return sampler.served; + }; + + const loadFailures = () => + errors.filter((message) => message.startsWith('Unable to load flags')); + + // A polling test waits for a poll and then for the rewrites, several seconds in all. + const testTimeoutMs = 15000; + + it( + 'keeps the override while the file is rewritten in place', + async () => { + await start(); + let pauseMs = 300; + if (changeDetection === 'polling') { + // The rewrite starts 750 ms after a poll, so the next poll finds the file empty, and its + // reload 100 ms later still does. The file is written 250 ms after that, well before the + // retry reads it again a second after the reload. + await sleep((await nextPoll()) + 750 - Date.now()); + pauseMs = 600; + } + // In watching mode, the reload that the truncation starts runs after the 100 ms settle + // window, while the file is still empty. + const served = await sampleWhile(() => rewriteInPlace(path, document('b'), pauseMs), 'b'); + + expect(served).toEqual(['a', 'b']); + expect(loadFailures()).toEqual([]); + }, + testTimeoutMs, + ); + + it( + 'keeps the override through two in-place rewrites in a row', + async () => { + await start(); + let rewrites: () => Promise; + if (changeDetection === 'polling') { + // The first rewrite starts 750 ms after a poll, so the next poll finds the file empty. + // The second starts 1900 ms after that poll, before the next poll and the retry, so they + // find the file empty again before any load has read the first rewrite's content. + const poll = await nextPoll(); + await sleep(poll + 750 - Date.now()); + rewrites = async () => { + await rewriteInPlace(path, document('b'), 600); + await sleep(poll + 1900 - Date.now()); + await rewriteInPlace(path, document('c'), 600); + }; + } else { + // The first rewrite's write and the second's truncation fall in one settle window, so the + // load after them finds the file empty again. + rewrites = async () => { + await rewriteInPlace(path, document('b'), 300); + await sleep(20); + await rewriteInPlace(path, document('c'), 300); + }; + } + const served = await sampleWhile(rewrites, 'c'); + + expect(served).not.toContain('default'); + expect(served[0]).toEqual('a'); + expect(served[served.length - 1]).toEqual('c'); + expect(loadFailures()).toEqual([]); + }, + testTimeoutMs, + ); + }); }); diff --git a/packages/sdk/server-node/__tests__/inPlaceWrites.ts b/packages/sdk/server-node/__tests__/inPlaceWrites.ts new file mode 100644 index 0000000000..3db1b697b1 --- /dev/null +++ b/packages/sdk/server-node/__tests__/inPlaceWrites.ts @@ -0,0 +1,53 @@ +import { open } from 'node:fs/promises'; + +import { sleep } from '@launchdarkly/js-server-sdk-common'; + +/** + * Rewrites a file in place the slow way: it truncates the file, waits, and then writes the new + * content, so a reader can find the file empty in between. Applications and editors rewrite data + * files in place, and the SDK must tolerate reading one while it is being written. + */ +export async function rewriteInPlace( + path: string, + content: string, + pauseAfterTruncatingMs: number, +): Promise { + const handle = await open(path, 'r+'); + try { + await handle.truncate(0); + await sleep(pauseAfterTruncatingMs); + await handle.write(content, 0); + } finally { + await handle.close(); + } +} + +/** + * Evaluates over and over, and records each value that differs from the one before, until + * stopped. + */ +export function sampleValues(evaluate: () => Promise): { + served: unknown[]; + stop: () => Promise; +} { + const served: unknown[] = []; + let stopped = false; + const sampling = (async () => { + while (!stopped) { + // eslint-disable-next-line no-await-in-loop + const value = await evaluate(); + if (served.length === 0 || served[served.length - 1] !== value) { + served.push(value); + } + // eslint-disable-next-line no-await-in-loop + await sleep(2); + } + })(); + return { + served, + stop: async () => { + stopped = true; + await sampling; + }, + }; +} diff --git a/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.inPlaceRewrite.test.ts b/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.inPlaceRewrite.test.ts new file mode 100644 index 0000000000..863fe54546 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.inPlaceRewrite.test.ts @@ -0,0 +1,1226 @@ +import { + FileReloader, + FileReloaderConfig, + hasContent, + LoadFailure, + ReloadResult, +} from '../../../src/data_sources/filedata'; +import { parseOverrideDocument } from '../../../src/overrides/overrideDocument'; +import TestLogger, { LogLevel } from '../../Logger'; +import MockFilesystem from './MockFilesystem'; +import testPolicy from './testPolicy'; + +// These tests rewrite a file the way an application or editor rewrites it in place: the file is +// truncated, and the new content is written some time later. A reload in between finds the file +// empty. The clock is Jest's fake clock, so every time below is exact. + +const jsonPath = '/data/data.json'; +const yamlPath = '/data/data.yaml'; +const otherYamlPath = '/data/other.yaml'; +const retryDelayMs = 1000; +// The retry that a hold arms comes a tenth of the retry delay after the delay ends. +const holdRetryMs = retryDelayMs + retryDelayMs / 10; + +const documentA = '{"flagValues": {"flag1": "a"}}'; +const documentB = '{"flagValues": {"flag1": "b"}}'; +const documentC = '{"flagValues": {"flag1": "c"}}'; +const otherA = '{"flagValues": {"flag2": "a"}}'; +const otherB = '{"flagValues": {"flag2": "b"}}'; + +// A stand-in for a YAML parser. A flow-style document like the ones above is both JSON and YAML. +// Like the YAML parsers, it returns null for content with no document: white space, comments and +// document markers. +const yamlParser = (data: string) => { + const body = data + .split('\n') + .filter((line) => !/^\s*(#.*)?$/.test(line) && line.trim() !== '---') + .join('\n'); + return hasContent(body) ? JSON.parse(body) : null; +}; + +const emptiedReadAgain = /is empty, possibly while it is being rewritten; reading it again/; +const emptyJson = 'the file has no content, which is not a JSON document'; + +describe('given a reloader over a file that is rewritten in place', () => { + let filesystem: MockFilesystem; + let logger: TestLogger; + let applied: ReloadResult[]; + let failures: LoadFailure[]; + let reloader: FileReloader; + let modified: number; + + beforeEach(() => { + jest.useFakeTimers(); + filesystem = new MockFilesystem(); + logger = new TestLogger(); + applied = []; + failures = []; + modified = 0; + }); + + afterEach(() => { + reloader?.close(); + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + // Every write, including a truncation, changes the file's modification time, as it does on a + // real filesystem. + const write = (path: string, data: string) => { + modified += 1; + filesystem.set(path, data, modified); + }; + + // The policy parses by extension and validates the document as the override source does, so a + // YAML file with no document has no entries and a JSON file with no content fails. + const makeReloader = (paths: string[], configure?: (config: FileReloaderConfig) => void) => { + const config: FileReloaderConfig = { + paths, + policy: testPolicy({ parseDocument: parseOverrideDocument(yamlParser) }), + filesystem, + logger, + apply: (result) => applied.push(result), + onFailure: (failure) => failures.push(failure), + debounceDelayMs: 0, + retryDelayMs, + skipUnchanged: true, + }; + configure?.(config); + reloader = new FileReloader(config); + return reloader; + }; + + const readAgainMessages = () => + logger.getMessages(LogLevel.Debug).filter((message) => emptiedReadAgain.test(message)); + + const values = () => + applied.map((result) => result.flags.map(({ item }) => item.variations?.[0]).join(',')); + + // Two triggers without a settle in between coalesce into one load, so every change settles. + const changeAndSettle = async () => { + reloader.trigger(); + await jest.advanceTimersByTimeAsync(0); + }; + + const advance = (ms: number) => jest.advanceTimersByTimeAsync(ms); + + const startWith = async (path: string, data: string) => { + write(path, data); + makeReloader([path]); + await reloader.reloadNow(); + }; + + it.each(['', ' \n\t\r\n', '\uFEFF'])( + 'holds back an emptied file and applies the rewritten content with no empty application, for content %j', + async (empty) => { + await startWith(yamlPath, documentA); + + write(yamlPath, empty); + await changeAndSettle(); + expect(values()).toEqual(['a']); + expect(failures).toEqual([]); + logger.expectMessages([{ level: LogLevel.Debug, matches: emptiedReadAgain }]); + + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + expect(failures).toEqual([]); + }, + ); + + it.each(['# only a comment\n', '---\n', '---\n# a comment\n'])( + 'holds back a YAML file whose content is no document, %j', + async (content) => { + await startWith(yamlPath, documentA); + + write(yamlPath, content); + await changeAndSettle(); + expect(values()).toEqual(['a']); + expect(readAgainMessages()).toHaveLength(1); + + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + }, + ); + + it('applies a file that holds an empty document right away', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, '{}'); + await changeAndSettle(); + expect(values()).toEqual(['a', '']); + expect(readAgainMessages()).toEqual([]); + }); + + it('applies a YAML file that stays empty when the retry comes after the delay', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + + await advance(holdRetryMs - 1); + expect(values()).toEqual(['a']); + await advance(1); + expect(values()).toEqual(['a', '']); + expect(failures).toEqual([]); + }); + + it('holds back a file until exactly the retry delay has passed', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(retryDelayMs * 0.95); + await changeAndSettle(); + expect(values()).toEqual(['a']); + + await advance(retryDelayMs * 0.05); + await changeAndSettle(); + expect(values()).toEqual(['a', '']); + }); + + it('does not report an emptied JSON file at the load that finds it empty, and the retry reports it', async () => { + await startWith(jsonPath, documentA); + + write(jsonPath, ''); + await changeAndSettle(); + expect(failures).toEqual([]); + + await advance(holdRetryMs); + expect(failures).toHaveLength(1); + expect(failures[0]).toMatchObject({ kind: 'parse', path: jsonPath, repeated: false }); + expect(failures[0].error.message).toEqual(emptyJson); + expect(values()).toEqual(['a']); + }); + + it('does not apply a file that stays empty at a further change signal, and the retry applies it when the delay ends', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(500); + await changeAndSettle(); + expect(values()).toEqual(['a']); + + // The retry that the further load armed comes when the delay that started at the first load + // ends, plus a tenth of the delay. + await advance(holdRetryMs - 500 - 1); + expect(values()).toEqual(['a']); + await advance(1); + expect(values()).toEqual(['a', '']); + }); + + it('restarts the delay when the file is written and truncated again between two loads', async () => { + // Two quick rewrites in watching mode: the write of the first and the truncation of the second + // fall in one settle window, so no load sees the first one's content. + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + await changeAndSettle(); + + // A load after the first delay would have ended still holds the file back. + await advance(450); + await changeAndSettle(); + expect(values()).toEqual(['a']); + + write(yamlPath, documentC); + await changeAndSettle(); + expect(values()).toEqual(['a', 'c']); + expect(failures).toEqual([]); + }); + + it('does not apply a file that is mid-rewrite when another file changes', async () => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(300); + write(otherYamlPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + + await advance(200); + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a,a', 'b,b']); + expect(failures).toEqual([]); + }); + + it('does not apply the truncation of a later rewrite that the retry finds', async () => { + // Polling with two rewrites about a second apart: no load reads the first one's content + // before the second truncation. + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(400); + write(yamlPath, documentB); + await advance(500); + write(yamlPath, ''); + await advance(holdRetryMs - 900); + expect(values()).toEqual(['a']); + expect(readAgainMessages()).toHaveLength(2); + + write(yamlPath, documentC); + await changeAndSettle(); + expect(values()).toEqual(['a', 'c']); + }); + + it('applies a file that keeps being rewritten so that every load finds it empty, once loads have held it back for five retry delays', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + // Every 600 ms the file is written and truncated again before a load reads it. + await Array.from({ length: 8 }).reduce>( + (previous) => + previous.then(async () => { + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + await changeAndSettle(); + }), + Promise.resolve(), + ); + expect(values()).toEqual(['a']); + + // The last load, at 4800 ms, armed the retry for when the five retry delays end, plus a + // tenth of the delay. + await advance(299); + expect(values()).toEqual(['a']); + await advance(1); + expect(values()).toEqual(['a', '']); + }); + + it('applies files that are rewritten in turn, once loads have held them back for five retry delays', async () => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + // Every 600 ms the file that is empty is written and the other one is truncated, so every load + // finds one of them empty, and each file's own delay starts over. + await [1, 2, 3, 4, 5, 6, 7, 8].reduce>( + (previous, step) => + previous.then(async () => { + await advance(600); + if (step % 2 === 1) { + write(yamlPath, documentB); + write(otherYamlPath, ''); + } else { + write(otherYamlPath, otherB); + write(yamlPath, ''); + } + await changeAndSettle(); + }), + Promise.resolve(), + ); + // Changes to the file that has content wait while the other one is held back. + expect(values()).toEqual(['a,a']); + + await advance(299); + expect(values()).toEqual(['a,a']); + await advance(1); + expect(values()).toEqual(['a,a', 'b']); + }); + + it('starts a fresh hold for a file that is emptied again after its content was read', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(300); + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + + // The load that applied the content ended the run of holding loads, so a load more than five + // retry delays after the first hold still holds the file back. + await advance(4600); + write(yamlPath, ''); + await changeAndSettle(); + await advance(200); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + + await advance(holdRetryMs - 200 - 1); + expect(values()).toEqual(['a', 'b']); + await advance(1); + expect(values()).toEqual(['a', 'b', '']); + }); + + it('starts a fresh hold for a JSON file that has been empty for long and is rewritten in place', async () => { + await startWith(jsonPath, documentA); + + write(jsonPath, ''); + await changeAndSettle(); + await advance(holdRetryMs); + expect(failures).toHaveLength(1); + expect(failures[0].error.message).toEqual(emptyJson); + + // The file stays empty, and the retries keep failing. A failing load ends the run of holding + // loads. + await advance(10000); + const failed = failures.length; + + // The file is rewritten in place. Its truncation is a new state, held back without a report. + write(jsonPath, ''); + await changeAndSettle(); + expect(failures).toHaveLength(failed); + expect(readAgainMessages()).toHaveLength(2); + + write(jsonPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + expect(failures).toHaveLength(failed); + }); + + it('starts a fresh delay for a file that is deleted and created empty again', async () => { + write(jsonPath, documentA); + makeReloader([jsonPath], (config) => { + config.policy = testPolicy({ missingFile: 'fail' }); + }); + await reloader.reloadNow(); + + write(jsonPath, ''); + await changeAndSettle(); + await advance(100); + filesystem.remove(jsonPath); + await changeAndSettle(); + // The retries keep failing to read the missing file, as repeats. + const reported = () => failures.filter((failure) => !failure.repeated); + expect(reported().map((failure) => failure.kind)).toEqual(['read']); + + // The data in effect still has the file's content, so the empty file is held back, with a + // delay that starts when it is found empty again. + await advance(1900); + write(jsonPath, ''); + await changeAndSettle(); + await advance(500); + await changeAndSettle(); + expect(reported().map((failure) => failure.kind)).toEqual(['read']); + + await advance(holdRetryMs - 500 - 1); + expect(reported().map((failure) => failure.kind)).toEqual(['read']); + await advance(1); + expect(reported().map((failure) => failure.kind)).toEqual(['read', 'parse']); + expect(reported()[1].error.message).toEqual(emptyJson); + }); + + describe('on a filesystem whose modification times are coarse', () => { + // Every write keeps the same modification time, as two writes within one tick of a coarse + // clock do, so a file that is empty again looks unchanged since it was last found empty. + const writeCoarse = (path: string, data: string) => filesystem.set(path, data, 1); + + it('starts a fresh delay for a file emptied again after a failed load read its content', async () => { + writeCoarse(yamlPath, documentA); + writeCoarse(jsonPath, otherA); + makeReloader([yamlPath, jsonPath]); + await reloader.reloadNow(); + + writeCoarse(yamlPath, ''); + await changeAndSettle(); + // The file gets its content, but another file is malformed, so nothing is applied. + await advance(300); + writeCoarse(yamlPath, documentB); + writeCoarse(jsonPath, '{"flagValues": '); + await changeAndSettle(); + expect(failures).toHaveLength(1); + + // The file is emptied again, more than a retry delay after it was first found empty. + await advance(1200); + writeCoarse(yamlPath, ''); + await changeAndSettle(); + await advance(100); + writeCoarse(jsonPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + + writeCoarse(yamlPath, documentC); + await changeAndSettle(); + expect(values()).toEqual(['a,a', 'c,b']); + }); + + it('starts a fresh delay for a file that is emptied again after its content was read', async () => { + writeCoarse(yamlPath, documentA); + makeReloader([yamlPath]); + await reloader.reloadNow(); + + writeCoarse(yamlPath, ''); + await changeAndSettle(); + await advance(300); + writeCoarse(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + + await advance(2000); + writeCoarse(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + expect(readAgainMessages()).toHaveLength(2); + }); + + it('starts a fresh delay for a file emptied again after content that fails to parse', async () => { + writeCoarse(yamlPath, documentA); + makeReloader([yamlPath]); + await reloader.reloadNow(); + + writeCoarse(yamlPath, ''); + await changeAndSettle(); + writeCoarse(yamlPath, '{"flagValues": '); + await changeAndSettle(); + expect(failures).toHaveLength(1); + + // More than a retry delay after the first emptying, the file is emptied again, and looks as + // it did then. + await advance(1500); + writeCoarse(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a']); + expect(readAgainMessages()).toHaveLength(2); + }); + + it('starts a fresh delay for a file that is deleted and created empty again', async () => { + writeCoarse(jsonPath, documentA); + makeReloader([jsonPath], (config) => { + config.policy = testPolicy({ missingFile: 'fail' }); + }); + await reloader.reloadNow(); + + writeCoarse(jsonPath, ''); + await changeAndSettle(); + await advance(100); + filesystem.remove(jsonPath); + await changeAndSettle(); + const reported = () => failures.filter((failure) => !failure.repeated); + expect(reported().map((failure) => failure.kind)).toEqual(['read']); + + await advance(2000); + writeCoarse(jsonPath, ''); + await changeAndSettle(); + expect(reported().map((failure) => failure.kind)).toEqual(['read']); + expect(readAgainMessages()).toHaveLength(2); + }); + }); + + it('does not restart the bound of a file that is held back when another file fails', async () => { + write(yamlPath, documentA); + write(jsonPath, otherA); + makeReloader([yamlPath, jsonPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + // Every 600 ms the first file is written and truncated again before a load reads it, and the + // other file is malformed at every other step, which fails those loads. + await [1, 2, 3, 4, 5, 6, 7, 8].reduce>( + (previous, step) => + previous.then(async () => { + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + write(jsonPath, step % 2 === 0 ? '{"flagValues": ' : otherA); + await changeAndSettle(); + }), + Promise.resolve(), + ); + expect(values()).toEqual(['a,a']); + expect(failures.length).toBeGreaterThan(0); + + // 5400 ms after it was first held back, the file is applied although it was just written. + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + write(jsonPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a', 'b']); + }); + + it("does not cut another file's rewrite short when one file's bound is over", async () => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + // The first file keeps being written and truncated again before every load. + write(yamlPath, ''); + await changeAndSettle(); + await Array.from({ length: 7 }).reduce>( + (previous) => + previous.then(async () => { + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + await changeAndSettle(); + }), + Promise.resolve(), + ); + // At 4750 ms, a quarter of a retry delay before the first file's bound is over, the second + // file starts an ordinary in-place rewrite. + await advance(550); + write(otherYamlPath, ''); + await changeAndSettle(); + await advance(50); + write(yamlPath, documentB); + write(yamlPath, ''); + await changeAndSettle(); + + // At 5100 ms the retry finds the first file's bound over, and the second file still in its + // delay, so nothing is applied. + await advance(300); + expect(values()).toEqual(['a,a']); + + write(otherYamlPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a', 'b']); + }); + + it('applies files that are emptied in staggered turns once their bounds are over', async () => { + // Two files stay empty and are truncated again in turn, so that whenever one of them has + // stayed empty for the whole delay, the other is still within its delay and holds the load + // back. A file that settles so does not get a fresh bound while the other is held back. + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + expect(readAgainMessages()).toHaveLength(1); + // Every 900 ms one file is truncated again, with a load then and one 200 ms later, standing + // for the retry. The second file is emptied at the first step. + await Array.from({ length: 14 }, (_, index) => index + 1).reduce>( + (previous, step) => + previous.then(async () => { + if (applied.length > 1) { + return; + } + await advance(step === 1 ? 900 : 700); + write(step % 2 === 1 ? otherYamlPath : yamlPath, ''); + await changeAndSettle(); + await advance(200); + await changeAndSettle(); + }), + Promise.resolve(), + ); + expect(applied.length).toBeGreaterThan(1); + }); + + it('starts the bound of a file held back during a failure over once a failed load reads it', async () => { + // While the other file fails to load, the first file is rewritten once and its gap is held + // back. Much later, the other file is fixed while the first file is in the middle of another + // rewrite, which is held back with a bound of its own. + write(yamlPath, documentA); + write(jsonPath, otherA); + makeReloader([yamlPath, jsonPath]); + await reloader.reloadNow(); + + write(jsonPath, '{"flagValues": '); + write(yamlPath, ''); + await changeAndSettle(); + write(yamlPath, documentB); + await changeAndSettle(); + await advance(6 * retryDelayMs); + // A reload that fails reports the failure, not the files it holds back. + expect(readAgainMessages()).toEqual([]); + + write(jsonPath, otherB); + write(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + expect(readAgainMessages()).toHaveLength(1); + }); + + it.each([yamlPath, jsonPath])( + 'holds back the in-place save that fixes %s after it was saved in place with an error', + async (path) => { + await startWith(path, documentA); + + write(path, ''); + await changeAndSettle(); + expect(readAgainMessages()).toHaveLength(1); + write(path, '{"flagValues": '); + await changeAndSettle(); + expect(failures).toHaveLength(1); + // The broken file keeps failing for longer than the bound. + await advance(6 * retryDelayMs); + const failed = failures.length; + + // The fix is saved in place. Its truncation is held back, and no failure is reported. + write(path, ''); + await changeAndSettle(); + expect(failures).toHaveLength(failed); + expect(readAgainMessages()).toHaveLength(2); + write(path, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + }, + ); + + it('holds back a file created empty again after it was missing for longer than the bound', async () => { + write(jsonPath, documentA); + makeReloader([jsonPath], (config) => { + config.policy = testPolicy({ missingFile: 'fail' }); + }); + await reloader.reloadNow(); + + write(jsonPath, ''); + await changeAndSettle(); + filesystem.remove(jsonPath); + await changeAndSettle(); + await advance(6 * retryDelayMs); + const failed = failures.length; + + // The file is created empty, and then written. + write(jsonPath, ''); + await changeAndSettle(); + expect(failures).toHaveLength(failed); + expect(readAgainMessages()).toHaveLength(2); + write(jsonPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + }); + + it('applies a file kept written while empty after its bound while an earlier file fails', async () => { + write(jsonPath, otherA); + write(yamlPath, documentA); + makeReloader([jsonPath, yamlPath]); + await reloader.reloadNow(); + + // The second file keeps being written while empty. The first file fails at every other load. + write(yamlPath, ''); + await changeAndSettle(); + await [1, 2, 3, 4, 5, 6, 7, 8].reduce>( + (previous, step) => + previous.then(async () => { + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + write(jsonPath, step % 2 === 0 ? '{"flagValues": ' : otherA); + await changeAndSettle(); + }), + Promise.resolve(), + ); + expect(values()).toEqual(['a,a']); + + // 5400 ms after it was first held back, the second file is applied although it was just + // written. + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + write(jsonPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a', 'b']); + }); + + it('starts the bound of a file after a failing file over once a failing load reads it', async () => { + // The first file breaks while the second is written with content. Much later, the first file + // is fixed while the second is in the middle of a rewrite, which is held back with a bound of + // its own. + write(jsonPath, otherA); + write(yamlPath, documentA); + makeReloader([jsonPath, yamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + expect(readAgainMessages()).toHaveLength(1); + write(jsonPath, '{"flagValues": '); + write(yamlPath, documentB); + await changeAndSettle(); + expect(failures).toHaveLength(1); + await advance(6 * retryDelayMs); + + write(jsonPath, otherB); + write(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + expect(readAgainMessages()).toHaveLength(2); + }); + + it('does not start bounds over at a load that holds a file back and then fails', async () => { + const thirdPath = '/data/third.json'; + write(yamlPath, documentA); + write(otherYamlPath, otherA); + write(thirdPath, '{"flagValues": {"flag3": "a"}}'); + makeReloader([yamlPath, otherYamlPath, thirdPath]); + await reloader.reloadNow(); + + // The first two files are rewritten in turn, so that every load holds one of them back, and + // the third file fails at every other load, after that. + write(yamlPath, ''); + await changeAndSettle(); + await Array.from({ length: 12 }, (_, index) => index + 1).reduce>( + (previous, step) => + previous.then(async () => { + if (applied.length > 1) { + return; + } + await advance(500); + if (step % 2 === 1) { + write(yamlPath, documentB); + write(otherYamlPath, ''); + write(thirdPath, '{"flagValues": '); + } else { + write(otherYamlPath, otherB); + write(yamlPath, ''); + write(thirdPath, '{"flagValues": {"flag3": "b"}}'); + } + await changeAndSettle(); + }), + Promise.resolve(), + ); + expect(applied.length).toBeGreaterThan(1); + }); + + it('starts a new delay for a missing file when it is created empty again with the same state', async () => { + write(yamlPath, documentA); + write(jsonPath, otherA); + makeReloader([yamlPath, jsonPath]); + await reloader.reloadNow(); + + write(jsonPath, '{"flagValues": '); + write(yamlPath, ''); + const firstEmptyModified = modified; + await changeAndSettle(); + filesystem.remove(yamlPath); + await changeAndSettle(); + expect(failures.length).toBeGreaterThan(0); + + // The file is created empty again, with the modification time of its first emptying, more than + // a retry delay after that. The other file is fixed. + await advance(1170); + filesystem.set(yamlPath, '', firstEmptyModified); + write(jsonPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + expect(readAgainMessages()).toHaveLength(1); + }); + + it('asks for the retry from the end of the load, when the read of another file takes longer than the rest of the delay', async () => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(retryDelayMs * 0.95); + + // The first file is still within its delay when it is read. The read of the second file + // takes 60 ms, past the end of that delay, so the retry is due right away, after the margin. + const readFile = filesystem.readFile.bind(filesystem); + jest.spyOn(filesystem, 'readFile').mockImplementation(async (path: string) => { + if (path === otherYamlPath) { + await new Promise((resolve) => { + setTimeout(resolve, 60); + }); + } + return readFile(path); + }); + reloader.trigger(); + await advance(60); + expect(readAgainMessages()).toHaveLength(2); + expect(reloader.lastRetryDelayMs).toEqual(retryDelayMs / 10); + }); + + it('reads the clock after each file is read, so a slow read of another file does not shorten the delay', async () => { + write(otherYamlPath, otherA); + write(yamlPath, documentA); + makeReloader([otherYamlPath, yamlPath]); + await reloader.reloadNow(); + + // The read of the first file takes 900 ms. The second file is empty when it is read. + write(yamlPath, ''); + filesystem.deferReads = true; + reloader.trigger(); + await advance(0); + expect(filesystem.pendingReads).toHaveLength(1); + await advance(900); + filesystem.deferReads = false; + filesystem.completePendingReads(); + await advance(0); + expect(readAgainMessages()).toHaveLength(1); + expect(reloader.lastRetryDelayMs).toEqual(holdRetryMs); + + // The delay started when the second file was read, so a load 500 ms later still holds it back. + await advance(500); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + }); + + it('asks for a retry a retry delay and a tenth after the first hold', async () => { + await startWith(yamlPath, documentA); + write(yamlPath, ''); + await changeAndSettle(); + expect(reloader.lastRetryDelayMs).toEqual(holdRetryMs); + }); + + it('asks for a retry when the delay ends, plus a tenth, after a later hold', async () => { + await startWith(yamlPath, documentA); + write(yamlPath, ''); + await changeAndSettle(); + await advance(500); + await changeAndSettle(); + expect(reloader.lastRetryDelayMs).toEqual(holdRetryMs - 500); + }); + + it('asks for a retry when the bound ends, plus a tenth, when that is sooner', async () => { + await startWith(yamlPath, documentA); + write(yamlPath, ''); + await changeAndSettle(); + await Array.from({ length: 8 }).reduce>( + (previous) => + previous.then(async () => { + await advance(600); + write(yamlPath, documentB); + write(yamlPath, ''); + await changeAndSettle(); + }), + Promise.resolve(), + ); + // At 4800 ms the file's delay would end at 5800 ms, and its bound ends at 5000 ms. + expect(reloader.lastRetryDelayMs).toEqual(5000 - 4800 + retryDelayMs / 10); + }); + + it.each([ + ['first', [yamlPath, otherYamlPath]], + ['second', [otherYamlPath, yamlPath]], + ])( + 'asks for a retry when the file whose delay ends first is due, when that file is configured %s', + async (_order, paths) => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader(paths); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(400); + write(otherYamlPath, ''); + await changeAndSettle(); + expect(readAgainMessages()).toHaveLength(3); + expect(reloader.lastRetryDelayMs).toEqual(holdRetryMs - 400); + }, + ); + + it('asks for a retry the same way for a hold by a retry', async () => { + await startWith(yamlPath, documentA); + write(yamlPath, '{"flagValues": '); + await changeAndSettle(); + expect(reloader.lastRetryDelayMs).toEqual(retryDelayMs); + + write(yamlPath, ''); + await advance(retryDelayMs); + expect(readAgainMessages()).toHaveLength(1); + expect(reloader.lastRetryDelayMs).toEqual(holdRetryMs); + }); + + it('asks for a retry a retry delay after a failure', async () => { + await startWith(jsonPath, documentA); + write(jsonPath, '{"flagValues": '); + await changeAndSettle(); + expect(failures).toHaveLength(1); + expect(reloader.lastRetryDelayMs).toEqual(retryDelayMs); + }); + + it('holds back a file emptied again after a read of its content when its state looks the same', async () => { + // On a filesystem that records modification times coarsely, a file emptied a second time can + // look exactly as it did the first time. The read of its content in between still starts its + // delay over. The other file is kept written while empty, so that no load applies. + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + const firstEmptyModified = modified; + write(otherYamlPath, ''); + await changeAndSettle(); + expect(readAgainMessages()).toHaveLength(2); + + await advance(500); + write(yamlPath, documentA); + write(otherYamlPath, ''); + await changeAndSettle(); + + await advance(400); + filesystem.set(yamlPath, '', firstEmptyModified); + write(otherYamlPath, ''); + await changeAndSettle(); + + // The other file gets its content back while the first file is still within its new delay. + await advance(700); + write(otherYamlPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + }); + + it('holds back a file whose content moved to it from another configured file', async () => { + // The content in effect comes from the second file. Moving it to the first leaves the + // contents the same, and the load applies them, because the first file now contributes them. + write(otherYamlPath, documentA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + expect(values()).toEqual(['a']); + + filesystem.remove(otherYamlPath); + write(yamlPath, documentA); + await changeAndSettle(); + expect(values()).toEqual(['a', 'a']); + + write(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a', 'a']); + + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'a', 'b']); + }); + + it('never holds back an empty placeholder file, and it does not delay a change to another file', async () => { + write(otherYamlPath, ''); + write(yamlPath, documentA); + makeReloader([otherYamlPath, yamlPath]); + await reloader.reloadNow(); + expect(values()).toEqual(['a']); + + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + expect(readAgainMessages()).toEqual([]); + }); + + it('holds back each of two files that are rewritten together', async () => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + write(otherYamlPath, ''); + await changeAndSettle(); + + // The first file is written. The second is still empty. + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a,a']); + + write(otherYamlPath, otherB); + await changeAndSettle(); + expect(values()).toEqual(['a,a', 'b,b']); + expect(failures).toEqual([]); + }); + + it('applies two files that are emptied together and stay empty at the same retry', async () => { + write(yamlPath, documentA); + write(otherYamlPath, otherA); + makeReloader([yamlPath, otherYamlPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + write(otherYamlPath, ''); + await changeAndSettle(); + expect(readAgainMessages()).toHaveLength(2); + + // Both files started their delays in the same load. + await advance(holdRetryMs - 1); + expect(values()).toEqual(['a,a']); + await advance(1); + expect(values()).toEqual(['a,a', '']); + }); + + it('reports a failure of another file even when a file is held back', async () => { + write(yamlPath, documentA); + write(jsonPath, otherA); + makeReloader([yamlPath, jsonPath]); + await reloader.reloadNow(); + + write(yamlPath, ''); + write(jsonPath, '{"flagValues": '); + await changeAndSettle(); + + expect(failures).toHaveLength(1); + expect(failures[0]).toMatchObject({ kind: 'parse', path: jsonPath, repeated: false }); + expect(values()).toEqual(['a,a']); + }); + + it('applies a file that is deleted and then created empty right away, where a missing file contributes nothing', async () => { + write(yamlPath, documentA); + makeReloader([yamlPath], (config) => { + config.skipUnchanged = false; + }); + await reloader.reloadNow(); + + filesystem.remove(yamlPath); + await changeAndSettle(); + expect(applied[1].files).toEqual([{ path: yamlPath, present: false, flags: 0, segments: 0 }]); + + write(yamlPath, ''); + await changeAndSettle(); + expect(applied).toHaveLength(3); + expect(applied[2].files).toEqual([{ path: yamlPath, present: true, flags: 0, segments: 0 }]); + }); + + it('holds back an emptied file when a retry left over from a failure finds it', async () => { + await startWith(yamlPath, documentA); + + // A malformed edit fails the reload and arms the retry. + write(yamlPath, '{"flagValues": '); + await changeAndSettle(); + expect(failures).toHaveLength(1); + + // The file is rewritten in place, and the retry runs before the new content is written. + write(yamlPath, ''); + await advance(retryDelayMs); + expect(values()).toEqual(['a']); + expect(failures).toHaveLength(1); + expect(readAgainMessages()).toHaveLength(1); + + write(yamlPath, documentB); + await changeAndSettle(); + expect(values()).toEqual(['a', 'b']); + }); + + it('parses an emptied file right away when retries are off', async () => { + write(yamlPath, documentA); + makeReloader([yamlPath], (config) => { + config.retryDelayMs = 0; + }); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a', '']); + }); + + it('does not hold back an empty file again once its empty state is in effect', async () => { + write(yamlPath, documentA); + makeReloader([yamlPath], (config) => { + config.skipUnchanged = false; + }); + await reloader.reloadNow(); + + write(yamlPath, ''); + await changeAndSettle(); + await advance(holdRetryMs); + expect(values()).toEqual(['a', '']); + + // The file is written empty again: nothing it supplied is in effect, so nothing is held back. + write(yamlPath, ' '); + await changeAndSettle(); + expect(values()).toEqual(['a', '', '']); + }); + + it('does not count holding back as a failure, so an unchanged rewrite applies nothing', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + await changeAndSettle(); + write(yamlPath, documentA); + await changeAndSettle(); + + // A reload after a failure applies even unchanged content. Holding back is not a failure. + expect(values()).toEqual(['a']); + expect(failures).toEqual([]); + }); + + it('holds back a file when its state cannot be read', async () => { + await startWith(yamlPath, documentA); + jest.spyOn(filesystem, 'getFileStats').mockRejectedValue(new Error('EIO: i/o error')); + + write(yamlPath, ''); + await changeAndSettle(); + expect(values()).toEqual(['a']); + expect(readAgainMessages()).toHaveLength(1); + expect(logger.getMessages(LogLevel.Error)).toEqual([]); + + await advance(holdRetryMs); + expect(values()).toEqual(['a', '']); + }); + + describe('when a failure is reported after a hold', () => { + // The file is malformed, and a retry has repeated the failure, so the last report is a parse + // failure of the file. + const startFailing = async () => { + await startWith(jsonPath, documentA); + write(jsonPath, '{"flagValues": '); + await changeAndSettle(); + await advance(retryDelayMs); + expect(failures.map((failure) => failure.repeated)).toEqual([false, true]); + }; + + it('reports it as news when the load that held the file back followed a change', async () => { + await startFailing(); + + // The file is emptied, which is a change. The retry that reads it again finds it still + // empty. That failure is news, as it would have been for the load that the change started. + write(jsonPath, ''); + await changeAndSettle(); + expect(failures).toHaveLength(2); + await advance(holdRetryMs); + expect(failures).toHaveLength(3); + expect(failures[2].repeated).toBe(false); + expect(failures[2].error.message).toEqual(emptyJson); + logger.expectMessages([ + { level: LogLevel.Debug, matches: /Reading flag data again after a file was found empty/ }, + ]); + }); + + it('reports it as a repeat when a retry held the file back and the next retry fails', async () => { + await startFailing(); + + // A retry finds the file emptied, without a change signal, and holds it back. The retry + // after it continues that retry, so its failure on the same file is a repeat. + write(jsonPath, ''); + await advance(retryDelayMs); + expect(failures).toHaveLength(2); + expect(readAgainMessages()).toHaveLength(1); + await advance(holdRetryMs); + expect(failures).toHaveLength(3); + expect(failures[2].repeated).toBe(true); + }); + + it('reports it as news when a change follows a hold by a retry', async () => { + await startFailing(); + + write(jsonPath, ''); + await advance(retryDelayMs); + expect(failures).toHaveLength(2); + expect(readAgainMessages()).toHaveLength(1); + + // A change signal does not continue the retry that held the file back. + write(jsonPath, '{"flagValues": '); + await changeAndSettle(); + expect(failures).toHaveLength(3); + expect(failures[2].repeated).toBe(false); + }); + }); + + it('does not read again after close', async () => { + await startWith(yamlPath, documentA); + + write(yamlPath, ''); + filesystem.deferReads = true; + reloader.trigger(); + await advance(0); + expect(filesystem.pendingReads).toHaveLength(1); + reloader.close(); + filesystem.completePendingReads(); + await advance(retryDelayMs * 2); + + expect(filesystem.readCount).toEqual(2); + expect(readAgainMessages()).toEqual([]); + expect(values()).toEqual(['a']); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.test.ts b/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.test.ts index ace23c0667..f2b3b5e211 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/filedata/FileReloader.test.ts @@ -165,7 +165,7 @@ describe('given a reloader over a mock filesystem', () => { expect(failures[0].error.message).toEqual('duplicate flag flag1'); }); - it('stops reading after the first failing file', async () => { + it('reads every file even after one fails, and reports the first failure', async () => { filesystem.set(second, '{"flagValues": {"flag2": true}}'); const readFile = jest.spyOn(filesystem, 'readFile'); makeReloader((config) => { @@ -175,7 +175,46 @@ describe('given a reloader over a mock filesystem', () => { await reloader.reloadNow(); expect(failures).toHaveLength(1); - expect(readFile).toHaveBeenCalledTimes(1); + expect(failures[0].path).toEqual(path); + expect(readFile).toHaveBeenCalledTimes(2); + expect(applied).toHaveLength(0); + }); + + it.each([ + ['content that fails to parse', '{"flagValues"', 'parse'], + ['no content', '', 'parse'], + ['a missing file', undefined, 'read'], + ])('reports the first of two failing files, for %s', async (_case, data, kind) => { + if (data !== undefined) { + filesystem.set(path, data); + filesystem.set(second, data); + } + makeReloader((config) => { + config.paths = [path, second]; + }); + + await reloader.reloadNow(); + + expect(failures).toHaveLength(1); + expect(failures[0]).toMatchObject({ kind, path }); + }); + + it('applies files that changed places with the same contents instead of skipping them', async () => { + filesystem.set(path, '{"flagValues": {"flag1": true}}'); + makeReloader((config) => { + config.paths = [path, second]; + config.policy = testPolicy({ missingFile: 'skip' }); + config.skipUnchanged = true; + }); + await reloader.reloadNow(); + expect(applied[0].files.map((file) => file.present)).toEqual([true, false]); + + filesystem.remove(path); + filesystem.set(second, '{"flagValues": {"flag1": true}}'); + reloader.trigger(); + await flushMicrotasks(); + expect(applied).toHaveLength(2); + expect(applied[1].files.map((file) => file.present)).toEqual([false, true]); }); it('merges multiple files in the configured order', async () => { diff --git a/packages/shared/sdk-server/__tests__/data_sources/filedata/document.test.ts b/packages/shared/sdk-server/__tests__/data_sources/filedata/document.test.ts index 01031edf90..e151315a72 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/filedata/document.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/filedata/document.test.ts @@ -1,4 +1,5 @@ import { + hasContent, isFileNotFoundError, isYamlPath, parseDocumentByExtension, @@ -35,6 +36,28 @@ describe('given the extension-based parser', () => { expect(yamlParser).not.toHaveBeenCalled(); }); + it.each([ + ['data.json', ''], + ['data.json', ' \n\t\r\n'], + ['data.json', '\uFEFF'], + ['data', ''], + ['data.txt', ' '], + ])('rejects %s with no content %j, which is not a JSON document', (path, data) => { + const yamlParser = jest.fn(); + const parse = () => parseDocumentByExtension(yamlParser)(path, data); + expect(parse).toThrow(SyntaxError); + expect(parse).toThrow('the file has no content, which is not a JSON document'); + expect(yamlParser).not.toHaveBeenCalled(); + }); + + it.each(['data.yml', 'data.yaml'])( + 'returns the YAML parser result for %s with no content as it is', + (path) => { + expect(parseDocumentByExtension(() => null)(path, '')).toBeNull(); + expect(parseDocumentByExtension(() => undefined)(path, ' \n')).toBeUndefined(); + }, + ); + it.each(['data.yml', 'data.yaml'])( 'parses %s with the YAML parser even when the content looks like JSON', (path) => { @@ -65,6 +88,14 @@ describe('given the extension-based parser', () => { }); }); +it('treats text with only white space or a byte order mark as no content', () => { + expect(hasContent('')).toBe(false); + expect(hasContent(' \n\t\r\n')).toBe(false); + expect(hasContent('\uFEFF\n')).toBe(false); + expect(hasContent('{}')).toBe(true); + expect(hasContent('\n a')).toBe(true); +}); + it('recognizes YAML paths', () => { expect(isYamlPath('a.yml')).toBe(true); expect(isYamlPath('a.yaml')).toBe(true); diff --git a/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts b/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts index 3cf4524b5f..714d7a422e 100644 --- a/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts @@ -1,3 +1,4 @@ +import { NO_ENTRIES } from '../../src/data_sources/filedata'; import { parseOverrideDocument } from '../../src/overrides/overrideDocument'; const flag1 = { @@ -75,9 +76,18 @@ describe('given the override document parser', () => { ); }); - it('treats a parser result of null or undefined as an empty document', () => { - expect(parseOverrideDocument(() => null)('empty.yaml', '')).toEqual({}); - expect(parseOverrideDocument(() => undefined)('empty.yaml', '')).toEqual({}); + it('treats a parser result of null or undefined as a document with no entries', () => { + expect(parseOverrideDocument(() => null)('empty.yaml', '')).toBe(NO_ENTRIES); + expect(parseOverrideDocument(() => undefined)('empty.yaml', '# a comment')).toBe(NO_ENTRIES); + // An empty document is a document, which the reloader applies right away. + expect(parseOverrideDocument(() => ({}))('empty.yaml', '{}')).not.toBe(NO_ENTRIES); + expect(parseOverrideDocument()('empty.json', '{}')).not.toBe(NO_ENTRIES); + }); + + it.each(['', ' \n\t\n'])('rejects a JSON file with no content %j', (data) => { + expect(() => parseOverrideDocument()('empty.json', data)).toThrow( + 'the file has no content, which is not a JSON document', + ); }); it('rejects malformed JSON', () => { diff --git a/packages/shared/sdk-server/src/api/integrations/FileDataSourceOptions.ts b/packages/shared/sdk-server/src/api/integrations/FileDataSourceOptions.ts index f3322d2858..4593a128f6 100644 --- a/packages/shared/sdk-server/src/api/integrations/FileDataSourceOptions.ts +++ b/packages/shared/sdk-server/src/api/integrations/FileDataSourceOptions.ts @@ -17,6 +17,14 @@ export interface FileDataSourceOptions { * A file that is replaced by a rename, or deleted and created again, is detected. A load * that fails, for example because a file was read while it was written, is retried after a * short delay and the previously loaded data stays in effect until it succeeds. + * A file can be rewritten in place: a file whose content is in effect and that is found empty is + * held back until it has stayed empty, without being written, for about a second, and for at most + * about five seconds while that file keeps being rewritten. Changes to the other files wait with it, + * because a load applies all of the files or none of them. A file that is still empty then fails + * to load, which is reported, and the previously loaded data stays in effect. YAML that is cut + * off between two entries can still parse, so for a YAML file, replacing its entire contents in + * one operation, such as renaming a fully written temporary file over it, avoids briefly applying + * only a part of it. * A change to another entry in a directory causes a reload only when the metadata of a * configured file changed, read through any symbolic link, which is how a mounted ConfigMap * or Secret is updated. diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index 452b6d48f6..3d47f3a879 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -119,6 +119,17 @@ export interface LDDataSystemOptions { * retries. Every applied change is logged at Info level with the overrides in effect and what * each file supplied. * + * A file can be rewritten in place. To remove a file's overrides, delete it or write a document + * with no entries, such as `{}`. A file whose content is in effect and that is found empty, or + * with only a YAML comment or document marker, may be in the middle of a rewrite. The source holds + * it back until it has stayed so, without being written, for about a second, and for at most about + * five seconds while that file keeps being rewritten. Changes to the other files wait with it, because + * a reload applies all of the files or none of them. After that, an empty YAML file removes its + * overrides. An empty JSON file is not a valid document: it fails to load, and the overrides in + * effect stay. YAML that is cut off between two entries can still parse, so for a YAML file, + * replacing its entire contents in one operation, such as renaming a fully written temporary file + * over it, avoids briefly applying only a part of it. + * * @example * ```typescript * const client = init(sdkKey, { @@ -171,6 +182,8 @@ export interface FileOverrideSourceOptions { * A YAML parser for YAML files. The SDK does not include one; to read YAML files, supply a * parser that produces the same structure as `JSON.parse`, for example the `parse` function * of the `yaml` package. A YAML file configured without a parser fails client construction. + * A result of `null` or `undefined`, which YAML parsers return for a file with no content, + * means that the file has no entries. */ yamlParser?: (data: string) => any; } diff --git a/packages/shared/sdk-server/src/data_sources/filedata/FilePoller.ts b/packages/shared/sdk-server/src/data_sources/filedata/FilePoller.ts index 9a3da6718a..a96a9c33f0 100644 --- a/packages/shared/sdk-server/src/data_sources/filedata/FilePoller.ts +++ b/packages/shared/sdk-server/src/data_sources/filedata/FilePoller.ts @@ -1,9 +1,12 @@ import { Filesystem } from '@launchdarkly/js-sdk-common'; /** - * The observed state of one file, or its absence. + * The observed state of one file, or its absence: its modification time and size, as the + * platform's `getFileStats` reports them. The Node platform reads them through any symbolic link. + * + * @internal */ -interface FileState { +export interface FileState { exists: boolean; timestamp: number; size: number; @@ -11,7 +14,12 @@ interface FileState { const absent: FileState = { exists: false, timestamp: 0, size: 0 }; -async function observe(filesystem: Filesystem, path: string): Promise { +/** + * Observes the state of one file. A file that the filesystem cannot examine counts as absent. + * + * @internal + */ +export async function observeFile(filesystem: Filesystem, path: string): Promise { try { if (filesystem.getFileStats) { const stats = await filesystem.getFileStats(path); @@ -31,10 +39,15 @@ async function observe(filesystem: Filesystem, path: string): Promise } function observeAll(filesystem: Filesystem, paths: string[]): Promise { - return Promise.all(paths.map((path) => observe(filesystem, path))); + return Promise.all(paths.map((path) => observeFile(filesystem, path))); } -function sameState(a: FileState, b: FileState): boolean { +/** + * Reports whether two observations of a file are the same. + * + * @internal + */ +export function sameFileState(a: FileState, b: FileState): boolean { return a.exists === b.exists && a.timestamp === b.timestamp && a.size === b.size; } @@ -103,7 +116,7 @@ export default class FilePoller { if (this._closed) { return; } - const changed = current.some((state, index) => !sameState(state, this._last[index])); + const changed = current.some((state, index) => !sameFileState(state, this._last[index])); this._last = current; if (changed) { this._onChange(); diff --git a/packages/shared/sdk-server/src/data_sources/filedata/FileReloader.ts b/packages/shared/sdk-server/src/data_sources/filedata/FileReloader.ts index 254686f481..f2690fa115 100644 --- a/packages/shared/sdk-server/src/data_sources/filedata/FileReloader.ts +++ b/packages/shared/sdk-server/src/data_sources/filedata/FileReloader.ts @@ -1,8 +1,9 @@ import { Filesystem, LDLogger, monotonicNow } from '@launchdarkly/js-sdk-common'; import { Flag } from '../../evaluation/data/Flag'; -import { FileDataDocument, isFileNotFoundError } from './document'; +import { FileDataDocument, hasContent, isFileNotFoundError, NO_ENTRIES } from './document'; import { FileDataPolicy } from './FileDataPolicy'; +import { FileState, observeFile, sameFileState } from './FilePoller'; import { FileSummary, mergeDocuments, MergeResult } from './merge'; /** @@ -31,6 +32,15 @@ export const DEFAULT_RETRY_DELAY_MS = 1000; */ export const MAX_DEBOUNCE_DELAY_MS = 1000; +/** + * Reloads stop holding a file back once they have held it back for this many retry delays, so + * that a file that keeps being written while every reload finds it empty, or files that are + * rewritten in turn, are not held back without end. + * + * @internal + */ +export const MAX_HOLD_RETRY_DELAYS = 5; + /** * The result of one successful reload: the merged items plus a summary of each configured * file. @@ -104,8 +114,11 @@ export interface FileReloaderConfig { */ debounceDelayMs: number; /** - * How long to wait after a failed reload before retrying it without a trigger. Zero or - * negative disables the automatic retry. + * How long to wait after a failed reload before retrying it without a trigger. A file whose + * content is in effect and that is found empty is not applied until it has stayed empty, without + * being written, for the same delay, or until reloads have held it back for + * `MAX_HOLD_RETRY_DELAYS` times the delay. Zero or negative disables the automatic retry, and an + * empty file is then parsed as it is. */ retryDelayMs: number; /** @@ -117,6 +130,27 @@ export interface FileReloaderConfig { type ReloadReason = 'initial' | 'change' | 'retry'; +/** + * A file whose content is in effect and that reloads have found empty, with times on the monotonic + * clock. `since` is since when it has been empty without being written, and `state` its state + * then; `since` is undefined after a reload found the file with content or missing. `firstHeld` is + * when a reload first held it back since a reload that held nothing back read it, so that a file + * that keeps being rewritten, or files that are rewritten in turn, are bounded. + */ +interface EmptyFile { + since?: number; + state?: FileState; + firstHeld?: number; +} + +/** + * Reports whether a parsed document has no entries because the content was no document at all, + * as a YAML parser reports for a file with only a comment or a document marker. + */ +function isNoDocument(document: FileDataDocument | null | undefined): boolean { + return document === null || document === undefined || document === NO_ENTRIES; +} + function asError(err: unknown): Error { return err instanceof Error ? err : new Error(String(err)); } @@ -126,6 +160,10 @@ function asError(err: unknown): Error { * signals, keeps the last good result on failure (by not calling `apply`), retries after * failures, and skips no-op applications. * + * A file that is rewritten in place is truncated and then written, so a reload can find it + * empty. A reload that finds a file empty whose content is in effect keeps the current data and + * reads the file again later, whatever the file's format. See `_holdsBack`. + * * @internal */ export default class FileReloader { @@ -153,6 +191,21 @@ export default class FileReloader { private _lastFailureKey?: string; + // Whether each configured path contributed content to the data in effect, that is, to the last + // reload that was applied or skipped as unchanged. A missing or empty file contributed none. + private readonly _appliedHadContent = new Map(); + + // The paths whose content is in effect and that reloads have found empty since. + private readonly _emptyFiles = new Map(); + + // The delay of the retry that was armed last. Tests read it. + private _lastRetryDelayMs?: number; + + // Set by a reload that held back an empty file and armed the retry to read it again, to that + // reload's reason, until the next reload starts. The next reload continues the deferred one, so + // it reports a failure the way the deferred reload would have. + private _readAgainReason?: ReloadReason; + constructor(private readonly _config: FileReloaderConfig) {} /** @@ -165,6 +218,15 @@ export default class FileReloader { return this._enqueue('initial'); } + /** + * The delay of the retry that was armed last, in milliseconds, if any. For tests. + * + * @internal + */ + get lastRetryDelayMs(): number | undefined { + return this._lastRetryDelayMs; + } + /** * Signals that the files may have changed. The reload happens after the debounce delay, or no * later than the maximum debounce delay after the first trigger of a burst. Triggers that @@ -241,15 +303,16 @@ export default class FileReloader { } } - private _armRetry(): void { + private _armRetry(delayMs: number = this._config.retryDelayMs): void { if (this._closed || this._config.retryDelayMs <= 0) { return; } this._clearRetry(); + this._lastRetryDelayMs = delayMs; this._retryTimer = setTimeout(() => { this._retryTimer = undefined; this._enqueue('retry'); - }, this._config.retryDelayMs); + }, delayMs); } /** @@ -258,14 +321,21 @@ export default class FileReloader { * wins for a key. Every filesystem and parsing failure is caught and reported, so the returned * promise never rejects. */ - private async _reload(reason: ReloadReason): Promise { + private async _reload(requested: ReloadReason): Promise { if (this._closed) { return; } const { logger, paths, policy } = this._config; - if (reason === 'change') { + // The retry that reads an emptied file again continues the reload that found it empty, so a + // failure is news when it would have been news to that reload. + const deferredReason = requested === 'retry' ? this._readAgainReason : undefined; + const reason = deferredReason ?? requested; + this._readAgainReason = undefined; + if (requested === 'change') { logger?.info('Reloading flag data after detecting a change'); - } else if (reason === 'retry') { + } else if (deferredReason) { + logger?.debug('Reading flag data again after a file was found empty'); + } else if (requested === 'retry') { logger?.debug('Retrying flag data load after earlier failure'); } // A pending retry is superseded by this reload. It either succeeds, or it fails and arms a @@ -274,40 +344,105 @@ export default class FileReloader { const documents: FileDataDocument[] = []; const files: FileSummary[] = []; - const contents: string[] = []; + // Each configured path with its content, or null when it is missing, for skip-unchanged. The + // format is chosen by the path's extension, so the same content under another path can be + // other data. + const contents: [string, string | null][] = []; + // Whether each path that this reload read, or found missing, has content. + const hadContent = new Map(); + // The first failure. The other files are still read and classified, so that every file this + // reload gets to is reached, and the reload fails with this failure after the loop. let failure: LoadFailure | undefined; + // The empty files that this reload holds back. The other files are still read, so that files + // that are emptied together start their delays in the same reload. + const heldBack: string[] = []; + // Every file that this reload read or found missing and did not hold back, including a file + // that failed to read or parse. + const reached: string[] = []; const readAll = paths.reduce( (previous, path) => previous.then(async () => { - if (failure) { - return; - } let data: string; try { data = await this._config.filesystem.readFile(path); } catch (err) { - if (policy.missingFile === 'skip' && isFileNotFoundError(err)) { + reached.push(path); + const missing = isFileNotFoundError(err); + if (missing) { + this._fileChanged(path); + } + if (missing && policy.missingFile === 'skip') { logger?.debug(`File ${path} does not exist. It contributes no data.`); + hadContent.set(path, false); + contents.push([path, null]); files.push({ path, present: false, flags: 0, segments: 0 }); return; } - failure = { kind: 'read', error: asError(err), path, repeated: false }; + failure = failure ?? { kind: 'read', error: asError(err), path, repeated: false }; return; } - contents.push(data); - try { - documents.push(policy.parseDocument(path, data)); - } catch (err) { - failure = { kind: 'parse', error: asError(err), path, repeated: false }; + // A file with no content, or with content that its parser reads as no document, such as + // YAML with only a comment, has no entries. A read can see that much of a file that is + // being written. Content with no document is invalid JSON, so it is only parsed when the + // file is not held back. + const blank = !hasContent(data); + let document: FileDataDocument | undefined; + if (!blank) { + try { + document = policy.parseDocument(path, data); + } catch (err) { + // Content ends the file's empty state, even content that fails to parse. + reached.push(path); + this._fileChanged(path); + failure = failure ?? { kind: 'parse', error: asError(err), path, repeated: false }; + return; + } + } + const noEntries = blank || isNoDocument(document); + if (!noEntries) { + this._fileChanged(path); + } else if (await this._holdsBack(path)) { + heldBack.push(path); return; } + reached.push(path); + hadContent.set(path, !noEntries); + if (blank) { + try { + document = policy.parseDocument(path, data); + } catch (err) { + failure = failure ?? { kind: 'parse', error: asError(err), path, repeated: false }; + return; + } + } + contents.push([path, data]); + documents.push(document as FileDataDocument); files.push({ path, present: true, flags: 0, segments: 0 }); }), Promise.resolve(), ); await readAll; + if (heldBack.length === 0) { + // A reload that holds nothing back, whether it applies, skips or fails, starts the bound over + // for every file, since it reads them all: such a file is no longer held, so its next rewrite + // is a new one. A reload that holds a file back starts no bound over, even when it fails. + reached.forEach((path) => { + const empty = this._emptyFiles.get(path); + if (empty) { + empty.firstHeld = undefined; + } + }); + } + + // A file that cannot be read or parsed fails the reload, whether or not another file is held + // back. + if (!failure && heldBack.length > 0) { + this._holdBack(heldBack, reason); + return; + } + let merged: MergeResult | undefined; if (!failure) { try { @@ -342,6 +477,7 @@ export default class FileReloader { const recovering = this._lastFailureKey !== undefined; const content = JSON.stringify(contents); if (this._config.skipUnchanged && !recovering && content === this._lastGoodContent) { + // Every file has the same content as in the data in effect, so it contributes what it did. this._lastFailureKey = undefined; return; } @@ -361,6 +497,106 @@ export default class FileReloader { this._lastFailureKey = undefined; this._lastGoodContent = content; this._lastGoodFlags = nextFlags; + this._rememberAppliedContent(hadContent); + } + + // Called when the data of a reload is in effect. The files' bounds start over. + private _rememberAppliedContent(hadContent: Map): void { + hadContent.forEach((value, path) => this._appliedHadContent.set(path, value)); + this._emptyFiles.clear(); + } + + // Called when a reload finds a file with content, even content that fails to parse, or finds it + // missing. That ends the file's empty state: the next time a reload finds it empty, its delay + // starts over. Its bound does not. + private _fileChanged(path: string): void { + const empty = this._emptyFiles.get(path); + if (empty) { + empty.since = undefined; + } + } + + /** + * Decides whether a reload holds back a file that it found with no entries. + * + * A file whose content is in effect and that has none now may be in the middle of an in-place + * rewrite: a writer that truncates the file and then writes the new content. Such a file is held + * back until it has stayed so, without being written, for the retry delay. A change of its + * state, its modification time and size as the platform reports them, means that it was written + * since a reload last found it so, for example rewritten and truncated again between two + * reloads, so the delay starts over. A file that keeps being written while every reload finds it + * empty, or files that are rewritten in turn so that every reload finds one of them empty, would + * be held back without end, so a file is held back for at most `MAX_HOLD_RETRY_DELAYS` retry + * delays. That bound is the file's own: it starts at the first reload that holds the file back, + * and starts over only when data is applied, or at a reload that holds nothing back, whether the + * file reads, parses, fails or is missing. A reload reads every file even after one fails, so + * that it starts every file's bound over. The bounds of files that keep being written while + * empty do not overlap: with n such files whose rewrites start 4.5 retry delays apart, the first + * application comes about 4.5 n + 0.5 retry delays after the first rewrite started, and a failure + * of another file that recurs exactly when each bound ends could hold them back without end. A + * file whose content is not in effect, such as a placeholder that has always been empty, is + * parsed right away, and so is any file when retries are off, since nothing would read it again. + * + * The clock is read after the file's own read, so that a slow read of another file does not + * shorten this file's delay. + */ + private async _holdsBack(path: string): Promise { + const { retryDelayMs } = this._config; + if (retryDelayMs <= 0 || this._appliedHadContent.get(path) !== true) { + return false; + } + const state = await observeFile(this._config.filesystem, path); + const now = monotonicNow(); + let empty = this._emptyFiles.get(path); + if (!empty) { + empty = {}; + this._emptyFiles.set(path, empty); + } + if (empty.since === undefined || !empty.state || !sameFileState(empty.state, state)) { + empty.since = now; + empty.state = state; + } + if (now - empty.since >= retryDelayMs) { + return false; + } + if ( + empty.firstHeld !== undefined && + now - empty.firstHeld >= MAX_HOLD_RETRY_DELAYS * retryDelayMs + ) { + return false; + } + empty.firstHeld = empty.firstHeld ?? now; + return true; + } + + /** + * Holds back the files that `_holdsBack` decided to hold back. Whatever their format, the reload + * keeps the current data and arms the retry without reporting a failure. The retry comes when the + * first of the files has stayed empty for the retry delay, or has been held back for as long as + * it may be, plus a tenth of the retry delay, so that a timer that fires a little early still + * finds the time over. The change signal for a write starts a reload sooner. Once a file has + * stayed empty for the retry delay, a reload parses it: a file that is still empty is invalid + * JSON, or a YAML document with no entries. + */ + private _holdBack(paths: string[], reason: ReloadReason): void { + if (this._closed) { + return; + } + const { logger, retryDelayMs } = this._config; + let until = Infinity; + paths.forEach((path) => { + logger?.debug( + `File ${path} is empty, possibly while it is being rewritten; reading it again before applying it`, + ); + const { since, firstHeld } = this._emptyFiles.get(path)!; + until = Math.min( + until, + since! + retryDelayMs, + firstHeld! + MAX_HOLD_RETRY_DELAYS * retryDelayMs, + ); + }); + this._readAgainReason = reason; + this._armRetry(Math.max(until - monotonicNow(), 0) + retryDelayMs / 10); } private _fail(failure: LoadFailure, reason: ReloadReason): void { diff --git a/packages/shared/sdk-server/src/data_sources/filedata/document.ts b/packages/shared/sdk-server/src/data_sources/filedata/document.ts index d535c9e86b..27e905f0e1 100644 --- a/packages/shared/sdk-server/src/data_sources/filedata/document.ts +++ b/packages/shared/sdk-server/src/data_sources/filedata/document.ts @@ -20,6 +20,16 @@ export interface FileDataDocument { segments?: Record; } +/** + * The document of a file that has no entries because its content is no document at all: a YAML + * file with only white space, a comment, or a document marker, for which a YAML parser returns + * null. A parser returns it, rather than an empty object, so that the reloader can tell such a + * file from one that holds an empty document such as `{}`. + * + * @internal + */ +export const NO_ENTRIES: FileDataDocument = Object.freeze({}); + /** * Parses the text of one file into a document. * @@ -36,6 +46,16 @@ export function isYamlPath(path: string): boolean { return path.endsWith('.yml') || path.endsWith('.yaml'); } +/** + * Reports whether the text of a file has content: anything other than white space. A byte order + * mark counts as white space. + * + * @internal + */ +export function hasContent(data: string): boolean { + return /\S/.test(data); +} + function parseYaml(path: string, data: string, yamlParser?: YamlParser): any { if (!yamlParser) { throw new Error(`Attempted to parse yaml file (${path}) without parser.`); @@ -43,11 +63,24 @@ function parseYaml(path: string, data: string, yamlParser?: YamlParser): any { return yamlParser(data); } +function parseJson(data: string): any { + if (!hasContent(data)) { + // JSON.parse fails here too, with a message that does not say what is wrong. A file that is + // rewritten in place can be empty for a moment, and the failure keeps the last good data. + throw new SyntaxError('the file has no content, which is not a JSON document'); + } + return JSON.parse(data); +} + /** * A parser that chooses the format by file extension only. A `.yml` or `.yaml` file uses the * YAML parser, and every other file is JSON. The parsed value is returned as it is. This is the * rule of the file data sources. * + * Whether a file with no content is valid depends on its format. A JSON file with no content, + * or only white space, is not a JSON document and fails to parse. A YAML parser returns null or + * undefined for it, which is returned as it is. + * * @internal */ export function parseDocumentByExtension(yamlParser?: YamlParser): DocumentParser { @@ -55,7 +88,7 @@ export function parseDocumentByExtension(yamlParser?: YamlParser): DocumentParse if (isYamlPath(path)) { return parseYaml(path, data, yamlParser); } - return JSON.parse(data); + return parseJson(data); }; } diff --git a/packages/shared/sdk-server/src/data_sources/filedata/index.ts b/packages/shared/sdk-server/src/data_sources/filedata/index.ts index 18ef8828a9..114b0ffb48 100644 --- a/packages/shared/sdk-server/src/data_sources/filedata/index.ts +++ b/packages/shared/sdk-server/src/data_sources/filedata/index.ts @@ -1,8 +1,10 @@ import { DocumentParser, FileDataDocument, + hasContent, isFileNotFoundError, isYamlPath, + NO_ENTRIES, parseDocumentByExtension, YamlParser, } from './document'; @@ -13,7 +15,7 @@ import { MissingFileHandling, } from './FileDataPolicy'; import FileDirectoryWatcher, { directoryOf } from './FileDirectoryWatcher'; -import FilePoller from './FilePoller'; +import FilePoller, { FileState, observeFile, sameFileState } from './FilePoller'; import FileReloader, { DEFAULT_DEBOUNCE_DELAY_MS, DEFAULT_RETRY_DELAY_MS, @@ -37,7 +39,9 @@ export { FilePoller, FileReloader, FileReloaderConfig, + FileState, FileSummary, + hasContent, isFileNotFoundError, isYamlPath, ItemCategory, @@ -47,7 +51,10 @@ export { mergeDocuments, MergeResult, MissingFileHandling, + NO_ENTRIES, + observeFile, parseDocumentByExtension, ReloadResult, + sameFileState, YamlParser, }; diff --git a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts index cca60452aa..91af292363 100644 --- a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts +++ b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts @@ -143,7 +143,11 @@ function countsText(flags: number, segments: number): string { * A reload replaces the entire override set. A configured file that does not exist contributes * no overrides, so deleting a file removes its overrides. A file that exists but cannot be read * or parsed makes that whole reload fail. The previously loaded overrides stay in effect, the - * failure is logged, and the source retries after a short delay. Every applied change is logged + * failure is logged, and the source retries after a short delay. A JSON file with no content + * cannot be parsed. A file whose content is in effect and that is found with no entries, empty or + * with only a YAML comment, may be in the middle of an in-place rewrite, so it is held back until + * it has stayed so, without being written, for the retry delay, and for at most five retry delays + * while that file keeps being rewritten; the other files wait with it. Every applied change is logged * at Info level with the overrides in effect and what each file supplied. * * Flag overrides are currently experimental and subject to change. diff --git a/packages/shared/sdk-server/src/overrides/overrideDocument.ts b/packages/shared/sdk-server/src/overrides/overrideDocument.ts index 80459806b5..e8ec2e0d03 100644 --- a/packages/shared/sdk-server/src/overrides/overrideDocument.ts +++ b/packages/shared/sdk-server/src/overrides/overrideDocument.ts @@ -3,6 +3,7 @@ import { isNullish } from '@launchdarkly/js-sdk-common'; import { DocumentParser, FileDataDocument, + NO_ENTRIES, parseDocumentByExtension, YamlParser, } from '../data_sources/filedata'; @@ -33,8 +34,10 @@ function validateItems(document: Record, member: 'flags' | 'segment function validateDocument(parsed: any): FileDataDocument { if (isNullish(parsed)) { - // An empty file is an empty document. - return {}; + // A YAML parser returns null or undefined for a file with no content, or with only a comment + // or a document marker, which is a document with no entries. A JSON file with no content fails + // to parse instead. + return NO_ENTRIES; } if (!isPlainObject(parsed)) { throw new Error('file data must be an object'); @@ -65,7 +68,8 @@ function validateDocument(parsed: any): FileDataDocument { * * The result is validated: the document and its `flags`, `flagValues`, and `segments` members * must be objects, every flag and segment entry must be an object, and an entry that omits its - * `key` gets it from the map key. An empty document is a document with no members. + * `key` gets it from the map key. A YAML file with no content is a document with no members, and + * a JSON file with no content fails to parse. * * @internal */