Skip to content

fix(emulator): stringify send() errors before writing them to stderr - #10905

Open
mattsears18 wants to merge 3 commits into
firebase:mainfrom
mattsears18:fix/emulator-log-flush-stringify-err
Open

fix(emulator): stringify send() errors before writing them to stderr#10905
mattsears18 wants to merge 3 commits into
firebase:mainfrom
mattsears18:fix/emulator-log-flush-stringify-err

Conversation

@mattsears18

Copy link
Copy Markdown

Description

Fixes #10876.

EmulatorLog.flush() handed the Error object from process.send()'s callback straight to process.stderr.write():

(process.send as any)(nextMsg, undefined, {}, (err: any) => {
  if (err) {
    process.stderr.write(err);   // err is an Error object
  }
  ...
});

stream.write() accepts only a string, Buffer, TypedArray, or DataView, so writing an Error throws synchronously — inside a callback with no surrounding try/catch, which makes it an uncaught exception:

TypeError: The "chunk" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Error
  at _write (node:internal/streams/writable:480)
  at lib/emulator/types.js:180

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 TypeError about 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 from src/error.ts so a non-Error value is handled too:

process.stderr.write(`${getErrStack(err)}\n`);

Behavior is otherwise unchanged — same call site, same flush loop, same buffering semantics.

Scenarios Tested

Added src/emulator/types.spec.ts covering the flush() error path. Its process.stderr.write stub enforces Node's real chunk-type restriction, so the test reproduces the actual crash rather than just asserting on the argument:

  • process.send() reports an Error → the stack is written to stderr, no throw. Fails on main with the exact TypeError from 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.
  • Full unit suite (mocha 'src/**/*.spec.{ts,js}') → 4954 passing, 10 pending, 2 failing. Both failures are unrelated to this change and reproduce on unmodified main: src/commands/login.spec.ts ("authCode completion flow") fails identically on a pristine checkout, and the accounts:update auth-emulator failure is a socket hang up under full-suite load — that spec file passes 46/46 in isolation both with and without this change.
  • tsc --project tsconfig.compile.json → clean.
  • eslint on both touched files → no errors; prettier --check → clean.

Sample Commands

No command or flag surface changes.

@google-cla

google-cla Bot commented Aug 8, 2026

Copy link
Copy Markdown

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/emulator/types.ts Outdated
Comment thread src/emulator/types.ts
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
@mattsears18
mattsears18 force-pushed the fix/emulator-log-flush-stringify-err branch from ecfb771 to 4f1c712 Compare August 8, 2026 14:18
@mattsears18
mattsears18 marked this pull request as ready for review August 8, 2026 14:28
mattsears18 and others added 2 commits August 8, 2026 10:29
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EmulatorLog.flush() crashes with TypeError when process.send() reports an error, destroying the original error

2 participants