diff --git a/.eslintrc.js b/.eslintrc.js index c3b81a2e0d9..55afdaf558b 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -53,7 +53,6 @@ module.exports = { "@typescript-eslint/explicit-function-return-type": ["warn", { allowExpressions: true }], // TODO(bkendall): SET to error. "@typescript-eslint/no-floating-promises": "warn", // TODO(bkendall): remove, allow to error. - "@typescript-eslint/no-misused-promises": "warn", // TODO(bkendall): remove, allow to error. "@typescript-eslint/no-unsafe-argument": "warn", // TODO(bkendall): remove, allow to error. "@typescript-eslint/no-unsafe-assignment": "warn", // TODO(bkendall): remove, allow to error. "@typescript-eslint/no-unsafe-call": "warn", // TODO(bkendall): remove, allow to error. diff --git a/CHANGELOG.md b/CHANGELOG.md index 086d2291470..c5e127a8fea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,3 +4,5 @@ - Fixed the emulators startup message showing 127.0.0.1 for the Emulator UI and Hub when they listen on 0.0.0.0; it now also prints the address they listen on. (#6222) - Fixed a race condition across a second boundary during `accounts:signUp` in the Auth emulator where `validSince` could exceed `auth_time`. (#11413) - Added local machine config persistence to `appcheck:debugtokens:create`, `--local` support to `appcheck:debugtokens:list`, and `appcheck:debugtokens:prune` to clean up stale local debug tokens. +- Fixed `database:import` failing with an unhandled promise rejection when a batch write fails. +- Fixed emulator crashes when a Firestore rules reload or emulator HTTP handler throws. diff --git a/src/auth.ts b/src/auth.ts index cd62c40fc3b..e7d2b3d3807 100644 --- a/src/auth.ts +++ b/src/auth.ts @@ -605,7 +605,17 @@ async function loginWithLocalhost( getTokens: (queryCode: string, callbackUrl: string) => Promise, ): Promise { return new Promise((resolve, reject) => { - const server = http.createServer(async (req, res) => { + const server = http.createServer((req, res) => { + handleCallback(req, res).catch((err: unknown) => { + reject(err); + server.close(); + }); + }); + + async function handleCallback( + req: http.IncomingMessage, + res: http.ServerResponse, + ): Promise { let searchParams: URLSearchParams; try { searchParams = new URL(req.url || "", callbackUrl).searchParams; @@ -634,7 +644,7 @@ async function loginWithLocalhost( } server.close(); return; - }); + } server.listen(port, () => { logger.info(); diff --git a/src/database/import.spec.ts b/src/database/import.spec.ts index b0534dbabad..6a14fdb56e7 100644 --- a/src/database/import.spec.ts +++ b/src/database/import.spec.ts @@ -217,4 +217,11 @@ describe("DatabaseImporter", () => { expect(responses).to.have.length(1); expect(nock.isDone()).to.be.true; }); + + it("rejects with the write error when a batch write fails", async () => { + nock("https://test-db.firebaseio.com").get("/foo.json?shallow=true").reply(200); + nock("https://test-db.firebaseio.com").patch("/foo.json").reply(400, { error: "bad" }); + const importer = new DatabaseImporter(dbUrl, DATA_STREAM, "/", payloadSize, concurrencyLimit); + await expect(importer.execute()).to.be.rejectedWith(FirebaseError, /400/); + }); }); diff --git a/src/database/import.ts b/src/database/import.ts index 01d848a9e3d..f6284624734 100644 --- a/src/database/import.ts +++ b/src/database/import.ts @@ -186,10 +186,14 @@ export default class DatabaseImporter { }; const writeBatch = new stream.Transform({ objectMode: true }); - writeBatch._transform = async function (batch: SizedData, _, done) { - const res = await doWriteBatch(batch); - this.push(res); - done(); + writeBatch._transform = function (batch: SizedData, _, done) { + void doWriteBatch(batch).then( + (res) => { + this.push(res); + done(); + }, + (err: Error) => done(err), + ); }; return new Promise((resolve, reject) => { diff --git a/src/deploy/functions/runtimes/discovery/index.ts b/src/deploy/functions/runtimes/discovery/index.ts index 28f1380b83a..9d2e665383b 100644 --- a/src/deploy/functions/runtimes/discovery/index.ts +++ b/src/deploy/functions/runtimes/discovery/index.ts @@ -162,7 +162,7 @@ export async function detectFromOutputPath( stderrBuffer += chunk.toString(); }); - childProcess.on("exit", async (code: number | null) => { + const onExit = async (code: number | null): Promise => { if (!resolved) { clearTimeout(timer); resolved = true; @@ -192,6 +192,11 @@ export async function detectFromOutputPath( } } } + }; + + // onExit catches and routes all outcomes and errors to resolve or reject + childProcess.on("exit", (code: number | null) => { + void onExit(code); }); childProcess.on("error", (err: Error) => { diff --git a/src/deploy/functions/services/index.ts b/src/deploy/functions/services/index.ts index c5ba4ff2f92..fa16427f11e 100644 --- a/src/deploy/functions/services/index.ts +++ b/src/deploy/functions/services/index.ts @@ -29,6 +29,9 @@ import { ensureAuthEventarcTriggerRegion } from "./authEventarc"; /** A standard void No Op */ export const noop = (): Promise => Promise.resolve(); +/** A synchronous No Op, for hooks such as validateTrigger that return void */ +export const syncNoop = (): void => undefined; + /** A No Op that's useful for Services that don't have specific bindings but should still try to set default bindings */ export const noopProjectBindings = (): Promise> => Promise.resolve([]); @@ -73,7 +76,7 @@ const noOpService: Service = { name: "noop", api: "", ensureTriggerRegion: noop, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: () => Promise.resolve(FALLBACK_DEPLOYMENT_REGION), @@ -85,7 +88,7 @@ const pubSubService: Service = { api: "pubsub.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: noop, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION), @@ -97,7 +100,7 @@ const storageService: Service = { api: "storage.googleapis.com", requiredProjectBindings: obtainStorageBindings, ensureTriggerRegion: ensureStorageTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: ensureStorageDefaultRegion, @@ -109,7 +112,7 @@ const firebaseAlertsService: Service = { api: "firebasealerts.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureFirebaseAlertsTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION), @@ -125,7 +128,7 @@ const databaseService: Service = { api: "firebasedatabase.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureDatabaseTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: ensureDatabaseDefaultRegion, @@ -137,7 +140,7 @@ const remoteConfigService: Service = { api: "firebaseremoteconfig.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureRemoteConfigTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION), @@ -149,7 +152,7 @@ const testLabService: Service = { api: "testing.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureTestLabTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION), @@ -161,7 +164,7 @@ const firestoreService: Service = { api: "firestore.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureFirestoreTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: ensureFirestoreDefaultRegion, @@ -173,7 +176,7 @@ const dataconnectService: Service = { api: "firebasedataconnect.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureDataConnectTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: ensureDataConnectDefaultRegion, @@ -185,7 +188,7 @@ const authEventarcService: Service = { api: "eventarc.googleapis.com", requiredProjectBindings: noopProjectBindings, ensureTriggerRegion: ensureAuthEventarcTriggerRegion, - validateTrigger: noop, + validateTrigger: syncNoop, registerTrigger: noop, unregisterTrigger: noop, getDefaultRegion: () => Promise.resolve(DEFAULT_GLOBAL_TRIGGER_REGION), diff --git a/src/emulator/commandUtils.ts b/src/emulator/commandUtils.ts index 9e3a8fd26ad..6d2561ba00f 100644 --- a/src/emulator/commandUtils.ts +++ b/src/emulator/commandUtils.ts @@ -265,7 +265,7 @@ function processKillSignal( ): SignalsListener { let lastSignal = new Date().getTime(); let signalCount = 0; - return async () => { + const handleSignal = async (): Promise => { try { const now = new Date().getTime(); const diff = now - lastSignal; @@ -339,6 +339,11 @@ function processKillSignal( rej(getError(e)); } }; + + // handleSignal catches all errors and routes them to res/rej + return () => { + void handleSignal(); + }; } /** diff --git a/src/emulator/databaseEmulator.ts b/src/emulator/databaseEmulator.ts index 31641a14fcc..cf5ec8e1a53 100644 --- a/src/emulator/databaseEmulator.ts +++ b/src/emulator/databaseEmulator.ts @@ -9,7 +9,7 @@ import { EmulatorInfo, EmulatorInstance, Emulators } from "../emulator/types"; import { Constants } from "./constants"; import { EmulatorRegistry } from "./registry"; import { EmulatorLogger } from "./emulatorLogger"; -import { FirebaseError } from "../error"; +import { FirebaseError, getErrMsg } from "../error"; import { connectableHostname } from "../utils"; export interface DatabaseEmulatorArgs { @@ -50,25 +50,30 @@ export class DatabaseEmulator implements EmulatorInstance { const watcher = chokidar.watch(c.rules, { persistent: true, ignoreInitial: true }); this.rulesWatchers.push(watcher); - watcher.on("change", async () => { - // There have been some race conditions reported (on Windows) where reading the - // file too quickly after the watcher fires results in an empty file being read. - // Adding a small delay prevents that at very little cost. - await new Promise((res) => setTimeout(res, 5)); + watcher.on("change", () => { + const update = async (): Promise => { + // There have been some race conditions reported (on Windows) where reading the + // file too quickly after the watcher fires results in an empty file being read. + // Adding a small delay prevents that at very little cost. + await new Promise((res) => setTimeout(res, 5)); + + this.logger.logLabeled( + "BULLET", + "database", + `Change detected, updating rules for ${c.instance}...`, + ); - this.logger.logLabeled( - "BULLET", - "database", - `Change detected, updating rules for ${c.instance}...`, - ); - - try { - await this.updateRules(c.instance, c.rules); - this.logger.logLabeled("SUCCESS", "database", "Rules updated."); - } catch (e: any) { - this.logger.logLabeled("WARN", "database", this.prettyPrintRulesError(c.rules, e)); - this.logger.logLabeled("WARN", "database", "Failed to update rules"); - } + try { + await this.updateRules(c.instance, c.rules); + this.logger.logLabeled("SUCCESS", "database", "Rules updated."); + } catch (e: any) { + this.logger.logLabeled("WARN", "database", this.prettyPrintRulesError(c.rules, e)); + this.logger.logLabeled("WARN", "database", "Failed to update rules"); + } + }; + update().catch((err: unknown) => { + this.logger.logLabeled("WARN", "database", `Failed to update rules: ${getErrMsg(err)}`); + }); }); } } diff --git a/src/emulator/dataconnect/pgliteServer.ts b/src/emulator/dataconnect/pgliteServer.ts index 8225370397f..883beaf9e84 100644 --- a/src/emulator/dataconnect/pgliteServer.ts +++ b/src/emulator/dataconnect/pgliteServer.ts @@ -49,43 +49,49 @@ export class PostgresServer { public async createPGServer(host = "127.0.0.1", port: number): Promise { const getDb = this.getDb.bind(this); - const server = net.createServer(async (socket) => { - const connection = await fromNodeSocket(socket, { - serverVersion: "17.4 (PGlite 0.3.3)", - auth: { method: "trust" }, - - async *onMessage(data: Uint8Array, { isAuthenticated }: { isAuthenticated: boolean }) { - // Only forward messages to PGlite after authentication - if (!isAuthenticated) { - return; - } - const db = await getDb(); - if (data[0] === FrontendMessageCode.Terminate) { - // When the frontend terminates a connection, throw out all prepared statements - // because the next client won't know about them (and may create overlapping statements) - await db.query("DEALLOCATE ALL"); - } - const response = await db.execProtocolRaw(data); - for await (const message of extendedQueryPatch.getMessages(data, response)) { - yield message; - } - - // Extended query patch removes the extra Ready for Query messages that - // pglite wrongly sends. - }, - }); - - const extendedQueryPatch: PGliteLoggerPatch = new PGliteLoggerPatch(connection); - socket.on("end", () => { - logger.debug("Postgres client disconnected"); - }); - // Log and destroy individual client socket errors instead of escalating them - // to server-level errors. - socket.on("error", (err) => { - logger.debug( - "Postgres client socket error (if this happened during request cancellation, it is expected and harmless):", - err, - ); + const server = net.createServer((socket) => { + const handleConnection = async (): Promise => { + const connection = await fromNodeSocket(socket, { + serverVersion: "17.4 (PGlite 0.3.3)", + auth: { method: "trust" }, + + async *onMessage(data: Uint8Array, { isAuthenticated }: { isAuthenticated: boolean }) { + // Only forward messages to PGlite after authentication + if (!isAuthenticated) { + return; + } + const db = await getDb(); + if (data[0] === FrontendMessageCode.Terminate) { + // When the frontend terminates a connection, throw out all prepared statements + // because the next client won't know about them (and may create overlapping statements) + await db.query("DEALLOCATE ALL"); + } + const response = await db.execProtocolRaw(data); + for await (const message of extendedQueryPatch.getMessages(data, response)) { + yield message; + } + + // Extended query patch removes the extra Ready for Query messages that + // pglite wrongly sends. + }, + }); + + const extendedQueryPatch: PGliteLoggerPatch = new PGliteLoggerPatch(connection); + socket.on("end", () => { + logger.debug("Postgres client disconnected"); + }); + // Log and destroy individual client socket errors instead of escalating them + // to server-level errors. + socket.on("error", (err) => { + logger.debug( + "Postgres client socket error (if this happened during request cancellation, it is expected and harmless):", + err, + ); + socket.destroy(); + }); + }; + handleConnection().catch((err: unknown) => { + logger.debug("Postgres client connection setup failed:", err); socket.destroy(); }); }); diff --git a/src/emulator/downloadableEmulators.ts b/src/emulator/downloadableEmulators.ts index d6075b3e34e..9b3758c91a7 100755 --- a/src/emulator/downloadableEmulators.ts +++ b/src/emulator/downloadableEmulators.ts @@ -558,11 +558,12 @@ async function _runBinary( void handleEmulatorProcessError(emulator.name, err, command.port); }); - emulator.instance.once("exit", async (code, signal) => { + emulator.instance.once("exit", (code, signal) => { if (signal) { utils.logWarning(`${description} has exited upon receiving signal: ${signal}`); } else if (code && code !== 0 && code !== /* SIGINT */ 130) { - await _fatal(emulator.name, `${description} has exited with code: ${code}`); + // _fatal never rejects because its finally block calls process.exit(1) + void _fatal(emulator.name, `${description} has exited with code: ${code}`); } }); resolve(); diff --git a/src/emulator/firestoreEmulator.ts b/src/emulator/firestoreEmulator.ts index 3c9e944851d..99fa1e55957 100644 --- a/src/emulator/firestoreEmulator.ts +++ b/src/emulator/firestoreEmulator.ts @@ -4,6 +4,7 @@ import * as clc from "colorette"; import * as path from "path"; import * as utils from "../utils"; +import { getErrMsg } from "../error"; import * as downloadableEmulators from "./downloadableEmulators"; import { EmulatorInfo, EmulatorInstance, Emulators, Severity } from "../emulator/types"; import { EmulatorRegistry } from "./registry"; @@ -55,25 +56,30 @@ export class FirestoreEmulator implements EmulatorInstance { "If your environment does not support file watching, set CHOKIDAR_USEPOLLING=1 to fall back to polling.", ); }); - this.rulesWatcher.on("change", async () => { - // There have been some race conditions reported (on Windows) where reading the - // file too quickly after the watcher fires results in an empty file being read. - // Adding a small delay prevents that at very little cost. - await new Promise((res) => setTimeout(res, 5)); - - utils.logLabeledBullet("firestore", "Change detected, updating rules..."); - const newContent = fs.readFileSync(rulesPath, "utf8").toString(); - const issues = await this.updateRules(newContent); - if (issues) { - for (const issue of issues) { - utils.logWarning(this.prettyPrintRulesIssue(rulesPath, issue)); + this.rulesWatcher.on("change", () => { + const update = async (): Promise => { + // There have been some race conditions reported (on Windows) where reading the + // file too quickly after the watcher fires results in an empty file being read. + // Adding a small delay prevents that at very little cost. + await new Promise((res) => setTimeout(res, 5)); + + utils.logLabeledBullet("firestore", "Change detected, updating rules..."); + const newContent = fs.readFileSync(rulesPath, "utf8").toString(); + const issues = await this.updateRules(newContent); + if (issues) { + for (const issue of issues) { + utils.logWarning(this.prettyPrintRulesIssue(rulesPath, issue)); + } } - } - if (issues.some((issue) => issue.severity === Severity.ERROR)) { - utils.logWarning("Failed to update rules"); - } else { - utils.logLabeledSuccess("firestore", "Rules updated."); - } + if (issues.some((issue) => issue.severity === Severity.ERROR)) { + utils.logWarning("Failed to update rules"); + } else { + utils.logLabeledSuccess("firestore", "Rules updated."); + } + }; + update().catch((err: unknown) => { + utils.logWarning(`Failed to update rules: ${getErrMsg(err)}`); + }); }); } diff --git a/src/emulator/functionsEmulator.ts b/src/emulator/functionsEmulator.ts index 24d6fb8a930..0a7268ba07f 100644 --- a/src/emulator/functionsEmulator.ts +++ b/src/emulator/functionsEmulator.ts @@ -42,7 +42,7 @@ import { EmulatorRegistry } from "./registry"; import { EmulatorLogger, Verbosity } from "./emulatorLogger"; import { RuntimeWorker, RuntimeWorkerPool } from "./functionsRuntimeWorker"; import { PubsubEmulator } from "./pubsubEmulator"; -import { FirebaseError } from "../error"; +import { FirebaseError, getErrMsg } from "../error"; import { WorkQueue, Work } from "./workQueue"; import { allSettled, connectableHostname, createDestroyer, debounce, randomInt } from "../utils"; import { @@ -534,7 +534,15 @@ export class FunctionsEmulator implements EmulatorInstance { "functions", `Starting build_runner watch for Dart functions...`, ); - const debouncedLoadTriggers = debounce(() => this.loadTriggers(backend), 1000); + const debouncedLoadTriggers = debounce(() => { + this.loadTriggers(backend).catch((err: unknown) => + this.logger.logLabeled( + "WARN", + "functions", + `Failed to reload triggers: ${getErrMsg(err)}`, + ), + ); + }, 1000); const cleanup = await delegate.watch(() => { this.logger.log("DEBUG", "build_runner rebuilt, reloading triggers"); debouncedLoadTriggers(); @@ -549,10 +557,18 @@ export class FunctionsEmulator implements EmulatorInstance { this.watchers.push(watcher); - const debouncedLoadTriggers = debounce(() => this.loadTriggers(backend), 1000); + const debouncedLoadTriggers = debounce(() => { + this.loadTriggers(backend).catch((err: unknown) => + this.logger.logLabeled( + "WARN", + "functions", + `Failed to reload triggers: ${getErrMsg(err)}`, + ), + ); + }, 1000); watcher.on("change", (filePath) => { this.logger.log("DEBUG", `File ${filePath} changed, reloading triggers`); - return debouncedLoadTriggers(); + debouncedLoadTriggers(); }); } } diff --git a/src/emulator/functionsEmulatorRuntime.ts b/src/emulator/functionsEmulatorRuntime.ts index c563937cd68..a70b2b5b61a 100644 --- a/src/emulator/functionsEmulatorRuntime.ts +++ b/src/emulator/functionsEmulatorRuntime.ts @@ -869,7 +869,7 @@ async function main(): Promise { app.all("/favicon.ico|/robots.txt", (req, res) => { res.status(404).send(); }); - app.all(`/*`, async (req: express.Request, res: express.Response) => { + async function handleRequest(req: express.Request, res: express.Response): Promise { try { const trigger = FUNCTION_TARGET_NAME.split(".").reduce((mod, functionTargetPart) => { return mod?.[functionTargetPart]; @@ -898,8 +898,15 @@ async function main(): Promise { } } catch (err: unknown) { new EmulatorLog("FATAL", "runtime-error", getErrStack(err)).log(); - res.status(500).send(getErrMsg(err)); + if (!res.headersSent) { + res.status(500).send(getErrMsg(err)); + } } + } + + // handleRequest catches all errors and responds with status 500 + app.all(`/*`, (req: express.Request, res: express.Response) => { + void handleRequest(req, res); }); app.listen(process.env.PORT, () => { logDebug(`Listening to port: ${process.env.PORT}`); diff --git a/src/emulator/hub.spec.ts b/src/emulator/hub.spec.ts new file mode 100644 index 00000000000..493a05df176 --- /dev/null +++ b/src/emulator/hub.spec.ts @@ -0,0 +1,57 @@ +import { expect } from "chai"; +import type { Express } from "express"; +import * as sinon from "sinon"; +import * as supertest from "supertest"; +import { EmulatorHub, EmulatorHubArgs } from "./hub"; +import { HubExport } from "./hubExport"; +import { EmulatorRegistry } from "./registry"; + +describe("EmulatorHub", () => { + let app: Express; + let exportAll: sinon.SinonStub; + let registryGet: sinon.SinonStub; + + beforeEach(async () => { + exportAll = sinon.stub(HubExport.prototype, "exportAll").resolves(); + registryGet = sinon.stub(EmulatorRegistry, "get").returns(undefined); + const hub = new EmulatorHub({ + projectId: "test-project", + listen: [{ address: "127.0.0.1", port: 4400, family: "IPv4" }], + listenForEmulator: {} as EmulatorHubArgs["listenForEmulator"], + }); + app = await hub["createExpressApp"](); + }); + + afterEach(() => { + sinon.restore(); + }); + + describe("PATH_EXPORT", () => { + it("should reject external requests with 403 without exporting", async () => { + const res = await supertest(app) + .post(EmulatorHub.PATH_EXPORT) + .set("origin", "http://malicious.example.com") + .send({ path: "/tmp/export" }); + + expect(res.status).to.equal(403); + expect(res.body).to.deep.equal({ + message: "Export cannot be triggered by external callers.", + }); + expect(exportAll).to.not.have.been.called; + }); + }); + + describe("PATH_CLEAR_DATA_CONNECT", () => { + it("should reject external requests with 403 without clearing data", async () => { + const res = await supertest(app) + .post(EmulatorHub.PATH_CLEAR_DATA_CONNECT) + .set("origin", "http://malicious.example.com"); + + expect(res.status).to.equal(403); + expect(res.body).to.deep.equal({ + message: "Clear SQL Connect cannot be triggered by external callers.", + }); + expect(registryGet).to.not.have.been.called; + }); + }); +}); diff --git a/src/emulator/hub.ts b/src/emulator/hub.ts index 43260048e06..135faff3611 100644 --- a/src/emulator/hub.ts +++ b/src/emulator/hub.ts @@ -12,6 +12,7 @@ import { FunctionsEmulator } from "./functionsEmulator"; import { ExpressBasedEmulator } from "./ExpressBasedEmulator"; import { PortName } from "./portUtils"; import { DataConnectEmulator } from "./dataconnectEmulator"; +import { asyncHandler, jsonErrorHandler } from "./shared/express"; // We use the CLI version from package.json const pkg = require("../../package.json"); @@ -104,87 +105,102 @@ export class EmulatorHub extends ExpressBasedEmulator { res.json(this.getRunningEmulatorsMapping()); }); - app.post(EmulatorHub.PATH_EXPORT, async (req, res) => { - if (req.headers.origin) { - res.status(403).json({ - message: `Export cannot be triggered by external callers.`, - }); - } - const path: string = req.body.path; - const initiatedBy: string = req.body.initiatedBy || "unknown"; - const targets: string[] = req.body.targets; - utils.logLabeledBullet("emulators", `Received export request. Exporting data to ${path}.`); - try { - await new HubExport(this.args.projectId, { - path, - initiatedBy, - targets, - }).exportAll(); - utils.logLabeledSuccess("emulators", "Export complete."); - res.status(200).send({ - message: "OK", - }); - } catch (e: any) { - const errorString = e.message || JSON.stringify(e); - utils.logLabeledWarning("emulators", `Export failed: ${errorString}`); - res.status(500).json({ - message: errorString, - }); - } - }); - - app.put(EmulatorHub.PATH_DISABLE_FUNCTIONS, async (req, res) => { - utils.logLabeledBullet( - "emulators", - `Disabling Cloud Functions triggers, non-HTTP functions will not execute.`, - ); - - const instance = EmulatorRegistry.get(Emulators.FUNCTIONS); - if (!instance) { - res.status(400).json({ error: "The Cloud Functions emulator is not running." }); - return; - } - - const emu = instance as FunctionsEmulator; - await emu.disableBackgroundTriggers(); - res.status(200).json({ enabled: false }); - }); - - app.put(EmulatorHub.PATH_ENABLE_FUNCTIONS, async (req, res) => { - utils.logLabeledBullet( - "emulators", - `Enabling Cloud Functions triggers, non-HTTP functions will execute.`, - ); - - const instance = EmulatorRegistry.get(Emulators.FUNCTIONS); - if (!instance) { - res.status(400).send("The Cloud Functions emulator is not running."); - return; - } + app.post( + EmulatorHub.PATH_EXPORT, + asyncHandler(async (req, res) => { + if (req.headers.origin) { + res.status(403).json({ + message: `Export cannot be triggered by external callers.`, + }); + return; + } + const path: string = req.body.path; + const initiatedBy: string = req.body.initiatedBy || "unknown"; + const targets: string[] = req.body.targets; + utils.logLabeledBullet("emulators", `Received export request. Exporting data to ${path}.`); + try { + await new HubExport(this.args.projectId, { + path, + initiatedBy, + targets, + }).exportAll(); + utils.logLabeledSuccess("emulators", "Export complete."); + res.status(200).send({ + message: "OK", + }); + } catch (e: any) { + const errorString = e.message || JSON.stringify(e); + utils.logLabeledWarning("emulators", `Export failed: ${errorString}`); + res.status(500).json({ + message: errorString, + }); + } + }), + ); + + app.put( + EmulatorHub.PATH_DISABLE_FUNCTIONS, + asyncHandler(async (req, res) => { + utils.logLabeledBullet( + "emulators", + `Disabling Cloud Functions triggers, non-HTTP functions will not execute.`, + ); + + const instance = EmulatorRegistry.get(Emulators.FUNCTIONS); + if (!instance) { + res.status(400).json({ error: "The Cloud Functions emulator is not running." }); + return; + } - const emu = instance as FunctionsEmulator; - await emu.reloadTriggers(); - res.status(200).json({ enabled: true }); - }); + const emu = instance as FunctionsEmulator; + await emu.disableBackgroundTriggers(); + res.status(200).json({ enabled: false }); + }), + ); + + app.put( + EmulatorHub.PATH_ENABLE_FUNCTIONS, + asyncHandler(async (req, res) => { + utils.logLabeledBullet( + "emulators", + `Enabling Cloud Functions triggers, non-HTTP functions will execute.`, + ); + + const instance = EmulatorRegistry.get(Emulators.FUNCTIONS); + if (!instance) { + res.status(400).send("The Cloud Functions emulator is not running."); + return; + } - app.post(EmulatorHub.PATH_CLEAR_DATA_CONNECT, async (req, res) => { - if (req.headers.origin) { - res.status(403).json({ - message: `Clear SQL Connect cannot be triggered by external callers.`, - }); - } - utils.logLabeledBullet("emulators", `Clearing data from SQL Connect data sources.`); + const emu = instance as FunctionsEmulator; + await emu.reloadTriggers(); + res.status(200).json({ enabled: true }); + }), + ); + + app.post( + EmulatorHub.PATH_CLEAR_DATA_CONNECT, + asyncHandler(async (req, res) => { + if (req.headers.origin) { + res.status(403).json({ + message: `Clear SQL Connect cannot be triggered by external callers.`, + }); + return; + } + utils.logLabeledBullet("emulators", `Clearing data from SQL Connect data sources.`); - const instance = EmulatorRegistry.get(Emulators.DATACONNECT) as DataConnectEmulator; - if (!instance) { - res.status(400).json({ error: "The SQL Connect emulator is not running." }); - return; - } + const instance = EmulatorRegistry.get(Emulators.DATACONNECT) as DataConnectEmulator; + if (!instance) { + res.status(400).json({ error: "The SQL Connect emulator is not running." }); + return; + } - await instance.clearData(); - res.status(200).json({ success: true }); - }); + await instance.clearData(); + res.status(200).json({ success: true }); + }), + ); + app.use(jsonErrorHandler(Emulators.HUB)); return app; } diff --git a/src/emulator/shared/express.spec.ts b/src/emulator/shared/express.spec.ts new file mode 100644 index 00000000000..750298fa84f --- /dev/null +++ b/src/emulator/shared/express.spec.ts @@ -0,0 +1,106 @@ +import { expect } from "chai"; +import * as express from "express"; +import * as sinon from "sinon"; +import * as supertest from "supertest"; +import { EmulatorLogger } from "../emulatorLogger"; +import { Emulators } from "../types"; +import { asyncHandler, jsonErrorHandler } from "./express"; + +describe("shared/express", () => { + let logStub: sinon.SinonStub; + + beforeEach(() => { + logStub = sinon.stub(EmulatorLogger.prototype, "log"); + }); + + afterEach(() => { + sinon.restore(); + }); + + function appWithRoute( + handler: (req: express.Request, res: express.Response) => Promise, + ): express.Express { + const app = express(); + // Keeps Express from printing errors it handles itself (after a response was sent) to stderr. + app.set("env", "test"); + app.get("/route", asyncHandler(handler)); + app.use(jsonErrorHandler(Emulators.HUB)); + return app; + } + + describe("jsonErrorHandler", () => { + it("returns a generic JSON 500 without the error details when a handler rejects", async () => { + const app = appWithRoute(() => Promise.reject(new Error("boom at /home/someone/project"))); + + const res = await supertest(app).get("/route"); + + expect(res.status).to.equal(500); + expect(res.type).to.equal("application/json"); + expect(res.body).to.deep.equal({ error: { code: 500, message: "Internal error" } }); + expect(res.text).to.not.include("boom"); + expect(res.text).to.not.include("/home/someone"); + }); + + it("logs server errors through the emulator logger", async () => { + const app = appWithRoute(() => Promise.reject(new Error("boom"))); + + await supertest(app).get("/route"); + + expect(logStub).to.have.been.calledOnce; + const [type, text] = logStub.firstCall.args as [string, string]; + expect(type).to.equal("WARN"); + expect(text).to.include("GET /route failed"); + expect(text).to.include("boom"); + }); + + it("keeps a client error's status and message", async () => { + const app = appWithRoute(() => + Promise.reject(Object.assign(new Error("Bad request body"), { status: 400 })), + ); + + const res = await supertest(app).get("/route"); + + expect(res.status).to.equal(400); + expect(res.body).to.deep.equal({ error: { code: 400, message: "Bad request body" } }); + expect(logStub).to.not.have.been.called; + }); + + it("answers malformed JSON bodies with a JSON 400", async () => { + const app = express(); + app.post("/route", express.json(), (req, res) => { + res.json({ ok: true }); + }); + app.use(jsonErrorHandler(Emulators.STORAGE)); + + const res = await supertest(app) + .post("/route") + .set("Content-Type", "application/json") + .send("{"); + + expect(res.status).to.equal(400); + expect(res.type).to.equal("application/json"); + expect(res.body).to.have.nested.property("error.code", 400); + expect(res.text).to.not.include(" at "); + }); + + it("leaves a response that was already sent alone", async () => { + let handlerDone: () => void = () => undefined; + const handlerSettled = new Promise((resolve) => { + handlerDone = resolve; + }); + const app = appWithRoute(async (req, res) => { + res.status(200).json({ ok: true }); + await new Promise((resolve) => res.once("finish", resolve)); + setImmediate(handlerDone); + throw new Error("late failure"); + }); + + const res = await supertest(app).get("/route"); + await handlerSettled; + + expect(res.status).to.equal(200); + expect(res.body).to.deep.equal({ ok: true }); + expect(logStub).to.not.have.been.called; + }); + }); +}); diff --git a/src/emulator/shared/express.ts b/src/emulator/shared/express.ts new file mode 100644 index 00000000000..15dddb96232 --- /dev/null +++ b/src/emulator/shared/express.ts @@ -0,0 +1,45 @@ +import type { ErrorRequestHandler, NextFunction, Request, RequestHandler, Response } from "express"; +import { getErrMsg, getErrStack } from "../../error"; +import { EmulatorLogger } from "../emulatorLogger"; +import { Emulators } from "../types"; + +/** + * Adapts an async route handler for Express 4, which ignores the promise a handler returns. + * A rejection is passed to `next(err)` (Express's error handling) instead of becoming an + * unhandled promise rejection, which by default terminates the Node.js process. + */ +export function asyncHandler( + fn: (req: Request, res: Response, next: NextFunction) => Promise, +): RequestHandler { + return (req, res, next) => { + void fn(req, res, next).catch((err: unknown) => { + next(err ?? new Error("Async route handler rejected without a reason")); + }); + }; +} + +/** + * Last-resort error handler for an emulator's Express app. Register it after all routes. + * Replies with JSON instead of Express's default HTML error page, which includes the stack + * trace unless NODE_ENV is "production", and logs server errors through the emulator logger. + */ +export function jsonErrorHandler(emulator: Emulators): ErrorRequestHandler { + return (err: unknown, req, res, next) => { + if (res.headersSent) { + next(err); + return; + } + const e = err as { status?: unknown; statusCode?: unknown } | undefined; + const status = e?.status ?? e?.statusCode; + const code = typeof status === "number" && status >= 400 && status < 600 ? status : 500; + if (code >= 500) { + EmulatorLogger.forEmulator(emulator).log( + "WARN", + `${req.method} ${req.path} failed: ${getErrStack(err)}`, + ); + } + res + .status(code) + .json({ error: { code, message: code < 500 ? getErrMsg(err) : "Internal error" } }); + }; +} diff --git a/src/emulator/storage/apis/firebase.ts b/src/emulator/storage/apis/firebase.ts index 419a30d7250..03cf6f73b50 100644 --- a/src/emulator/storage/apis/firebase.ts +++ b/src/emulator/storage/apis/firebase.ts @@ -16,6 +16,7 @@ import { UploadPreviouslyFinalizedError, } from "../upload"; import { reqBodyToBuffer } from "../../shared/request"; +import { asyncHandler } from "../../shared/express"; import { ListObjectsResponse } from "../files"; /** @@ -111,92 +112,98 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router { next(); }); - firebaseStorageAPI.get("/b/:bucketId/o/:objectId", async (req, res) => { - let metadata: StoredFileMetadata; - let data: Buffer; - try { - // Both object data and metadata get can use the same handler since they share auth logic. - ({ metadata, data } = await storageLayer.getObject({ - bucketId: req.params.bucketId, - decodedObjectId: decodeURIComponent(req.params.objectId), - authorization: req.header("authorization"), - downloadToken: req.query.token?.toString(), - })); - } catch (err) { - if (err instanceof NotFoundError) { - return res.sendStatus(404); - } else if (err instanceof ForbiddenError) { - return res.status(403).json({ - error: { - code: 403, - message: `Permission denied. No READ permission.`, - }, - }); + firebaseStorageAPI.get( + "/b/:bucketId/o/:objectId", + asyncHandler(async (req, res) => { + let metadata: StoredFileMetadata; + let data: Buffer; + try { + // Both object data and metadata get can use the same handler since they share auth logic. + ({ metadata, data } = await storageLayer.getObject({ + bucketId: req.params.bucketId, + decodedObjectId: decodeURIComponent(req.params.objectId), + authorization: req.header("authorization"), + downloadToken: req.query.token?.toString(), + })); + } catch (err) { + if (err instanceof NotFoundError) { + return res.sendStatus(404); + } else if (err instanceof ForbiddenError) { + return res.status(403).json({ + error: { + code: 403, + message: `Permission denied. No READ permission.`, + }, + }); + } + throw err; } - throw err; - } - if (metadata.downloadTokens.length === 0) { - metadata.addDownloadToken(/* shouldTrigger = */ true); - } + if (metadata.downloadTokens.length === 0) { + metadata.addDownloadToken(/* shouldTrigger = */ true); + } - // Object data request - if (req.query.alt === "media") { - return sendFileBytes(metadata, data, req, res); - } + // Object data request + if (req.query.alt === "media") { + return sendFileBytes(metadata, data, req, res); + } - // Object metadata request - return res.json(new OutgoingFirebaseMetadata(metadata)); - }); + // Object metadata request + return res.json(new OutgoingFirebaseMetadata(metadata)); + }), + ); // list object handler - firebaseStorageAPI.get("/b/:bucketId/o", async (req, res) => { - const maxResults = req.query.maxResults?.toString(); - let listResponse: ListObjectsResponse; - // The prefix query param must be empty or end in a "/" - let prefix = ""; - if (req.query.prefix) { - prefix = req.query.prefix.toString(); - if (!prefix.endsWith("/")) { - return res.status(400).json({ - error: { - code: 400, - message: - "The prefix parameter is required to be empty or ends with a single / character.", - }, - }); + firebaseStorageAPI.get( + "/b/:bucketId/o", + asyncHandler(async (req, res) => { + const maxResults = req.query.maxResults?.toString(); + let listResponse: ListObjectsResponse; + // The prefix query param must be empty or end in a "/" + let prefix = ""; + if (req.query.prefix) { + prefix = req.query.prefix.toString(); + if (!prefix.endsWith("/")) { + return res.status(400).json({ + error: { + code: 400, + message: + "The prefix parameter is required to be empty or ends with a single / character.", + }, + }); + } } - } - try { - listResponse = await storageLayer.listObjects({ - bucketId: req.params.bucketId, - prefix: prefix, - delimiter: req.query.delimiter ? req.query.delimiter.toString() : "", - pageToken: req.query.pageToken?.toString(), - maxResults: maxResults ? +maxResults : undefined, - authorization: req.header("authorization"), - }); - } catch (err) { - if (err instanceof ForbiddenError) { - return res.status(403).json({ - error: { - code: 403, - message: `Permission denied. No LIST permission.`, - }, + try { + listResponse = await storageLayer.listObjects({ + bucketId: req.params.bucketId, + prefix: prefix, + delimiter: req.query.delimiter ? req.query.delimiter.toString() : "", + pageToken: req.query.pageToken?.toString(), + maxResults: maxResults ? +maxResults : undefined, + authorization: req.header("authorization"), }); + } catch (err) { + if (err instanceof ForbiddenError) { + return res.status(403).json({ + error: { + code: 403, + message: `Permission denied. No LIST permission.`, + }, + }); + } + throw err; } - throw err; - } - return res.status(200).json({ - nextPageToken: listResponse.nextPageToken, - prefixes: (listResponse.prefixes ?? []).filter(isValidPrefix), - items: (listResponse.items ?? []) - .filter((item) => isValidNonEncodedPathString(item.name)) - .map((item) => { - return { name: item.name, bucket: item.bucket }; - }), - }); - }); + return res.status(200).json({ + nextPageToken: listResponse.nextPageToken, + prefixes: (listResponse.prefixes ?? []).filter(isValidPrefix), + items: (listResponse.items ?? []) + .filter((item) => isValidNonEncodedPathString(item.name)) + .map((item) => { + return { name: item.name, bucket: item.bucket }; + }), + }); + }), + ); const handleUpload = async (req: Request, res: Response) => { const bucketId = req.params.bucketId; @@ -492,50 +499,53 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router { firebaseStorageAPI.patch( "/b/:bucketId/o/:objectId", json({ limit: "130mb" }), - handleMetadataUpdate, + asyncHandler(handleMetadataUpdate), ); firebaseStorageAPI.put( "/b/:bucketId/o/:objectId?", firebasePostUploadMiddleware, - async (req, res) => { + asyncHandler(async (req, res) => { switch (req.header("x-http-method-override")?.toLowerCase()) { case "patch": return handleMetadataUpdate(req, res); default: return handleObjectPostRequest(req, res); } - }, + }), ); firebaseStorageAPI.post( "/b/:bucketId/o/:objectId?", firebasePostUploadMiddleware, - handleObjectPostRequest, + asyncHandler(handleObjectPostRequest), ); - firebaseStorageAPI.delete("/b/:bucketId/o/:objectId", async (req, res) => { - try { - await storageLayer.deleteObject({ - bucketId: req.params.bucketId, - decodedObjectId: decodeURIComponent(req.params.objectId), - authorization: req.header("authorization"), - }); - } catch (err) { - if (err instanceof ForbiddenError) { - return res.status(403).json({ - error: { - code: 403, - message: `Permission denied. No WRITE permission.`, - }, + firebaseStorageAPI.delete( + "/b/:bucketId/o/:objectId", + asyncHandler(async (req, res) => { + try { + await storageLayer.deleteObject({ + bucketId: req.params.bucketId, + decodedObjectId: decodeURIComponent(req.params.objectId), + authorization: req.header("authorization"), }); + } catch (err) { + if (err instanceof ForbiddenError) { + return res.status(403).json({ + error: { + code: 403, + message: `Permission denied. No WRITE permission.`, + }, + }); + } + if (err instanceof NotFoundError) { + return res.sendStatus(404); + } + throw err; } - if (err instanceof NotFoundError) { - return res.sendStatus(404); - } - throw err; - } - res.sendStatus(204); - }); + res.sendStatus(204); + }), + ); firebaseStorageAPI.get("/", (req, res) => { res.json({ emulator: "storage" }); diff --git a/src/emulator/storage/apis/gcloud.ts b/src/emulator/storage/apis/gcloud.ts index 30a868baace..775229a3e2e 100644 --- a/src/emulator/storage/apis/gcloud.ts +++ b/src/emulator/storage/apis/gcloud.ts @@ -17,6 +17,7 @@ import { parseFormDataMultipartRequest, parseObjectUploadMultipartRequest } from import { Upload, UploadNotActiveError } from "../upload"; import { ForbiddenError, NotFoundError } from "../errors"; import { reqBodyToBuffer } from "../../shared/request"; +import { asyncHandler } from "../../shared/express"; import type { Query } from "express-serve-static-core"; export function createCloudEndpoints(emulator: StorageEmulator): Router { @@ -87,12 +88,15 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { next(); }); - gcloudStorageAPI.get("/b", async (req, res) => { - res.json({ - kind: "storage#buckets", - items: await adminStorageLayer.listBuckets(), - }); - }); + gcloudStorageAPI.get( + "/b", + asyncHandler(async (req, res) => { + res.json({ + kind: "storage#buckets", + items: await adminStorageLayer.listBuckets(), + }); + }), + ); gcloudStorageAPI.get( [ @@ -100,7 +104,7 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { "/download/storage/v1/b/:bucketId/o/:objectId", "/storage/v1/b/:bucketId/o/:objectId", ], - async (req, res) => { + asyncHandler(async (req, res) => { let getObjectResponse: GetObjectResponse; try { getObjectResponse = await adminStorageLayer.getObject({ @@ -121,58 +125,65 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { return sendFileBytes(getObjectResponse.metadata, getObjectResponse.data, req, res); } return res.json(new CloudStorageObjectMetadata(getObjectResponse.metadata)); - }, + }), ); - gcloudStorageAPI.patch("/b/:bucketId/o/:objectId", json({ limit: "130mb" }), async (req, res) => { - let updatedMetadata: StoredFileMetadata; - try { - updatedMetadata = await adminStorageLayer.updateObjectMetadata({ - bucketId: req.params.bucketId, - decodedObjectId: req.params.objectId, - metadata: req.body as IncomingMetadata, - }); - } catch (err) { - if (err instanceof NotFoundError) { - return sendObjectNotFound(req, res); - } - if (err instanceof ForbiddenError) { - return res.sendStatus(403); + gcloudStorageAPI.patch( + "/b/:bucketId/o/:objectId", + json({ limit: "130mb" }), + asyncHandler(async (req, res) => { + let updatedMetadata: StoredFileMetadata; + try { + updatedMetadata = await adminStorageLayer.updateObjectMetadata({ + bucketId: req.params.bucketId, + decodedObjectId: req.params.objectId, + metadata: req.body as IncomingMetadata, + }); + } catch (err) { + if (err instanceof NotFoundError) { + return sendObjectNotFound(req, res); + } + if (err instanceof ForbiddenError) { + return res.sendStatus(403); + } + throw err; } - throw err; - } - return res.json(new CloudStorageObjectMetadata(updatedMetadata)); - }); + return res.json(new CloudStorageObjectMetadata(updatedMetadata)); + }), + ); - gcloudStorageAPI.get(["/b/:bucketId/o", "/storage/v1/b/:bucketId/o"], async (req, res) => { - let listResponse: ListObjectsResponse; - // TODO validate that all query params are single strings and are not repeated. - try { - listResponse = await adminStorageLayer.listObjects({ - bucketId: req.params.bucketId, - prefix: req.query.prefix ? req.query.prefix.toString() : "", - delimiter: req.query.delimiter ? req.query.delimiter.toString() : "", - pageToken: req.query.pageToken ? req.query.pageToken.toString() : undefined, - maxResults: req.query.maxResults ? +req.query.maxResults.toString() : undefined, - authorization: req.header("authorization"), - }); - } catch (err) { - if (err instanceof ForbiddenError) { - return res.sendStatus(403); + gcloudStorageAPI.get( + ["/b/:bucketId/o", "/storage/v1/b/:bucketId/o"], + asyncHandler(async (req, res) => { + let listResponse: ListObjectsResponse; + // TODO validate that all query params are single strings and are not repeated. + try { + listResponse = await adminStorageLayer.listObjects({ + bucketId: req.params.bucketId, + prefix: req.query.prefix ? req.query.prefix.toString() : "", + delimiter: req.query.delimiter ? req.query.delimiter.toString() : "", + pageToken: req.query.pageToken ? req.query.pageToken.toString() : undefined, + maxResults: req.query.maxResults ? +req.query.maxResults.toString() : undefined, + authorization: req.header("authorization"), + }); + } catch (err) { + if (err instanceof ForbiddenError) { + return res.sendStatus(403); + } + throw err; } - throw err; - } - return res.status(200).json({ - kind: "storage#objects", - nextPageToken: listResponse.nextPageToken, - prefixes: listResponse.prefixes, - items: listResponse.items?.map((item) => new CloudStorageObjectMetadata(item)), - }); - }); + return res.status(200).json({ + kind: "storage#objects", + nextPageToken: listResponse.nextPageToken, + prefixes: listResponse.prefixes, + items: listResponse.items?.map((item) => new CloudStorageObjectMetadata(item)), + }); + }), + ); gcloudStorageAPI.delete( ["/b/:bucketId/o/:objectId", "/storage/v1/b/:bucketId/o/:objectId"], - async (req, res) => { + asyncHandler(async (req, res) => { try { await adminStorageLayer.deleteObject({ bucketId: req.params.bucketId, @@ -188,13 +199,13 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { throw err; } return res.sendStatus(204); - }, + }), ); gcloudStorageAPI.put( "/upload/storage/v1/b/:bucketId/o", bodyParser.raw({ type: "*/*", limit: "130mb" }), - async (req, res) => { + asyncHandler(async (req, res) => { if (!req.query.upload_id) { res.sendStatus(400); return; @@ -224,13 +235,13 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { throw err; } return res.json(new CloudStorageObjectMetadata(metadata)); - }, + }), ); gcloudStorageAPI.post( "/b/:bucketId/o/:objectId/acl", json({ limit: "130mb" }), - async (req, res) => { + asyncHandler(async (req, res) => { EmulatorLogger.forEmulator(Emulators.STORAGE).log( "WARN_ONCE", "Cloud Storage ACLs are not supported in the Storage Emulator. All related methods will succeed, but have no effect.", @@ -268,13 +279,13 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { etag: "someEtag", generation: metadata.generation.toString(), } as CloudStorageObjectAccessControlMetadata); - }, + }), ); gcloudStorageAPI.post( "/upload/storage/v1/b/:bucketId/o", postUploadMiddleware, - async (req, res) => { + asyncHandler(async (req, res) => { const uploadType = req.query.uploadType || req.header("X-Goog-Upload-Protocol"); // Resumable upload protocol. @@ -368,27 +379,30 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { authorization: req.header("authorization"), }); return await finalizeOneShotUpload(upload); - }, + }), ); - gcloudStorageAPI.get("/:bucketId/:objectId(**)", async (req, res) => { - let getObjectResponse: GetObjectResponse; - try { - getObjectResponse = await adminStorageLayer.getObject({ - bucketId: req.params.bucketId, - decodedObjectId: req.params.objectId, - }); - } catch (err) { - if (err instanceof NotFoundError) { - return sendObjectNotFound(req, res); - } - if (err instanceof ForbiddenError) { - return res.sendStatus(403); + gcloudStorageAPI.get( + "/:bucketId/:objectId(**)", + asyncHandler(async (req, res) => { + let getObjectResponse: GetObjectResponse; + try { + getObjectResponse = await adminStorageLayer.getObject({ + bucketId: req.params.bucketId, + decodedObjectId: req.params.objectId, + }); + } catch (err) { + if (err instanceof NotFoundError) { + return sendObjectNotFound(req, res); + } + if (err instanceof ForbiddenError) { + return res.sendStatus(403); + } + throw err; } - throw err; - } - return sendFileBytes(getObjectResponse.metadata, getObjectResponse.data, req, res); - }); + return sendFileBytes(getObjectResponse.metadata, getObjectResponse.data, req, res); + }), + ); gcloudStorageAPI.post( "/:bucketId", @@ -396,7 +410,7 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { adminStorageLayer.createBucket(req.params.bucketId); next(); }, - async (req, res) => { + asyncHandler(async (req, res) => { const contentTypeHeader = req.header("content-type"); if (!contentTypeHeader?.includes("multipart/form-data")) { return res.status(400).send("Content-Type must be multipart/form-data"); @@ -458,7 +472,7 @@ export function createCloudEndpoints(emulator: StorageEmulator): Router { } throw err; } - }, + }), ); gcloudStorageAPI.post( diff --git a/src/emulator/storage/rules/manager.ts b/src/emulator/storage/rules/manager.ts index adcce0c4efa..55bd393f2ab 100644 --- a/src/emulator/storage/rules/manager.ts +++ b/src/emulator/storage/rules/manager.ts @@ -80,6 +80,36 @@ class DefaultStorageRulesManager implements StorageRulesManager { } private updateWatcher(rulesFile: string): Promise { + const handleFileChange = async (): Promise => { + // There have been some race conditions reported (on Windows) where reading the + // file too quickly after the watcher fires results in an empty file being read. + // Adding a small delay prevents that at very little cost. + await new Promise((res) => setTimeout(res, WATCHER_CHANGE_DELAY_MS)); + + // In some OSes and CI environments, chokidar can emit a 'change' event + // when a file is deleted or if it is a non-existent mock file (e.g. /dev/null/storage.rules). + // Because we are inside an async callback that is not awaited by the event emitter, + // any synchronous errors thrown by `readFile` will result in an UnhandledPromiseRejection, + // which crashes the process. We catch these errors to prevent the emulator and tests from crashing. + try { + const content = readFile(rulesFile); + this._rules.content = content; + this._logger.logLabeled( + "BULLET", + "storage", + "Change detected, updating rules for Cloud Storage...", + ); + await this.loadRuleset(); + } catch (e) { + const message = e instanceof Error ? e.message : String(e); + this._logger.logLabeled( + "DEBUG", + "storage", + `A rule file change was detected, but there was an error reading it: ${message}`, + ); + } + }; + return new Promise((resolve) => { // Fallback timer so start() never hangs if chokidar ready event doesn't fire const timer = setTimeout(resolve, WATCHER_READY_TIMEOUT_MS); @@ -101,34 +131,9 @@ class DefaultStorageRulesManager implements StorageRulesManager { ); resolve(); }) - .on("change", async () => { - // There have been some race conditions reported (on Windows) where reading the - // file too quickly after the watcher fires results in an empty file being read. - // Adding a small delay prevents that at very little cost. - await new Promise((res) => setTimeout(res, WATCHER_CHANGE_DELAY_MS)); - - // In some OSes and CI environments, chokidar can emit a 'change' event - // when a file is deleted or if it is a non-existent mock file (e.g. /dev/null/storage.rules). - // Because we are inside an async callback that is not awaited by the event emitter, - // any synchronous errors thrown by `readFile` will result in an UnhandledPromiseRejection, - // which crashes the process. We catch these errors to prevent the emulator and tests from crashing. - try { - const content = readFile(rulesFile); - this._rules.content = content; - this._logger.logLabeled( - "BULLET", - "storage", - "Change detected, updating rules for Cloud Storage...", - ); - await this.loadRuleset(); - } catch (e) { - const message = e instanceof Error ? e.message : String(e); - this._logger.logLabeled( - "DEBUG", - "storage", - `A rule file change was detected, but there was an error reading it: ${message}`, - ); - } + .on("change", () => { + // handleFileChange catches all errors and logs via logger + void handleFileChange(); }); }); } diff --git a/src/emulator/storage/server.ts b/src/emulator/storage/server.ts index e2b8ccc923e..56601dc2de1 100644 --- a/src/emulator/storage/server.ts +++ b/src/emulator/storage/server.ts @@ -8,6 +8,7 @@ import { RulesConfig, StorageEmulator } from "./index"; import { createFirebaseEndpoints } from "./apis/firebase"; import { InvalidArgumentError } from "../auth/errors"; import { SourceFile } from "./rules/types"; +import { asyncHandler, jsonErrorHandler } from "../shared/express"; /** * @param defaultProjectId @@ -61,17 +62,21 @@ export function createApp( app.use(bodyParser.raw({ limit: "130mb", type: "application/x-www-form-urlencoded" })); app.use(bodyParser.raw({ limit: "130mb", type: "multipart/related" })); - app.post("/internal/export", express.json({ limit: "130mb" }), async (req, res) => { - const initiatedBy: string = req.body.initiatedBy || "unknown"; - const path: string = req.body.path; - if (!path) { - res.status(400).send("Export request body must include 'path'."); - return; - } + app.post( + "/internal/export", + express.json({ limit: "130mb" }), + asyncHandler(async (req, res) => { + const initiatedBy: string = req.body.initiatedBy || "unknown"; + const path: string = req.body.path; + if (!path) { + res.status(400).send("Export request body must include 'path'."); + return; + } - await storageLayer.export(path, { initiatedBy }); - res.sendStatus(200); - }); + await storageLayer.export(path, { initiatedBy }); + res.sendStatus(200); + }), + ); /** * Internal endpoint to overwrite current rules. Callers provide either a single set of rules to @@ -100,63 +105,70 @@ export function createApp( * } * ``` */ - app.put("/internal/setRules", express.json({ limit: "130mb" }), async (req, res) => { - const rulesRaw = req.body.rules; - if (!(rulesRaw && Array.isArray(rulesRaw.files) && rulesRaw.files.length > 0)) { - res.status(400).json({ - message: "Request body must include 'rules.files' array", - }); - return; - } + app.put( + "/internal/setRules", + express.json({ limit: "130mb" }), + asyncHandler(async (req, res) => { + const rulesRaw = req.body.rules; + if (!(rulesRaw && Array.isArray(rulesRaw.files) && rulesRaw.files.length > 0)) { + res.status(400).json({ + message: "Request body must include 'rules.files' array", + }); + return; + } - const { files } = rulesRaw; + const { files } = rulesRaw; + + function parseRulesFromFiles(files: Array): SourceFile | RulesConfig[] { + if (files.length === 1) { + const file = files[0]; + if (!isRulesFile(file)) { + throw new InvalidArgumentError( + "Each member of 'rules.files' array must contain 'name' and 'content'", + ); + } + return { name: file.name, content: file.content }; + } - function parseRulesFromFiles(files: Array): SourceFile | RulesConfig[] { - if (files.length === 1) { - const file = files[0]; - if (!isRulesFile(file)) { - throw new InvalidArgumentError( - "Each member of 'rules.files' array must contain 'name' and 'content'", - ); + const rules: RulesConfig[] = []; + for (const file of files) { + if (!isRulesFile(file) || !file.resource) { + throw new InvalidArgumentError( + "Each member of 'rules.files' array must contain 'name', 'content', and 'resource'", + ); + } + rules.push({ + resource: file.resource, + rules: { name: file.name, content: file.content }, + }); } - return { name: file.name, content: file.content }; + return rules; } - const rules: RulesConfig[] = []; - for (const file of files) { - if (!isRulesFile(file) || !file.resource) { - throw new InvalidArgumentError( - "Each member of 'rules.files' array must contain 'name', 'content', and 'resource'", - ); + let rules: SourceFile | RulesConfig[]; + try { + rules = parseRulesFromFiles(files); + } catch (err) { + if (err instanceof InvalidArgumentError) { + res.status(400).json({ message: err.message }); + return; } - rules.push({ resource: file.resource, rules: { name: file.name, content: file.content } }); + throw err; } - return rules; - } - let rules: SourceFile | RulesConfig[]; - try { - rules = parseRulesFromFiles(files); - } catch (err) { - if (err instanceof InvalidArgumentError) { - res.status(400).json({ message: err.message }); + const issues = await emulator.replaceRules(rules); + if (issues.errors.length > 0) { + res.status(400).json({ + message: "There was an error updating rules, see logs for more details", + }); return; } - throw err; - } - const issues = await emulator.replaceRules(rules); - if (issues.errors.length > 0) { - res.status(400).json({ - message: "There was an error updating rules, see logs for more details", + res.status(200).json({ + message: "Rules updated successfully", }); - return; - } - - res.status(200).json({ - message: "Rules updated successfully", - }); - }); + }), + ); app.post("/internal/reset", (req, res) => { emulator.reset(); @@ -165,6 +177,7 @@ export function createApp( app.use("/v0", createFirebaseEndpoints(emulator)); app.use("/", createCloudEndpoints(emulator)); + app.use(jsonErrorHandler(Emulators.STORAGE)); return Promise.resolve(app); } diff --git a/src/frameworks/utils.spec.ts b/src/frameworks/utils.spec.ts index f694dbcf5a6..af9f5ecfedc 100644 --- a/src/frameworks/utils.spec.ts +++ b/src/frameworks/utils.spec.ts @@ -254,7 +254,10 @@ describe("Frameworks utils", () => { nock("http://localhost:8999").post("/api", "payload").reply(404); const app = express(); - app.use(simpleProxy("http://localhost:8999")); + const proxy = simpleProxy("http://localhost:8999"); + app.use((req, res, next) => { + void proxy(req, res, next).catch(next); + }); app.use((req, res) => { res.json({ rawBody: (req as any).rawBody?.toString() }); }); @@ -266,7 +269,10 @@ describe("Frameworks utils", () => { nock("http://localhost:8999").post("/api", "payload").reply(200, "handled"); const app = express(); - app.use(simpleProxy("http://localhost:8999")); + const proxy = simpleProxy("http://localhost:8999"); + app.use((req, res, next) => { + void proxy(req, res, next).catch(next); + }); app.use((_req, res) => { res.status(500).send("should not cascade"); }); @@ -284,7 +290,10 @@ describe("Frameworks utils", () => { .reply(404); const app = express(); - app.use(simpleProxy("http://localhost:8999")); + const proxy = simpleProxy("http://localhost:8999"); + app.use((req, res, next) => { + void proxy(req, res, next).catch(next); + }); app.use((req, res) => { res.json({ rawBody: (req as any).rawBody?.toString() }); }); diff --git a/src/functionsShellCommandAction.ts b/src/functionsShellCommandAction.ts index fb6cefbba08..3ac62b52bf5 100644 --- a/src/functionsShellCommandAction.ts +++ b/src/functionsShellCommandAction.ts @@ -143,7 +143,7 @@ export const actionFunction = async (options: Options) => { return new Promise((resolve) => { replServer.on("exit", () => { - return serveFunctions.stop().then(resolve).catch(resolve); + serveFunctions.stop().then(resolve).catch(resolve); }); }); }); diff --git a/src/hosting/proxy.ts b/src/hosting/proxy.ts index e3f90ddaffe..51fad8e336a 100644 --- a/src/hosting/proxy.ts +++ b/src/hosting/proxy.ts @@ -1,7 +1,7 @@ import { capitalize, includes } from "lodash"; import { IncomingMessage, ServerResponse } from "http"; import { PassThrough } from "stream"; -import { Request, RequestHandler, Response } from "express"; +import { NextFunction, Request, RequestHandler, Response } from "express"; import { URL } from "url"; import { Client, HttpMethod } from "../apiv2"; @@ -55,7 +55,11 @@ export function proxyRequestHandler( rewriteIdentifier: string, options: { forceCascade?: boolean } = {}, ): RequestHandler { - return async (req: IncomingMessage, res: ServerResponse, next: () => void): Promise => { + const handle = async ( + req: IncomingMessage, + res: ServerResponse, + next: NextFunction, + ): Promise => { logger.info(`[hosting] Rewriting ${req.url} to ${url} for ${rewriteIdentifier}`); // Extract the __session cookie from headers to forward it to the // functions cookie is not a string[]. @@ -229,6 +233,9 @@ export function proxyRequestHandler( res.statusCode = proxyRes.status; proxyRes.body.pipe(res); }; + return (req, res, next) => { + void handle(req, res, next).catch(next); + }; } /** diff --git a/src/mcp/apps/init/mcp-app.ts b/src/mcp/apps/init/mcp-app.ts index b509973e814..761b4bd8fec 100644 --- a/src/mcp/apps/init/mcp-app.ts +++ b/src/mcp/apps/init/mcp-app.ts @@ -111,7 +111,7 @@ googleCheckbox.addEventListener("change", (e) => { } }); -initBtn.addEventListener("click", async () => { +async function onInitClick(): Promise { const selectedProduct = Array.from(productRadios).find((r) => r.checked)?.value; if (!selectedProjectId) { @@ -205,6 +205,11 @@ initBtn.addEventListener("click", async () => { initBtn.disabled = false; initBtn.textContent = "Initialize"; } +} + +// onInitClick catches all errors and reports status via setStatus +initBtn.addEventListener("click", () => { + void onInitClick(); }); app.onhostcontextchanged = (ctx) => { diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 40add802de2..1ff7b00225f 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -537,7 +537,7 @@ export class FirebaseMcpServer { const port = options.port || 3000; const transports: Record = {}; // session ID to transport - app.get("/sse", async (req: express.Request, res: express.Response) => { + const handleSse = async (req: express.Request, res: express.Response): Promise => { this.logger.debug(`[SSE] GET /sse connection attempt from ${req.ip}`); try { const transport = new SSEServerTransport("/message", res); @@ -564,9 +564,14 @@ export class FirebaseMcpServer { } catch (err) { this.logger.error(`[SSE] Connection error: ${err}`); } + }; + + // handleSse catches all errors and logs via this.logger.error + app.get("/sse", (req: express.Request, res: express.Response) => { + void handleSse(req, res); }); - app.post("/message", async (req: express.Request, res: express.Response) => { + const handleMessage = async (req: express.Request, res: express.Response): Promise => { const sessionId = req.query.sessionId as string; this.logger.debug(`[SSE] POST /message attempt for session ${sessionId}`); @@ -585,6 +590,11 @@ export class FirebaseMcpServer { ); res.status(400).send("No active SSE transport connection found for this session"); } + }; + + // handleMessage catches all errors and logs via this.logger.error + app.post("/message", (req: express.Request, res: express.Response) => { + void handleMessage(req, res); }); app.listen(port, "127.0.0.1", () => { diff --git a/src/utils.ts b/src/utils.ts index d5b5daede23..a657208657a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -392,12 +392,14 @@ export async function promiseWhile( if (check(res)) { return resolve(res); } - setTimeout(run, interval); + // run catches all errors and routes them to promiseReject + setTimeout(() => void run(), interval); } catch (err: unknown) { return promiseReject(err); } }; - run(); + // run catches all errors and routes them to promiseReject + void run(); }); }