grpc-js: Fix handling of calls after resolution failure #2092
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
nicolasnoble
approved these changes
Apr 13, 2022
|
For some reason the test passes everywhere except on Node 10 on Windows. Previously, it probably passed nowhere, so it's still an improvement. Plus, Node 10 is well past EOL, so I don't think it's worth worrying about. |
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.
This fixes #2090. The change to the channel here is just to track name resolution errors so that if a call comes in while the channel is in TRANSIENT_FAILURE, backing off waiting to resolve again, we end the call with an error, the same as if the LB policy was in TRANSIENT_FAILURE. The change to the resolving load balancer just lets the
exitIdleschedule another name resolution attempt if the state is TRANSIENT_FAILURE, which is consistent with handling ofexitIdleand backoffs in other places. I removed the state transition from there because it is redundant with the one inupdateResolution, and inappropriate when not calling that function immediately. I also moved the backoff timer start to the start of name resolution instead of the end, for consistency with other backoff timers.