fix(emulator): stringify send() errors before writing them to stderr - #10905
fix(emulator): stringify send() errors before writing them to stderr#10905mattsears18 wants to merge 3 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue in the Functions emulator where an IPC failure was masked by a TypeError when writing the raw error object to process.stderr. It introduces unit tests for EmulatorLog.flush() and uses getErrStack to safely log the error stack. The review feedback suggests avoiding the 'as any' cast on process.send by using a specific function signature to align with the repository style guide, and clearing EmulatorLog.LOG_BUFFER upon encountering an IPC error to prevent flooding stderr with duplicate stack traces.
bda5615 to
ecfb771
Compare
EmulatorLog.flush() passed the Error object from process.send()'s callback straight to process.stderr.write(), which accepts only a string, Buffer, TypedArray, or DataView. Writing an Error makes write() throw synchronously, and it does so inside a callback with no surrounding try/catch -- an uncaught exception. The cost is diagnostic and it compounds: this path only runs when the IPC channel to the functions runtime has already failed, so the error that gets destroyed is the one explaining why the runtime became unreachable, and what operators see instead is a TypeError about stream chunk types pointing at Node's stream internals. Fixes firebase#10876
ecfb771 to
4f1c712
Compare
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Applying the buffer-clearing suggestion removed the process.send() call line itself along with its callback, leaving the if (err) body and its closing brace behind, so the file no longer compiled. Restore the call without the `as any` cast. @types/node declares send(message, sendHandle?, options?, callback?) with the callback overload, so the cast is unnecessary and the comment claiming otherwise was stale; typing the callback parameter Error | null typechecks cleanly. The node docs link is kept as the reference for that signature.
Description
Fixes #10876.
EmulatorLog.flush()handed theErrorobject fromprocess.send()'s callback straight toprocess.stderr.write():stream.write()accepts only a string,Buffer,TypedArray, orDataView, so writing anErrorthrows synchronously — inside a callback with no surroundingtry/catch, which makes it an uncaught exception:That path only runs when the IPC channel to the functions runtime has already failed (the subprocess died, was killed, or the channel closed), so the error being destroyed is exactly the one explaining why the runtime became unreachable. Operators are left with a
TypeErrorabout stream chunk types pointing at Node's stream internals instead. We hit this on CI runners where the emulator subprocess gets killed under memory pressure, and it made two separate emulator-flake investigations materially harder.This stringifies the error before writing, using the existing
getErrStack()helper fromsrc/error.tsso a non-Errorvalue is handled too:Behavior is otherwise unchanged — same call site, same flush loop, same buffering semantics.
Scenarios Tested
Added
src/emulator/types.spec.tscovering theflush()error path. Itsprocess.stderr.writestub enforces Node's real chunk-type restriction, so the test reproduces the actual crash rather than just asserting on the argument:process.send()reports anError→ the stack is written to stderr, no throw. Fails onmainwith the exactTypeErrorfrom the issue; passes with this change.process.send()reports a non-Error(e.g. a string code) → written as-is, no throw.process.send()succeeds → nothing written to stderr.Verification run locally on Node v24.15.0:
mocha src/emulator/*.spec.ts→ 143 passing.mocha 'src/**/*.spec.{ts,js}') → 4954 passing, 10 pending, 2 failing. Both failures are unrelated to this change and reproduce on unmodifiedmain:src/commands/login.spec.ts("authCode completion flow") fails identically on a pristine checkout, and theaccounts:updateauth-emulator failure is asocket hang upunder full-suite load — that spec file passes 46/46 in isolation both with and without this change.tsc --project tsconfig.compile.json→ clean.eslinton both touched files → no errors;prettier --check→ clean.Sample Commands
No command or flag surface changes.