fix: Keep file data and overrides through an in-place rewrite of a file

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<void>,
+    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([]);
+    },
+  );
+});

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<FileOverrideSourceOptions, 'type'>,
+  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<void>, 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<void>;
+        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,
+    );
+  });
 });

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<void> {
+  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<unknown>): {
+  served: unknown[];
+  stop: () => Promise<void>;
+} {
+  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;
+    },
+  };
+}