fix signal handling (#1460)

* fix signal handling

* fix kill test

Author: mbostock

src/telemetry.ts

@@ -97,9 +97,9 @@ export class Telemetry {
     this.disabled = !!process.env.OBSERVABLE_TELEMETRY_DISABLE;
     this.debug = !!process.env.OBSERVABLE_TELEMETRY_DEBUG;
     this.endpoint = new URL("/cli", getOrigin(process.env));
-    process.on("SIGHUP", this.handleSignal(1));
-    process.on("SIGINT", this.handleSignal(2));
-    process.on("SIGTERM", this.handleSignal(15));
+    this.handleSignal("SIGHUP");
+    this.handleSignal("SIGINT");
+    this.handleSignal("SIGTERM");
   }
 
   record(data: TelemetryData) {
@@ -122,17 +122,23 @@ export class Telemetry {
     return Promise.all(this._pending);
   }
 
-  private handleSignal(value: number) {
-    const code = 128 + value;
-    return async (signal: NodeJS.Signals) => {
-      const {process} = this.effects;
-      // Give ourselves 1s to record a signal event and flush.
-      const deadline = setTimeout(() => process.exit(code), 1000);
+  private handleSignal(name: string) {
+    const {process} = this.effects;
+    let exiting = false;
+    const signaled = async (signal: NodeJS.Signals) => {
+      if (exiting) return; // already exiting
+      exiting = true;
       this.record({event: "signal", signal});
-      await this.pending;
-      clearTimeout(deadline);
-      process.exit(code);
+      try {
+        // Allow one second to record a signal event and flush.
+        await Promise.race([this.pending, new Promise((resolve) => setTimeout(resolve, 1000))]);
+      } catch {
+        // ignore error
+      }
+      process.off(name, signaled); // don’t handle our own kill
+      process.kill(process.pid, signal);
     };
+    process.on(name, signaled);
   }
 
   private async getPersistentId(name: string, generator = randomUUID) {

test/telemetry-test.ts

@@ -104,8 +104,8 @@ describe("telemetry", () => {
   it("saves a signal record on exit", async () => {
     const logger = new MockLogger();
     const listeners = {};
-    let exit: (value: unknown) => void;
-    const exited = new Promise((resolve) => (exit = resolve));
+    let kill: (value: unknown) => void;
+    const killed = new Promise((resolve) => (kill = resolve));
     new Telemetry({
       ...noopEffects,
       logger,
@@ -115,14 +115,14 @@ describe("telemetry", () => {
           listeners[event] = listener;
           return this;
         },
-        exit(code?: number) {
-          exit(code);
+        kill(pid: number, signal?: string) {
+          kill(signal);
           throw new Error("exit");
         }
       })
     });
     listeners["SIGINT"]("SIGINT");
-    assert.equal(await exited, 130);
+    assert.equal(await killed, "SIGINT");
     assert.equal(logger.errorLines.length, 1);
     assert.deepEqual(logger.errorLines[0][1].data, {event: "signal", signal: "SIGINT"});
   });