Remove configurable spawn (#77)

* Remove configurable spawn function

* Expand on changeset

* Biome fix

* Remove type export

Author: tmcw

.changeset/good-hats-show.md

@@ -0,0 +1,9 @@
+---
+"@valtown/deno-http-worker": major
+---
+
+Remove configurable spawn function option
+
+Previously, we supported a `spawnFunc` option which let you use spawn methods
+other than child_process.spawn. Given the lack of useful alternatives to child_process.spawn
+and our efforts to really optimize this module, we're removing this option.

src/DenoHTTPWorker.test.ts

@@ -3,7 +3,6 @@ import { type DenoHTTPWorker, newDenoHTTPWorker } from "./index.js";
 import fs from "node:fs";
 import path from "node:path";
 import { Worker } from "node:worker_threads";
-import { type SpawnOptions, spawn } from "node:child_process";
 import { EarlyExitDenoHTTPWorkerError } from "./DenoHTTPWorker.js";
 
 // Uncomment this if you want to debug serial test execution
@@ -73,29 +72,6 @@ describe("DenoHTTPWorker", { timeout: 1000 }, () => {
     await worker.terminate();
   });
 
-  it("alternate spawnFunc can be provided", async () => {
-    let firstArg = "";
-    const worker = await newDenoHTTPWorker(
-      `
-        export default { async fetch (req: Request): Promise<Response> {
-          let headers = {};
-          for (let [key, value] of req.headers.entries()) {
-            headers[key] = value;
-          }
-          return Response.json({ ok: req.url, headers: headers })
-        } }
-      `,
-      {
-        spawnFunc: (command: string, args: string[], options: SpawnOptions) => {
-          firstArg = args[0] as string;
-          return spawn(command, args, options);
-        },
-      }
-    );
-    expect(firstArg).toEqual("run");
-    await worker.terminate();
-  });
-
   it("don't crash on socket removal", async () => {
     const worker = await newDenoHTTPWorker(
       `

src/DenoHTTPWorker.ts

@@ -1,5 +1,9 @@
 import path, { resolve } from "node:path";
-import { spawn, type SpawnOptions } from "node:child_process";
+import {
+  type ChildProcess,
+  spawn,
+  type SpawnOptions,
+} from "node:child_process";
 import type { Readable } from "node:stream";
 import readline from "node:readline";
 import fs from "node:fs/promises";
@@ -31,26 +35,6 @@ export class EarlyExitDenoHTTPWorkerError extends Error {
   }
 }
 
-export interface MinimalChildProcess {
-  stdout: Readable | null;
-  stderr: Readable | null;
-  readonly pid?: number | undefined;
-  readonly exitCode: number | null;
-  kill(signal?: NodeJS.Signals | number): boolean;
-  on(event: string, listener: (...args: any[]) => void): this;
-  on(
-    event: "close",
-    listener: (code: number | null, signal: NodeJS.Signals | null) => void
-  ): this;
-  on(event: "disconnect", listener: () => void): this;
-  on(event: "error", listener: (err: Error) => void): this;
-  on(
-    event: "exit",
-    listener: (code: number | null, signal: NodeJS.Signals | null) => void
-  ): this;
-  on(event: "spawn", listener: () => void): this;
-}
-
 export interface DenoWorkerOptions {
   /**
    * The path to the executable that should be use when spawning the subprocess.
@@ -97,16 +81,7 @@ export interface DenoWorkerOptions {
   /**
    * Callback that is called when the process is spawned.
    */
-  onSpawn?: (process: MinimalChildProcess) => void;
-
-  /**
-   * Provide an alternative spawn functions. Defaults to child_process.spawn.
-   */
-  spawnFunc: (
-    command: string,
-    args: string[],
-    options: SpawnOptions
-  ) => MinimalChildProcess;
+  onSpawn?: (process: ChildProcess) => void;
 }
 
 /**
@@ -123,7 +98,6 @@ export const newDenoHTTPWorker = async (
     printCommandAndArguments: false,
     spawnOptions: {},
     printOutput: false,
-    spawnFunc: spawn,
     ...options,
   };
 
@@ -203,7 +177,7 @@ export const newDenoHTTPWorker = async (
         console.log("Spawning deno process:", [command, ...args]);
       }
 
-      const process = _options.spawnFunc(command, args, _options.spawnOptions);
+      const process = spawn(command, args, _options.spawnOptions);
       let running = false;
       let exited = false;
       // eslint-disable-next-line prefer-const
@@ -305,7 +279,7 @@ export interface DenoHTTPWorker {
 
 class denoHTTPWorker implements DenoHTTPWorker {
   #onexitListeners: OnExitListener[];
-  #process: MinimalChildProcess;
+  #process: ChildProcess;
   #socketFile: string;
   #stderr: Readable;
   #stdout: Readable;
@@ -314,7 +288,7 @@ class denoHTTPWorker implements DenoHTTPWorker {
 
   constructor(
     socketFile: string,
-    process: MinimalChildProcess,
+    process: ChildProcess,
     stdout: Readable,
     stderr: Readable
   ) {
@@ -327,6 +301,9 @@ class denoHTTPWorker implements DenoHTTPWorker {
     this.#pool = new undici.Pool("http://deno", { socketPath: socketFile });
   }
 
+  /**
+   * Force-kill the process with SIGKILL, firing exit events.
+   */
   async _terminate(code?: number, signal?: string) {
     if (this.#terminated) {
       return;
@@ -347,6 +324,11 @@ class denoHTTPWorker implements DenoHTTPWorker {
     return await this._terminate();
   }
 
+  /**
+   * Kill the process with SIGINT.
+   * This resolves once we receive the 'exit' event from the underlying
+   * process.
+   */
   async shutdown() {
     this.#process.kill("SIGINT");
     await new Promise<void>((res) => {
@@ -358,7 +340,7 @@ class denoHTTPWorker implements DenoHTTPWorker {
     url: string,
     headers: Headers = new Headers()
   ): Promise<WebSocket> {
-    headers = this.#processHeaders(headers, url);
+    headers = processHeaders(headers, url);
     headers.set("x-deno-worker-connection", "upgrade"); // Required for websockets
 
     return new WebSocket(url, {
@@ -368,7 +350,7 @@ class denoHTTPWorker implements DenoHTTPWorker {
   }
 
   async request(options: RequestOptions): Promise<ResponseData> {
-    const headers = this.#processHeaders(
+    const headers = processHeaders(
       options.headers || new Headers(),
       options.url
     );
@@ -390,27 +372,6 @@ class denoHTTPWorker implements DenoHTTPWorker {
       }));
   }
 
-  #processHeaders(headers: Headers, url: string): Headers {
-    headers.set("x-deno-worker-url", url);
-
-    // To prevent the user from setting these headers, we either update them to
-    // the real host / connection, or clear them
-    headers.delete("x-deno-worker-host");
-    headers.delete("x-deno-worker-connection");
-
-    const host = headers.get("host");
-    if (host) {
-      headers.set("x-deno-worker-host", host);
-    }
-
-    const connection = headers.get("connection");
-    if (connection) {
-      headers.set("x-deno-worker-connection", connection);
-    }
-
-    return headers;
-  }
-
   // We send this request to Deno so that we get a live connection in the
   // http.Agent and subsequent requests are do not have to wait for a new
   // connection.
@@ -457,3 +418,24 @@ function killUnix(pid: number) {
     }
   }
 }
+
+function processHeaders(headers: Headers, url: string): Headers {
+  headers.set("x-deno-worker-url", url);
+
+  // To prevent the user from setting these headers, we either update them to
+  // the real host / connection, or clear them
+  headers.delete("x-deno-worker-host");
+  headers.delete("x-deno-worker-connection");
+
+  const host = headers.get("host");
+  if (host) {
+    headers.set("x-deno-worker-host", host);
+  }
+
+  const connection = headers.get("connection");
+  if (connection) {
+    headers.set("x-deno-worker-connection", connection);
+  }
+
+  return headers;
+}

src/index.ts

@@ -1,7 +1,6 @@
 export {
   DenoHTTPWorker,
   DenoWorkerOptions,
-  MinimalChildProcess,
   newDenoHTTPWorker,
 } from "./DenoHTTPWorker.js";