diff --git a/scripts/bin-test/sources/broken-syntax/index.js b/scripts/bin-test/sources/broken-syntax/index.js new file mode 100644 index 000000000..05cbeaa10 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/scripts/bin-test/sources/broken-syntax/package.json b/scripts/bin-test/sources/broken-syntax/package.json new file mode 100644 index 000000000..bfada79a1 --- /dev/null +++ b/scripts/bin-test/sources/broken-syntax/package.json @@ -0,0 +1,3 @@ +{ + "name": "broken-syntax" +} \ No newline at end of file diff --git a/scripts/bin-test/test.ts b/scripts/bin-test/test.ts index 85b2eb249..c3230b264 100644 --- a/scripts/bin-test/test.ts +++ b/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; + expected: Record; +} + +interface DiscoveryResult { + success: boolean; + manifest?: Record; + error?: string; } async function retryUntil( @@ -134,102 +142,131 @@ async function retryUntil( await Promise.race([retry, timedOut]); } -async function startBin( - tc: Testcase, - debug?: boolean -): Promise<{ port: number; cleanup: () => Promise }> { +async function runHttpDiscovery(modulePath: string): Promise { const getPort = promisify(portfinder.getPort) as () => Promise; 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; + return { success: true, manifest }; + } else { + return { success: false, error: body }; } - return true; - }, TIMEOUT_L); + } finally { + if (proc.pid) { + proc.kill(9); + await new Promise((resolve) => proc.on("exit", resolve)); + } + } +} + +async function runFileDiscovery(modulePath: string): Promise { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "firebase-functions-test-")); + const outputPath = path.join(tempDir, "manifest.json"); - if (debug) { - proc.stdout?.on("data", (data: unknown) => { - console.log(`[${tc.name} stdout] ${data}`); + 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(() => { + proc.kill(9); + 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; + 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; - - 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 + ) { + 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 +357,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 +391,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); + }); + } }); } }); diff --git a/src/bin/firebase-functions.ts b/src/bin/firebase-functions.ts index d04d09fb8..19cee056e 100644 --- a/src/bin/firebase-functions.ts +++ b/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); +}