fix: Reset the retry counter to 0 when receiving data #1604
Conversation
This file contains 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
…prepare-reset-counter-to-0-PR-fix # Conflicts: # gax/test/test-application/src/index.ts
danieljbruce
changed the title
Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>
leahecole
approved these changes
May 24, 2024
Merged
This was referenced Aug 1, 2024
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.
Source code changes
A handler for data is added along with a line of code that sets the retry counter to 0 when data is received. This ensures that the max retries limit is only reached if enough consecutive retriable errors are encountered without any data being received between those errors. This is being introduced to match current nodejs-bigtable retry behaviour where a counter is reset that interacts with the
maxRetriesparameters for custom retries.Test Application changes
A test is added that emits 4 retryable errors from a server while also emitting data between each error. Since max retries is set in this test to 2 then the test ensures that the retry count is actually being reset because the test only passes if none of the retryable errors result in the max retries limit being reached.
Unit Test changes
An existing test pushes data between retries, but expects an error to be thrown when the number of retries exceeds the max retries value of 2. We no longer want this behaviour because if data is being emitted between retries then an error saying the max retries has been exceeded should not get emitted. Therefore, we change the test so that it doesn't emit data in which case we should see the max retries exceeded error making the test validate client library behaviour that we expect.