From 37cc5596692249b28453490e1d9124e71430171b Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 3 Mar 2020 10:52:13 -0800 Subject: [PATCH] grpc-js: Only automatically retry picks on known error --- packages/grpc-js/src/channel.ts | 51 +++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index 8420b909e..1d555ea2d 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -265,29 +265,42 @@ export class ChannelImplementation implements Channel { callStream ); } catch (error) { - /* An error here indicates that something went wrong with - * the picked subchannel's http2 stream right before we - * tried to start the stream. We are handling a promise - * result here, so this is asynchronous with respect to the - * original tryPick call, so calling it again is not - * recursive. We call tryPick immediately instead of - * queueing this pick again because handling the queue is - * triggered by state changes, and we want to immediately - * check if the state has already changed since the - * previous tryPick call. We do this instead of cancelling - * the stream because the correct behavior may be - * re-queueing instead, based on the logic in the rest of - * tryPick */ - trace( - LogVerbosity.INFO, - 'channel', - 'Failed to start call on picked subchannel ' + + if ((error as NodeJS.ErrnoException).code === 'ERR_HTTP2_GOAWAY_SESSION') { + /* An error here indicates that something went wrong with + * the picked subchannel's http2 stream right before we + * tried to start the stream. We are handling a promise + * result here, so this is asynchronous with respect to the + * original tryPick call, so calling it again is not + * recursive. We call tryPick immediately instead of + * queueing this pick again because handling the queue is + * triggered by state changes, and we want to immediately + * check if the state has already changed since the + * previous tryPick call. We do this instead of cancelling + * the stream because the correct behavior may be + * re-queueing instead, based on the logic in the rest of + * tryPick */ + trace( + LogVerbosity.INFO, + 'channel', + 'Failed to start call on picked subchannel ' + pickResult.subchannel!.getAddress() + ' with error ' + (error as Error).message + '. Retrying pick' - ); - this.tryPick(callStream, callMetadata); + ); + this.tryPick(callStream, callMetadata); + } else { + trace( + LogVerbosity.INFO, + 'channel', + 'Failed to start call on picked subchanel ' + + pickResult.subchannel!.getAddress() + + ' with error ' + + (error as Error).message + + '. Ending call' + ); + callStream.cancelWithStatus(Status.INTERNAL, 'Failed to start HTTP/2 stream'); + } } } else { /* The logic for doing this here is the same as in the catch