Skip to content

feat: add stdio-based function discovery mode #1711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions scripts/bin-test/sources/broken-syntax/index.js
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions scripts/bin-test/sources/broken-syntax/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "broken-syntax"
}
197 changes: 128 additions & 69 deletions scripts/bin-test/test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -134,102 +142,123 @@ 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 {
proc.kill(9);
}
}

if (debug) {
proc.stdout?.on("data", (data: unknown) => {
console.log(`[${tc.name} stdout] ${data}`);
async function runFileDiscovery(modulePath: string): Promise<DiscoveryResult> {
const outputPath = path.join(os.tmpdir(), `firebase-functions-test-${Date.now()}.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(() => {
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<string, unknown>;
await fs.unlink(outputPath).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);
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>;
const discoveryMethods = [
{ name: "http", fn: runHttpDiscovery },
{ name: "file", fn: runFileDiscovery },
];

before(async () => {
const r = await startBin(tc);
port = r.port;
cleanup = r.cleanup;
});

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);
});
}

Expand Down Expand Up @@ -320,7 +349,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);
});
}
});
}
});
Expand Down Expand Up @@ -350,7 +383,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);
});
}
});
}
});
Expand Down
69 changes: 48 additions & 21 deletions src/bin/firebase-functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import * as http from "http";
import * as express from "express";
import * as fs from "fs/promises";
import { loadStack } from "../runtime/loader";
import { stackToWire } from "../runtime/manifest";

Expand All @@ -49,34 +50,60 @@ 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 () => {
try {
const stack = await loadStack(functionsDir);
res.setHeader("content-type", "text/yaml");
res.send(JSON.stringify(stackToWire(stack)));
const wireFormat = stackToWire(stack);
await fs.writeFile(
process.env.FUNCTIONS_MANIFEST_OUTPUT_PATH,
JSON.stringify(wireFormat, null, 2)
);
process.exit(0);
} catch (e) {
console.error(e);
res.status(400).send(`Failed to generate manifest from function source: ${e}`);
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);
}
Loading