From ee159b80923a732c259cd33bafae7cf015dfd72c Mon Sep 17 00:00:00 2001 From: Jeffrey Chupp Date: Tue, 8 Apr 2025 10:52:59 -0400 Subject: [PATCH] fix: Telemetry hanging after `prefab.close()` Telemetry timeouts kept prefab running such that you'd need to manually `process.exit()` to close your script. --- .github/workflows/test.yaml | 6 ++++ .gitignore | 1 + src/__tests__/prefab.test.ts | 54 ++++++++++++++++++++++++++++++++++++ src/prefab.ts | 12 ++++++++ src/telemetry/reporter.ts | 10 +++++++ 5 files changed, 83 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9172375..22df99b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -25,6 +25,12 @@ jobs: with: node-version: ${{ matrix.node-version }} + # We use this for a single test that runs Bun for a subprocess + - name: Install Bun + uses: oven-sh/setup-bun@v1 + with: + bun-version: latest + - run: npm ci - run: npm run build --if-present diff --git a/.gitignore b/.gitignore index 933120b..889d31f 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ node_modules .eslintcache dist .envrc +src/__tests__/temp-test.ts diff --git a/src/__tests__/prefab.test.ts b/src/__tests__/prefab.test.ts index f519355..549c7c5 100644 --- a/src/__tests__/prefab.test.ts +++ b/src/__tests__/prefab.test.ts @@ -1,5 +1,7 @@ import * as path from "path"; import Long from "long"; +import fs from "fs"; +import { spawn } from "child_process"; import basicConfig from "./fixtures/basicConfig"; import { DEFAULT_SOURCES } from "../sources"; @@ -1158,4 +1160,56 @@ describe("prefab", () => { expect(prefab.get(secret.key)).toEqual(clearText); }); }); + + describe("closing behavior", () => { + // NOTE: for ease of running the subprocess, we use Bun here. + // https://bun.sh/docs/installation + it("closes the prefab when the resolver is closed", async () => { + const testFile = path.join(__dirname, "temp-test.ts"); + const testCode = ` +import { Prefab } from "../prefab"; + +(async () => { + const prefab = new Prefab({ + apiKey: "${validApiKey}", + }); + await prefab.init(); + const value = prefab.get("abc"); + console.log(\`ABC is \${value}\`); + prefab.close(); +})(); +`; + fs.writeFileSync(testFile, testCode); + + const startTime = Date.now(); + const subprocess = spawn("bun", ["run", testFile]); + + let output = ""; + let errorOutput = ""; + + subprocess.stdout.on("data", (data: Buffer) => { + output += data.toString(); + }); + + subprocess.stderr.on("data", (data: Buffer) => { + errorOutput += data.toString(); + }); + + await new Promise((resolve) => { + subprocess.on("close", () => { + resolve(); + }); + }); + + const duration = Date.now() - startTime; + fs.unlinkSync(testFile); + + if (errorOutput.length > 0) { + console.error("Subprocess stderr:", errorOutput); + } + + expect(output.trim()).toEqual("ABC is true"); + expect(duration).toBeLessThan(3000); + }); + }); }); diff --git a/src/prefab.ts b/src/prefab.ts index 07d17c7..3a22bbd 100644 --- a/src/prefab.ts +++ b/src/prefab.ts @@ -458,6 +458,18 @@ class Prefab implements PrefabInterface { if (this.pollTimeout !== undefined && this.pollTimeout !== null) { this.pollTimeout.unref(); } + + // Clear all telemetry timeouts + Object.values(this.telemetry).forEach((telemetryComponent) => { + // First disable the component + telemetryComponent.enabled = false; + // Then clear its timeout + if (telemetryComponent.timeout !== undefined) { + clearTimeout(telemetryComponent.timeout); + telemetryComponent.timeout = undefined; + } + }); + this.running = false; } } diff --git a/src/telemetry/reporter.ts b/src/telemetry/reporter.ts index 02f7e87..5f071d2 100644 --- a/src/telemetry/reporter.ts +++ b/src/telemetry/reporter.ts @@ -5,9 +5,19 @@ import type { Telemetry } from "./types"; export const now = (): Long => Long.fromNumber(Date.now()); const syncTelemetry = (telemetry: Telemetry, backoff: Backoff): void => { + // Exit early if telemetry is disabled + if (!telemetry.enabled) { + return; + } + const delay = backoff.call(); telemetry.timeout = setTimeout(() => { + // Check if telemetry is still enabled before attempting to sync + if (!telemetry.enabled) { + return; + } + telemetry.sync().finally(() => { syncTelemetry(telemetry, backoff); });