From 692518ee69b28949cf96abee59d8a3879c05e051 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Fri, 11 Sep 2026 14:33:57 +0200 Subject: [PATCH 1/2] fix(node-sdk): leave graceful shutdown to the application --- .changeset/quiet-shutdown.md | 7 ++ packages/node-sdk/README.md | 14 ++- packages/node-sdk/src/client.ts | 5 +- packages/node-sdk/src/flusher.ts | 22 +--- packages/node-sdk/src/types.ts | 5 +- .../node-sdk/test/flusher-process.test.ts | 107 ++++++++++++++++++ packages/node-sdk/test/flusher.test.ts | 95 ++++------------ 7 files changed, 159 insertions(+), 96 deletions(-) create mode 100644 .changeset/quiet-shutdown.md create mode 100644 packages/node-sdk/test/flusher-process.test.ts diff --git a/.changeset/quiet-shutdown.md b/.changeset/quiet-shutdown.md new file mode 100644 index 000000000..3d630b265 --- /dev/null +++ b/.changeset/quiet-shutdown.md @@ -0,0 +1,7 @@ +--- +"@reflag/node-sdk": patch +--- + +Stop installing process signal handlers and forcing process termination after flushing. This prevents the SDK from interrupting application-managed graceful shutdown, including handlers registered after SDK initialization. + +`batchOptions.flushOnExit` now only flushes automatically on natural event-loop shutdown (`beforeExit`). Applications must await `client.flush()` in their own shutdown hooks to flush before signal-driven termination or an explicit `process.exit()`. diff --git a/packages/node-sdk/README.md b/packages/node-sdk/README.md index a06e82c0d..df4e5383b 100644 --- a/packages/node-sdk/README.md +++ b/packages/node-sdk/README.md @@ -986,8 +986,8 @@ these functions. ReflagClient employs a batching technique to minimize the number of calls that are sent to Reflag's servers. -By default, the SDK automatically subscribes to process exit signals and attempts to flush -any pending events. This behavior is controlled by the `flushOnExit` option in the client configuration: +By default, the SDK attempts to flush pending events when the Node.js event loop empties +(the `beforeExit` event). This behavior is controlled by the `flushOnExit` option in the client configuration: ```typescript const client = new ReflagClient({ @@ -997,6 +997,16 @@ const client = new ReflagClient({ }); ``` +The SDK does not install signal handlers or call `process.exit()`. Your application retains +control of its shutdown sequence, regardless of when it registers its signal handlers. + +Node.js does not emit `beforeExit` for unhandled termination signals (such as `SIGTERM` or +`SIGINT`) or explicit `process.exit()` calls. For these shutdown paths, **await `client.flush()` +in your application's existing graceful shutdown hook**, after stopping incoming work and +waiting for in-flight requests or jobs to finish, and before exiting. This ensures events +produced during shutdown are included. Automatic flushing is best-effort and does not replace +an application-managed shutdown hook. + ## Tracking custom events and setting custom attributes Tracking allows events and updating user/company attributes in Reflag. diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 293ee46dc..ac5e84032 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -918,7 +918,10 @@ export class ReflagClient { * It is recommended to call this method when the application is shutting down to ensure all events are sent * before the process exits. * - * This method is automatically called when the process exits if `batchOptions.flushOnExit` is `true` in the options (default). + * This method is automatically called on natural process exit (`beforeExit`) if + * `batchOptions.flushOnExit` is `true` in the options (default). For signal-driven + * shutdown or an explicit `process.exit()`, await this method in your application's + * shutdown hook before exiting. The SDK does not install signal handlers. */ public async flush() { if (this._config.offline) { diff --git a/packages/node-sdk/src/flusher.ts b/packages/node-sdk/src/flusher.ts index 930cb21ee..ea409f8b4 100644 --- a/packages/node-sdk/src/flusher.ts +++ b/packages/node-sdk/src/flusher.ts @@ -1,12 +1,8 @@ -import { constants } from "os"; - import { END_FLUSH_TIMEOUT_MS } from "./config"; import { TimeoutError, withTimeout } from "./utils"; type Callback = () => Promise; -const killSignals = ["SIGINT", "SIGTERM", "SIGHUP", "SIGBREAK"] as const; - export function subscribe( callback: Callback, timeout: number = END_FLUSH_TIMEOUT_MS, @@ -38,22 +34,12 @@ export function subscribe( state = true; }; - killSignals.forEach((signal) => { - const hasListeners = process.listenerCount(signal) > 0; - - if (hasListeners) { - process.prependListener(signal, wrappedCallback); - } else { - process.on(signal, async () => { - await wrappedCallback(); - process.exit(0x80 + constants.signals[signal]); - }); - } - }); - + // Signal listeners suppress Node's default termination behavior. Leave signals + // and shutdown coordination to the application; only flush on natural exit. process.on("beforeExit", wrappedCallback); process.on("exit", () => { - if (!state) { + // If beforeExit never ran, the application may have flushed explicitly. + if (state === false) { console.error( "[Reflag SDK] Failed to finalize the flushing of events on process exit.", ); diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index d7bd0123d..c599f5ef0 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -612,7 +612,10 @@ export type BatchBufferOptions = { intervalMs?: number; /** - * Whether to flush the buffer on exit. + * Whether to flush the buffer on natural process exit (`beforeExit`). + * This does not install signal handlers. For signal-driven shutdown or an + * explicit `process.exit()`, await `client.flush()` in your application's + * shutdown hook before exiting. * * @defaultValue `true` */ diff --git a/packages/node-sdk/test/flusher-process.test.ts b/packages/node-sdk/test/flusher-process.test.ts new file mode 100644 index 000000000..663d2d708 --- /dev/null +++ b/packages/node-sdk/test/flusher-process.test.ts @@ -0,0 +1,107 @@ +import { spawnSync } from "child_process"; +import { resolve } from "path"; + +import { describe, expect, it } from "vitest"; + +// Use real child processes: mocking process.exit or emitting an EventEmitter +// event cannot verify Node's default signal behavior or event-loop shutdown. +function runProcess(script: string) { + const flusherPath = resolve(__dirname, "../src/flusher.ts"); + const result = spawnSync( + process.execPath, + [ + "-r", + require.resolve("ts-node/register/transpile-only"), + "-e", + `const { subscribe } = require(${JSON.stringify(flusherPath)}); + const flush = async () => { + await new Promise(resolve => setTimeout(resolve, 20)); + console.log("flushed"); + }; + ${script}`, + ], + { + encoding: "utf8", + timeout: 10000, + env: { + ...process.env, + TS_NODE_SKIP_PROJECT: "true", + TS_NODE_COMPILER_OPTIONS: JSON.stringify({ + module: "CommonJS", + moduleResolution: "Node", + }), + }, + }, + ); + + expect(result.error).toBeUndefined(); + expect(result.stderr).toBe(""); + return result; +} + +describe("flusher process lifecycle", () => { + it("flushes each client once on natural exit and preserves the exit code", () => { + const result = runProcess(` + subscribe(flush); + subscribe(flush); + process.exitCode = 7; + `); + + expect(result.status).toBe(7); + expect(result.signal).toBeNull(); + expect(result.stdout).toBe("flushed\nflushed\n"); + }); + + it("allows an application to flush explicitly and exit without a false warning", () => { + const result = runProcess(` + subscribe(flush); + flush().then(() => process.exit(7)); + `); + + expect(result.status).toBe(7); + expect(result.signal).toBeNull(); + expect(result.stdout).toBe("flushed\n"); + }); + + describe.skipIf(process.platform === "win32")("POSIX signals", () => { + it.each(["SIGTERM", "SIGINT"])( + "preserves default termination for unhandled %s", + (signal) => { + const result = runProcess(` + subscribe(flush); + setInterval(() => {}, 1000); + setImmediate(() => process.kill(process.pid, "${signal}")); + `); + + expect(result.status).toBeNull(); + expect(result.signal).toBe(signal); + expect(result.stdout).toBe(""); + }, + ); + + it.each(["before", "after"])( + "allows async application shutdown registered %s SDK initialization to finish", + (order) => { + const registration = ` + process.once("SIGTERM", async () => { + await new Promise(resolve => setTimeout(resolve, 100)); + console.log("shutdown complete"); + process.exitCode = 7; + clearInterval(keepAlive); + }); + `; + const result = runProcess(` + const keepAlive = setInterval(() => {}, 1000); + ${order === "before" ? registration : ""} + subscribe(flush); + ${order === "after" ? registration : ""} + setImmediate(() => process.kill(process.pid, "SIGTERM")); + `); + + expect(result.status).toBe(7); + expect(result.signal).toBeNull(); + expect(result.stdout).toBe("shutdown complete\nflushed\n"); + }, + ); + }); +}); diff --git a/packages/node-sdk/test/flusher.test.ts b/packages/node-sdk/test/flusher.test.ts index afce802fb..7fa8ef5ae 100644 --- a/packages/node-sdk/test/flusher.test.ts +++ b/packages/node-sdk/test/flusher.test.ts @@ -1,14 +1,4 @@ -import { constants } from "os"; - -import { - afterEach, - beforeEach, - describe, - expect, - it, - MockInstance, - vi, -} from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { subscribe } from "../src/flusher"; @@ -25,16 +15,9 @@ describe("flusher", () => { .spyOn(process, "on") .mockImplementation((_, __) => process); - const mockProcessPrependListener = ( - vi.spyOn(process, "prependListener") as unknown as MockInstance< - [event: NodeJS.Signals, listener: NodeJS.SignalsListener], - NodeJS.Process - > - ).mockImplementation((_, __) => process); - - const mockListenerCount = vi - .spyOn(process, "listenerCount") - .mockReturnValue(0); + const mockProcessPrependListener = vi + .spyOn(process, "prependListener") + .mockImplementation((_, __) => process); function timedCallback(ms: number) { return vi.fn().mockImplementation( @@ -45,12 +28,8 @@ describe("flusher", () => { ); } - function getHandler(eventName: string, prepended = false) { - return prepended - ? mockProcessPrependListener.mock.calls.filter( - ([evt]) => evt === eventName, - )[0][1] - : mockProcessOn.mock.calls.filter(([evt]) => evt === eventName)[0][1]; + function getHandler(eventName: string) { + return mockProcessOn.mock.calls.filter(([evt]) => evt === eventName)[0][1]; } beforeEach(() => { @@ -62,44 +41,14 @@ describe("flusher", () => { vi.resetAllMocks(); }); - describe("signal handling", () => { - const signals = ["SIGINT", "SIGTERM", "SIGHUP", "SIGBREAK"] as const; - - describe.each(signals)("signal %s", (signal) => { - it("should handle signal with no existing listeners", async () => { - mockListenerCount.mockReturnValue(0); - const callback = vi.fn().mockResolvedValue(undefined); - - subscribe(callback); - expect(mockProcessOn).toHaveBeenCalledWith( - signal, - expect.any(Function), - ); - - getHandler(signal)(signal); - await vi.runAllTimersAsync(); - - expect(callback).toHaveBeenCalledTimes(1); - expect(mockExit).toHaveBeenCalledWith(0x80 + constants.signals[signal]); - }); - - it("should prepend handler when listeners exist", async () => { - mockListenerCount.mockReturnValue(1); - const callback = vi.fn().mockResolvedValue(undefined); - - subscribe(callback); - - expect(mockProcessPrependListener).toHaveBeenCalledWith( - signal, - expect.any(Function), - ); + it("should subscribe only to natural exit, without installing signal handlers", () => { + subscribe(vi.fn().mockResolvedValue(undefined)); - getHandler(signal, true)(signal); - - expect(callback).toHaveBeenCalledTimes(1); - expect(mockExit).not.toHaveBeenCalled(); - }); - }); + expect(mockProcessOn.mock.calls.map(([event]) => event)).toEqual([ + "beforeExit", + "exit", + ]); + expect(mockProcessPrependListener).not.toHaveBeenCalled(); }); describe("beforeExit handling", () => { @@ -108,9 +57,10 @@ describe("flusher", () => { subscribe(callback); - getHandler("beforeExit")(); + await getHandler("beforeExit")(); expect(callback).toHaveBeenCalledTimes(1); + expect(mockExit).not.toHaveBeenCalled(); }); it("should not call callback multiple times", async () => { @@ -149,14 +99,12 @@ describe("flusher", () => { }); describe("exit state handling", () => { - it("should log error if exit occurs before flushing starts", () => { + it("should not report a failed flush when beforeExit never ran", () => { subscribe(timedCallback(0)); getHandler("exit")(); - expect(mockConsoleError).toHaveBeenCalledWith( - "[Reflag SDK] Failed to finalize the flushing of events on process exit.", - ); + expect(mockConsoleError).not.toHaveBeenCalled(); }); it("should log error if exit occurs before flushing completes", async () => { @@ -196,16 +144,15 @@ describe("flusher", () => { }); }); - it("should run the callback only once", async () => { + it("should not flush again when beforeExit fires after flushing completes", async () => { const callback = vi.fn().mockResolvedValue(undefined); subscribe(callback); - getHandler("SIGINT")("SIGINT"); - getHandler("beforeExit")(); - - await vi.runAllTimersAsync(); + await getHandler("beforeExit")(); + await getHandler("beforeExit")(); expect(callback).toHaveBeenCalledTimes(1); + expect(mockExit).not.toHaveBeenCalled(); }); }); From 73289402adb913c0d5e10cf757f36f2a3cb53ef1 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Fri, 11 Sep 2026 14:37:00 +0200 Subject: [PATCH 2/2] docs(node-sdk): simplify flushing guidance --- packages/node-sdk/README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/node-sdk/README.md b/packages/node-sdk/README.md index df4e5383b..a2f756e9f 100644 --- a/packages/node-sdk/README.md +++ b/packages/node-sdk/README.md @@ -997,9 +997,6 @@ const client = new ReflagClient({ }); ``` -The SDK does not install signal handlers or call `process.exit()`. Your application retains -control of its shutdown sequence, regardless of when it registers its signal handlers. - Node.js does not emit `beforeExit` for unhandled termination signals (such as `SIGTERM` or `SIGINT`) or explicit `process.exit()` calls. For these shutdown paths, **await `client.flush()` in your application's existing graceful shutdown hook**, after stopping incoming work and