fix(bigtable): avoid self-referential error on RPC-level mutate failure#8823
Open
kyungseopk1m wants to merge 2 commits into
Open
fix(bigtable): avoid self-referential error on RPC-level mutate failure#8823kyungseopk1m wants to merge 2 commits into
kyungseopk1m wants to merge 2 commits into
Conversation
When mutate hits an RPC-level failure with entries still pending and no per-entry mutation errors, mutateInternal set err.errors to an array containing err itself. The resulting cycle (err.errors[i] === err) makes the error impossible to serialize with tools that follow the AggregateError convention of recursing into errors, such as pino, which throws RangeError: Maximum call stack size exceeded. Push a fresh error carrying the RPC-level code and message for each pending entry instead of err itself. Fixes googleapis#8075
Contributor
There was a problem hiding this comment.
Code Review
This pull request resolves a serialization issue in mutateInternal.ts where a self-referencing error was created by pushing the parent RPC error into its own errors array. It fixes this by creating a fresh entryError with the copied message and code, and adds a corresponding regression test. The reviewer suggested also copying metadata and details from the original RPC error to preserve debugging context.
… error The fix for googleapis#8075 replaced the self-referential error with a fresh error carrying only the RPC-level code and message. The former self-reference also exposed the metadata and details of the gRPC failure on each entry error; restore that debugging context on the fresh error so downstream consumers do not lose it. The regression test now seeds a non-empty metadata and details on the failure and asserts both are carried through. Fixes googleapis#8075
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
Table#mutate/Table#inserthits an RPC-level failure with entries still pending and no per-entry mutation errors,mutateInternalseterr.errorsto an array containingerritself. The resulting cycle (err.errors[i] === err) makes the error impossible to serialize with anything that follows theAggregateErrorconvention of recursing intoerrors— for example pino, which throwsRangeError: Maximum call stack size exceeded.This pushes a fresh error carrying the RPC-level
codeandmessagefor each pending entry instead oferritself. Theerrorsarray is only consumed for its per-entrycode/message(the existingTable#mutateerror tests and thecallback.err.errorsdocs), so nothing relies on the former self-reference.The existing "not retryable" test already exercises this branch (an index-0 success followed by a stream error); it now also asserts that
err.errors[0]is noterrand that the resulting error can be serialized.Fixes #8075 🦕