feat(firestore): Add support for 16MB documents#8649
Conversation
There was a problem hiding this comment.
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.
| 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) | ||
| } |
There was a problem hiding this comment.
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);
WIP