feat: add file-based function discovery mode (#1711)

* feat: add stdio-based function discovery mode

Adds an alternative discovery mechanism that outputs function manifests via stderr
instead of starting an HTTP server. This improves reliability by avoiding issues
where module loading blocks the HTTP endpoint from becoming available.

When FUNCTIONS_DISCOVERY_MODE=stdio is set:
- Outputs base64-encoded manifest to stderr with __FIREBASE_FUNCTIONS_MANIFEST__: prefix
- Outputs errors to stderr with __FIREBASE_FUNCTIONS_MANIFEST_ERROR__: prefix
- Exits immediately without starting HTTP server
- Maintains backward compatibility (HTTP remains default)

Includes comprehensive tests that verify both HTTP and stdio discovery work
correctly for all test cases (commonjs, esm, various configurations).

* test: add comprehensive tests for stdio discovery

- Add test fixture for broken syntax error handling
- Refactor tests to use unified DiscoveryResult interface
- Parameterize tests with nested loops to test all cases with both discovery methods
- Add error handling tests for both HTTP and stdio discovery
- All stdio discovery tests passing (16/16)

* fix: resolve linting errors in firebase-functions.ts

- Mark runStdioDiscovery() promise with void operator
- Move handleQuitquitquit function to program root
- Fix TypeScript warnings for error handling
- Add eslint-disable comment for Express async handler

* fix: address PR review comments for stdio discovery

- Add proper error handling for non-Error objects
- Define MANIFEST_PREFIX and MANIFEST_ERROR_PREFIX constants
- Fix HTTP discovery retry logic regression in tests
- Make regex patterns multiline-safe for manifest and error parsing
- Add timeout handling for stdio discovery in tests

* fix: resolve linting errors

* fix: clean up broken-syntax fixture to match other fixtures

- Remove unnecessary dependencies from package.json
- Remove package-lock.json
- The npm link is handled by run.sh script

* fix: address additional PR review comments

- Use process.exitCode instead of process.exit() to prevent race conditions with stderr buffer flushing
- Refactor test to define discoveryMethods array once to reduce code duplication

* remove extraneous comments

* fix: resolve prettier formatting issue

* refactor: use XML-style tags for stdio discovery output

- Replace magic string prefixes with self-documenting XML tags
- Add explicit start/end markers to handle OS buffering edge cases
- Make regex parsing more robust and less greedy
- Addresses review comment about regex being too greedy

* fix: resolve prettier formatting for long regex lines

* Update src/bin/firebase-functions.ts

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* linter.

* refactor: replace stdio discovery with file-based manifest output

- Remove XML-style stdio discovery mode
- Add FUNCTIONS_MANIFEST_OUTPUT_PATH environment variable support
- Write manifest directly to specified file path when env var is set
- Update tests to use file-based discovery instead of stdio parsing
- Simplify error handling with direct stderr output

* fix: resolve lint and formatting issues

* feat: add robust path validation and error handling for file output

- Validate output directory exists and is writable before attempting manifest generation
- Provide specific error messages for common failure cases (ENOENT, EACCES)
- Check directory permissions early to fail fast with helpful guidance
- Improve error messages to help users resolve issues quickly

* fix: address PR review comments for test reliability

- Wait for process termination after kill(9) to prevent orphaned processes
- Use fs.mkdtemp() instead of Date.now() for unique temp directories
- Clean up temp directories after tests complete
- Add proper error cleanup for temp directories

* fix: Gracefully kill child processes in tests

Ensures that child processes spawned during tests are properly killed and awaited, preventing orphaned processes in the CI environment.

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Author: taeold

scripts/bin-test/sources/broken-syntax/index.js

@@ -0,0 +1,6 @@
+const functions = require("firebase-functions");
+
+// This will cause a syntax error
+exports.broken = functions.https.onRequest((request, response) => {
+  response.send("Hello from Firebase!"
+}); // Missing closing parenthesis

scripts/bin-test/sources/broken-syntax/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "broken-syntax"
+}

scripts/bin-test/test.ts

@@ -1,6 +1,8 @@
 import * as subprocess from "child_process";
 import * as path from "path";
 import { promisify } from "util";
+import * as fs from "fs/promises";
+import * as os from "os";
 
 import { expect } from "chai";
 import * as yaml from "js-yaml";
@@ -105,7 +107,13 @@ const BASE_STACK = {
 interface Testcase {
   name: string;
   modulePath: string;
-  expected: Record<string, any>;
+  expected: Record<string, unknown>;
+}
+
+interface DiscoveryResult {
+  success: boolean;
+  manifest?: Record<string, unknown>;
+  error?: string;
 }
 
 async function retryUntil(
@@ -134,102 +142,134 @@ async function retryUntil(
   await Promise.race([retry, timedOut]);
 }
 
-async function startBin(
-  tc: Testcase,
-  debug?: boolean
-): Promise<{ port: number; cleanup: () => Promise<void> }> {
+async function runHttpDiscovery(modulePath: string): Promise<DiscoveryResult> {
   const getPort = promisify(portfinder.getPort) as () => Promise<number>;
   const port = await getPort();
 
   const proc = subprocess.spawn("npx", ["firebase-functions"], {
-    cwd: path.resolve(tc.modulePath),
+    cwd: path.resolve(modulePath),
     env: {
       PATH: process.env.PATH,
-      GLCOUD_PROJECT: "test-project",
+      GCLOUD_PROJECT: "test-project",
       PORT: port.toString(),
       FUNCTIONS_CONTROL_API: "true",
     },
   });
-  if (!proc) {
-    throw new Error("Failed to start firebase functions");
-  }
-  proc.stdout?.on("data", (chunk: Buffer) => {
-    console.log(chunk.toString("utf8"));
-  });
-  proc.stderr?.on("data", (chunk: Buffer) => {
-    console.log(chunk.toString("utf8"));
-  });
 
-  await retryUntil(async () => {
-    try {
-      await fetch(`http://localhost:${port}/__/functions.yaml`);
-    } catch (e) {
-      if (e?.code === "ECONNREFUSED") {
-        return false;
+  try {
+    // Wait for server to be ready
+    await retryUntil(async () => {
+      try {
+        await fetch(`http://localhost:${port}/__/functions.yaml`);
+        return true;
+      } catch (e: unknown) {
+        const error = e as { code?: string };
+        if (error.code === "ECONNREFUSED") {
+          // This is an expected error during server startup, so we should retry.
+          return false;
+        }
+        // Any other error is unexpected and should fail the test immediately.
+        throw e;
       }
-      throw e;
+    }, TIMEOUT_L);
+
+    const res = await fetch(`http://localhost:${port}/__/functions.yaml`);
+    const body = await res.text();
+
+    if (res.status === 200) {
+      const manifest = yaml.load(body) as Record<string, unknown>;
+      return { success: true, manifest };
+    } else {
+      return { success: false, error: body };
     }
-    return true;
-  }, TIMEOUT_L);
+  } finally {
+    if (proc.pid) {
+      proc.kill(9);
+      await new Promise<void>((resolve) => proc.on("exit", resolve));
+    }
+  }
+}
 
-  if (debug) {
-    proc.stdout?.on("data", (data: unknown) => {
-      console.log(`[${tc.name} stdout] ${data}`);
+async function runFileDiscovery(modulePath: string): Promise<DiscoveryResult> {
+  const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "firebase-functions-test-"));
+  const outputPath = path.join(tempDir, "manifest.json");
+
+  return new Promise((resolve, reject) => {
+    const proc = subprocess.spawn("npx", ["firebase-functions"], {
+      cwd: path.resolve(modulePath),
+      env: {
+        PATH: process.env.PATH,
+        GCLOUD_PROJECT: "test-project",
+        FUNCTIONS_MANIFEST_OUTPUT_PATH: outputPath,
+      },
     });
 
-    proc.stderr?.on("data", (data: unknown) => {
-      console.log(`[${tc.name} stderr] ${data}`);
+    let stderr = "";
+
+    proc.stderr?.on("data", (chunk: Buffer) => {
+      stderr += chunk.toString("utf8");
     });
-  }
 
-  return {
-    port,
-    cleanup: async () => {
-      process.kill(proc.pid, 9);
-      await retryUntil(async () => {
+    const timeoutId = setTimeout(async () => {
+      if (proc.pid) {
+        proc.kill(9);
+        await new Promise<void>((resolve) => proc.on("exit", resolve));
+      }
+      resolve({ success: false, error: `File discovery timed out after ${TIMEOUT_M}ms` });
+    }, TIMEOUT_M);
+
+    proc.on("close", async (code) => {
+      clearTimeout(timeoutId);
+
+      if (code === 0) {
         try {
-          process.kill(proc.pid, 0);
-        } catch {
-          // process.kill w/ signal 0 will throw an error if the pid no longer exists.
-          return Promise.resolve(true);
+          const manifestJson = await fs.readFile(outputPath, "utf8");
+          const manifest = JSON.parse(manifestJson) as Record<string, unknown>;
+          await fs.rm(tempDir, { recursive: true }).catch(() => {
+            // Ignore errors
+          });
+          resolve({ success: true, manifest });
+        } catch (e) {
+          resolve({ success: false, error: `Failed to read manifest file: ${e}` });
         }
-        return Promise.resolve(false);
-      }, TIMEOUT_L);
-    },
-  };
+      } else {
+        const errorLines = stderr.split("\n").filter((line) => line.trim());
+        const errorMessage = errorLines.join(" ") || "No error message found";
+        resolve({ success: false, error: errorMessage });
+      }
+    });
+
+    proc.on("error", (err) => {
+      clearTimeout(timeoutId);
+      // Clean up temp directory on error
+      fs.rm(tempDir, { recursive: true }).catch(() => {
+        // Ignore errors
+      });
+      reject(err);
+    });
+  });
 }
 
 describe("functions.yaml", function () {
   // eslint-disable-next-line @typescript-eslint/no-invalid-this
   this.timeout(TIMEOUT_XL);
 
-  function runTests(tc: Testcase) {
-    let port: number;
-    let cleanup: () => Promise<void>;
-
-    before(async () => {
-      const r = await startBin(tc);
-      port = r.port;
-      cleanup = r.cleanup;
-    });
+  const discoveryMethods = [
+    { name: "http", fn: runHttpDiscovery },
+    { name: "file", fn: runFileDiscovery },
+  ];
 
-    after(async () => {
-      await cleanup?.();
-    });
-
-    it("functions.yaml returns expected Manifest", async function () {
+  function runDiscoveryTests(
+    tc: Testcase,
+    discoveryFn: (path: string) => Promise<DiscoveryResult>
+  ) {
+    it("returns expected manifest", async function () {
       // eslint-disable-next-line @typescript-eslint/no-invalid-this
       this.timeout(TIMEOUT_M);
 
-      const res = await fetch(`http://localhost:${port}/__/functions.yaml`);
-      const text = await res.text();
-      let parsed: any;
-      try {
-        parsed = yaml.load(text);
-      } catch (err) {
-        throw new Error(`Failed to parse functions.yaml: ${err}`);
-      }
-      expect(parsed).to.be.deep.equal(tc.expected);
+      const result = await discoveryFn(tc.modulePath);
+      expect(result.success).to.be.true;
+      expect(result.manifest).to.deep.equal(tc.expected);
     });
   }
 
@@ -320,7 +360,11 @@ describe("functions.yaml", function () {
 
     for (const tc of testcases) {
       describe(tc.name, () => {
-        runTests(tc);
+        for (const discovery of discoveryMethods) {
+          describe(`${discovery.name} discovery`, () => {
+            runDiscoveryTests(tc, discovery.fn);
+          });
+        }
       });
     }
   });
@@ -350,7 +394,33 @@ describe("functions.yaml", function () {
 
     for (const tc of testcases) {
       describe(tc.name, () => {
-        runTests(tc);
+        for (const discovery of discoveryMethods) {
+          describe(`${discovery.name} discovery`, () => {
+            runDiscoveryTests(tc, discovery.fn);
+          });
+        }
+      });
+    }
+  });
+
+  describe("error handling", () => {
+    const errorTestcases = [
+      {
+        name: "broken syntax",
+        modulePath: "./scripts/bin-test/sources/broken-syntax",
+        expectedError: "missing ) after argument list",
+      },
+    ];
+
+    for (const tc of errorTestcases) {
+      describe(tc.name, () => {
+        for (const discovery of discoveryMethods) {
+          it(`${discovery.name} discovery handles error correctly`, async () => {
+            const result = await discovery.fn(tc.modulePath);
+            expect(result.success).to.be.false;
+            expect(result.error).to.include(tc.expectedError);
+          });
+        }
       });
     }
   });

src/bin/firebase-functions.ts

@@ -24,6 +24,8 @@
 
 import * as http from "http";
 import * as express from "express";
+import * as fs from "fs/promises";
+import * as path from "path";
 import { loadStack } from "../runtime/loader";
 import { stackToWire } from "../runtime/manifest";
 
@@ -49,34 +51,80 @@ if (args.length > 1) {
   functionsDir = args[0];
 }
 
-let server: http.Server = undefined;
-const app = express();
-
-function handleQuitquitquit(req: express.Request, res: express.Response) {
+function handleQuitquitquit(req: express.Request, res: express.Response, server: http.Server) {
   res.send("ok");
   server.close();
 }
 
-app.get("/__/quitquitquit", handleQuitquitquit);
-app.post("/__/quitquitquit", handleQuitquitquit);
-
-if (process.env.FUNCTIONS_CONTROL_API === "true") {
-  app.get("/__/functions.yaml", async (req, res) => {
+if (process.env.FUNCTIONS_MANIFEST_OUTPUT_PATH) {
+  void (async () => {
+    const outputPath = process.env.FUNCTIONS_MANIFEST_OUTPUT_PATH;
     try {
+      // Validate the output path
+      const dir = path.dirname(outputPath);
+      try {
+        await fs.access(dir, fs.constants.W_OK);
+      } catch (e) {
+        console.error(
+          `Error: Cannot write to directory '${dir}': ${e instanceof Error ? e.message : String(e)}`
+        );
+        console.error("Please ensure the directory exists and you have write permissions.");
+        process.exit(1);
+      }
+
       const stack = await loadStack(functionsDir);
-      res.setHeader("content-type", "text/yaml");
-      res.send(JSON.stringify(stackToWire(stack)));
-    } catch (e) {
-      console.error(e);
-      res.status(400).send(`Failed to generate manifest from function source: ${e}`);
+      const wireFormat = stackToWire(stack);
+      await fs.writeFile(outputPath, JSON.stringify(wireFormat, null, 2));
+      process.exit(0);
+    } catch (e: any) {
+      if (e.code === "ENOENT") {
+        console.error(`Error: Directory '${path.dirname(outputPath)}' does not exist.`);
+        console.error("Please create the directory or specify a valid path.");
+      } else if (e.code === "EACCES") {
+        console.error(`Error: Permission denied writing to '${outputPath}'.`);
+        console.error("Please check file permissions or choose a different location.");
+      } else if (e.message?.includes("Failed to generate manifest")) {
+        console.error(e.message);
+      } else {
+        console.error(
+          `Failed to generate manifest from function source: ${
+            e instanceof Error ? e.message : String(e)
+          }`
+        );
+      }
+      if (e instanceof Error && e.stack) {
+        console.error(e.stack);
+      }
+      process.exit(1);
     }
-  });
-}
+  })();
+} else {
+  let server: http.Server = undefined;
+  const app = express();
 
-let port = 8080;
-if (process.env.PORT) {
-  port = Number.parseInt(process.env.PORT);
-}
+  app.get("/__/quitquitquit", (req, res) => handleQuitquitquit(req, res, server));
+  app.post("/__/quitquitquit", (req, res) => handleQuitquitquit(req, res, server));
 
-console.log("Serving at port", port);
-server = app.listen(port);
+  if (process.env.FUNCTIONS_CONTROL_API === "true") {
+    // eslint-disable-next-line @typescript-eslint/no-misused-promises
+    app.get("/__/functions.yaml", async (req, res) => {
+      try {
+        const stack = await loadStack(functionsDir);
+        res.setHeader("content-type", "text/yaml");
+        res.send(JSON.stringify(stackToWire(stack)));
+      } catch (e) {
+        console.error(e);
+        const errorMessage = e instanceof Error ? e.message : String(e);
+        res.status(400).send(`Failed to generate manifest from function source: ${errorMessage}`);
+      }
+    });
+  }
+
+  let port = 8080;
+  if (process.env.PORT) {
+    port = Number.parseInt(process.env.PORT);
+  }
+
+  console.log("Serving at port", port);
+  server = app.listen(port);
+}