This repository was archived by the owner on Mar 26, 2026. It is now read-only.
fix: Do not allow gaxOptions to be modified downstream#1379
Merged
danieljbruce merged 1 commit intogoogleapis:owl-bot-copyfrom Jan 22, 2024
Merged
fix: Do not allow gaxOptions to be modified downstream#1379danieljbruce merged 1 commit intogoogleapis:owl-bot-copyfrom
danieljbruce merged 1 commit intogoogleapis:owl-bot-copyfrom
Conversation
This reverts commit 3cadbe0.
product-auto-label
Bot
added
size: xs
daniel-sanche
approved these changes
Jan 22, 2024
danieljbruce
added a commit
that referenced
this pull request
Jan 23, 2024
* feat: Modify ModifyColumnFamiliesRequest proto to expose ignore_warnings field PiperOrigin-RevId: 590940407 Source-Link: googleapis/googleapis@fb027c8 Source-Link: googleapis/googleapis-gen@f0728cd Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZjA3MjhjZGEyMjdiMzg4MzU4MjJjNGU1NTE5ZTU2OGNlOGQyYjVhYyJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat: Adding feature flags for routing cookie and retry info PiperOrigin-RevId: 591912877 Source-Link: googleapis/googleapis@f6505fe Source-Link: googleapis/googleapis-gen@7499187 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNzQ5OTE4NzQxNWY4ZDQwNWVmMGQ0NmRkNmZmNjA4YjEyNWM1M2M4ZiJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * docs: update comment for `AccountVerificationInfo.username` PiperOrigin-RevId: 598888823 Source-Link: googleapis/googleapis@955ff0a Source-Link: googleapis/googleapis-gen@8b0b992 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOGIwYjk5MmNkY2EwMDQxOWNkZjQ0MzNkNmJiZTNjZjNlOTNmMWRkNCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat: configure PHP client to generate new surface PiperOrigin-RevId: 599228576 Source-Link: googleapis/googleapis@ef54eaf Source-Link: googleapis/googleapis-gen@4d0d81b Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNGQwZDgxYjc4MjJlMmYzNjFkZTZkNmUwMWIzNzVjY2M5YWYzZTEwOSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: improve retry logic for streaming API calls build: update typescript generator version to 4.3.0 The streaming API call retry logic has changed, which in some rare cases may require code changes. Please feel free to reach out to us in the issues if you experience new problems with retrying streaming calls after this update. PiperOrigin-RevId: 599622271 Source-Link: googleapis/googleapis@6239c21 Source-Link: googleapis/googleapis-gen@da13d82 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZGExM2Q4MjIyZDNiYTMzNzM0NTAxOTk5ODY0NDU4NjQwZjE0MDVhZSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Make a copy of the gaxOpts parameter Don’t modify the gaxOptions parameter in place and pass it down. This change makes it so that changes to gax options downstream will not affect gaxOptions in this layer of the call stack which is desirable and which also allows the failing test to pass. * Revert "Make a copy of the gaxOpts parameter" This reverts commit aa0c46b. * Revert "Revert "Make a copy of the gaxOpts parameter"" (#1379) This reverts commit 3cadbe0. --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Daniel Bruce <[email protected]> Co-authored-by: danieljbruce <[email protected]>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Currently, #1372 has a failing test that produces an error that says
Uncaught Error: Only one of retry or retryRequestOptions may be set. This code change allows that test to pass on thegcf-owl-botPR branch.Here is what currently happens:
The failing test uses a mock server that aborts the stream simulating a network error. This causes the client to retry. However, on the first call the client makes,
options.gaxOptionsgets passed down the stack atnodejs-bigtable/src/table.ts
Line 906 in 8cc17aa
options.gaxOptionsis defined having settings that matchhttps://github.com/googleapis/nodejs-bigtable/blob/8cc17aa2acb2540b4c2b233c4245021815a7b030/src/v2/bigtable_client_config.json#L12-L20.. On the second call,options.gaxOptionsis passed down the stack with theretryparameter defined this time and with the retryRequestOptions parameter attached atnodejs-bigtable/src/index.ts
Line 859 in 8cc17aa
Uncaught Error: Only one of retry or retryRequestOptions may be set.What this fix does:
With this fix a copy of
options.gaxOptionsis passed down the call stack instead ofoptions.gaxOptionsitself. This means when the client retries,options.gaxOptionsdoes not contain theretryparameter in the createReadStream function as this parameter is normally filled out much further down the stack. This means the errorUncaught Error: Only one of retry or retryRequestOptions may be setis not thrown.Additional effects:
For retries, this means
options.gaxOptions?.retry?.backoffSettingswill beundefinedatnodejs-bigtable/src/table.ts
Lines 960 to 962 in 8cc17aa
DEFAULT_BACKOFF_SETTINGSif backoff settings are not provided by the user. This is exactly what we want and is also the intent of the current code.