firestore: wrapError() throws TypeError when the wrapped error has a non-writable stack, surfacing as an uncaughtException

[Komal-Dhawan]

Environment

• @google-cloud/firestore: 9.0.0 (also reproduced on 7.11.6)
• google-auth-library: 11.0.2 (also reproduced on 10.6.2)
• Node.js: 20.19.5
• OS: macOS (not platform specific)

Summary

wrapError() — handwritten/firestore/dev/src/util.ts:236, shipped as build/src/util.js — decorates an error by appending to its stack:

function wrapError(err, stack) {
    err.stack += '\nCaused by: ' + stack;
    return err;
}

This assumes err.stack is writable. That is not guaranteed. When the wrapped error has a non-writable stack, the compound assignment throws a TypeError and the original error is destroyed — replaced by an unrelated one that points at util.js.

Because util.js is emitted with "use strict", the write throws rather than silently no-op'ing (in sloppy mode the same assignment is silently discarded).

This is reachable in practice, not just theoretically: google-auth-library produces exactly such errors. getErrorFromOAuthErrorResponse() copies properties onto a new Error with writable: false, and explicitly includes stack in that copy (oauth2common.ts, the keys.push('stack') / Object.defineProperty(..., { writable: false }) block). So every STS/OAuth error response surfaced through an external_account (Workload Identity Federation) credential carries a non-writable stack.

I've filed the counterpart issue on that side too: googleapis/google-auth-library-nodejs — see cross-reference below. Fixing either end resolves the crash, but the guard here seems worth having regardless, since wrapError cannot know what kind of error it is handed.

Why this is worse than a confusing message

Firestore calls wrapError from stream 'error' handlers. The TypeError is therefore thrown inside an EventEmitter emit, so it escapes the surrounding promise chain and every enclosing try/catch, and lands as an uncaughtException. In a serverless runtime that takes down the invocation instead of producing a clean rejected promise.

Observed trace (paths genericised), repeated once per failing request:

TypeError: Cannot assign to read only property 'stack' of object 'Error: Error code invalid_grant: ID Token issued at <ts> is stale to sign-in.'
    at wrapError (/app/node_modules/@google-cloud/firestore/build/src/util.js:213:15)
    at Transform.<anonymous> (/app/node_modules/@google-cloud/firestore/build/src/index.js:...)
    at Transform.emit (node:events:524:28)
    at emitErrorNT (node:internal/streams/destroy:169:8)
    at emitErrorCloseNT (node:internal/streams/destroy:128:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

The real failure was an expired federated credential. What the process reported instead was a TypeError about a read-only property, which points at this library rather than at the credential — the actual cause is only visible once you inspect the message string.

Reproduction

// npm i google-auth-library
const { getErrorFromOAuthErrorResponse } = require('google-auth-library/build/src/auth/oauth2common.js');

// Exactly what the STS/external_account path produces on an error response:
const authError = getErrorFromOAuthErrorResponse(
  { error: 'invalid_grant', error_description: 'ID Token is stale to sign-in.' },
  new Error('underlying transport failure'),
);

console.log(Object.getOwnPropertyDescriptor(authError, 'stack').writable);
// => false

// This is the assignment wrapError() performs, in a strict-mode module:
(function () { 'use strict'; authError.stack += '\nCaused by: <caller stack>'; })();
// => TypeError: Cannot assign to read only property 'stack' of object
//    'Error: Error code invalid_grant: ID Token is stale to sign-in.'

Calling wrapError directly requires reaching past the package exports map, so the snippet above models the one line it executes. Passing that same authError into the real wrapError throws identically at util.js:216 on 9.0.0 (util.js:213 on 7.11.6), and routing it through a stream 'error' handler that calls wrapError reproduces the uncaughtException end to end.

Suggested fix

Make the decoration non-fatal, so a hostile stack degrades the message rather than replacing the error:

function wrapError(err, stack) {
    try {
        err.stack += '\nCaused by: ' + stack;
    } catch {
        // `stack` may be non-writable (e.g. errors from google-auth-library's
        // OAuth error path). Preserve the original error rather than throwing.
    }
    return err;
}

If keeping the appended context matters in that case, Object.defineProperty(err, 'stack', { value: <combined>, writable: true, configurable: true }) works when the property is configurable — it is in the google-auth-library case (configurable: true, writable: false).

[Komal-Dhawan]

Cross-reference: the google-auth-library side of this interaction is #9155 — getErrorFromOAuthErrorResponse() is what marks stack non-writable. Both packages live in this monorepo, so either fix resolves the crash; the guard proposed here also protects wrapError against any other error with a non-writable stack.