Skip to content

feat(firestore): Add support for 16MB documents#8649

Draft
dlarocque wants to merge 2 commits into
mainfrom
dl/largedoc
Draft

feat(firestore): Add support for 16MB documents#8649
dlarocque wants to merge 2 commits into
mainfrom
dl/largedoc

Conversation

@dlarocque

Copy link
Copy Markdown
Contributor

WIP

@product-auto-label product-auto-label Bot added the api: firestore Issues related to the Firestore API. label Jun 15, 2026

@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 updates the Firestore client to support larger documents by increasing the gRPC max receive and send message lengths to 17MB and configuring flow control window sizes. It also adds a new integration test suite (large_document.ts) to verify handling of large documents (up to 15.9MB) and rejection of oversized payloads. The review feedback correctly identifies an issue in the oversized payload test where a successful write would throw a generic error that gets caught and fails on an unrelated assertion, masking the true failure; a cleaner assertion pattern is suggested.

Comment on lines +169 to +174
try {
await oversizedDoc.set({chunk: largePayload});
throw new Error('Setting a document exceeding the 16MB limit should fail.');
} catch (error: any) {
expect(error.code).to.equal(3); // INVALID_ARGUMENT (gRPC status code 3)
}

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.

medium

If oversizedDoc.set unexpectedly succeeds, the explicitly thrown Error will be caught by the catch block. Since that Error object does not have a code property, error.code will be undefined, causing the assertion expect(error.code).to.equal(3) to fail. This masks the actual failure (that the document was successfully set when it shouldn't have been) with an unrelated assertion failure.

Consider capturing the error and asserting on it outside the try-catch block.

    let error: any;
    try {
      await oversizedDoc.set({chunk: largePayload});
    } catch (e) {
      error = e;
    }
    expect(error).to.exist;
    expect(error.code).to.equal(3);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the Firestore API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant