From 3a9f4d2aa698ec595c2b923e6aac6d478d4b2a44 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 16 Oct 2023 16:52:41 -0700 Subject: [PATCH] grpc-js: Propagate connectivity error information to request errors --- packages/grpc-js/package.json | 2 +- .../grpc-js/src/load-balancer-pick-first.ts | 20 +++++++++++++++---- .../grpc-js/src/load-balancer-round-robin.ts | 12 ++++++++--- packages/grpc-js/src/picker.ts | 17 +++++++--------- packages/grpc-js/src/subchannel-interface.ts | 3 ++- packages/grpc-js/src/subchannel.ts | 8 +++++--- packages/grpc-js/src/transport.ts | 9 +++++++-- 7 files changed, 47 insertions(+), 24 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 003663b6..9d66b409 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.9.5", + "version": "1.9.6", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts index 0f2d7d65..1a22b3f7 100644 --- a/packages/grpc-js/src/load-balancer-pick-first.ts +++ b/packages/grpc-js/src/load-balancer-pick-first.ts @@ -153,9 +153,11 @@ export class PickFirstLoadBalancer implements LoadBalancer { private subchannelStateListener: ConnectivityStateListener = ( subchannel, previousState, - newState + newState, + keepaliveTime, + errorMessage ) => { - this.onSubchannelStateUpdate(subchannel, previousState, newState); + this.onSubchannelStateUpdate(subchannel, previousState, newState, errorMessage); }; /** * Timer reference for the timer tracking when to start @@ -172,6 +174,12 @@ export class PickFirstLoadBalancer implements LoadBalancer { */ private stickyTransientFailureMode = false; + /** + * The most recent error reported by any subchannel as it transitioned to + * TRANSIENT_FAILURE. + */ + private lastError: string | null = null; + /** * Load balancer that attempts to connect to each backend in the address list * in order, and picks the first one that connects, using it for every @@ -200,7 +208,7 @@ export class PickFirstLoadBalancer implements LoadBalancer { if (this.stickyTransientFailureMode) { this.updateState( ConnectivityState.TRANSIENT_FAILURE, - new UnavailablePicker() + new UnavailablePicker({details: `No connection established. Last error: ${this.lastError}`}) ); } else { this.updateState(ConnectivityState.CONNECTING, new QueuePicker(this)); @@ -241,7 +249,8 @@ export class PickFirstLoadBalancer implements LoadBalancer { private onSubchannelStateUpdate( subchannel: SubchannelInterface, previousState: ConnectivityState, - newState: ConnectivityState + newState: ConnectivityState, + errorMessage?: string ) { if (this.currentPick?.realSubchannelEquals(subchannel)) { if (newState !== ConnectivityState.READY) { @@ -258,6 +267,9 @@ export class PickFirstLoadBalancer implements LoadBalancer { } if (newState === ConnectivityState.TRANSIENT_FAILURE) { child.hasReportedTransientFailure = true; + if (errorMessage) { + this.lastError = errorMessage; + } this.maybeEnterStickyTransientFailureMode(); if (index === this.currentSubchannelIndex) { this.startNextSubchannelConnecting(index + 1); diff --git a/packages/grpc-js/src/load-balancer-round-robin.ts b/packages/grpc-js/src/load-balancer-round-robin.ts index f389fefc..062aa9f0 100644 --- a/packages/grpc-js/src/load-balancer-round-robin.ts +++ b/packages/grpc-js/src/load-balancer-round-robin.ts @@ -105,18 +105,24 @@ export class RoundRobinLoadBalancer implements LoadBalancer { private currentReadyPicker: RoundRobinPicker | null = null; + private lastError: string | null = null; + constructor(private readonly channelControlHelper: ChannelControlHelper) { this.subchannelStateListener = ( subchannel: SubchannelInterface, previousState: ConnectivityState, - newState: ConnectivityState + newState: ConnectivityState, + keepaliveTime: number, + errorMessage?: string ) => { this.calculateAndUpdateState(); - if ( newState === ConnectivityState.TRANSIENT_FAILURE || newState === ConnectivityState.IDLE ) { + if (errorMessage) { + this.lastError = errorMessage; + } this.channelControlHelper.requestReresolution(); subchannel.startConnecting(); } @@ -157,7 +163,7 @@ export class RoundRobinLoadBalancer implements LoadBalancer { ) { this.updateState( ConnectivityState.TRANSIENT_FAILURE, - new UnavailablePicker() + new UnavailablePicker({details: `No connection established. Last error: ${this.lastError}`}) ); } else { this.updateState(ConnectivityState.IDLE, new QueuePicker(this)); diff --git a/packages/grpc-js/src/picker.ts b/packages/grpc-js/src/picker.ts index d95eca21..6474269f 100644 --- a/packages/grpc-js/src/picker.ts +++ b/packages/grpc-js/src/picker.ts @@ -97,16 +97,13 @@ export interface Picker { */ export class UnavailablePicker implements Picker { private status: StatusObject; - constructor(status?: StatusObject) { - if (status !== undefined) { - this.status = status; - } else { - this.status = { - code: Status.UNAVAILABLE, - details: 'No connection established', - metadata: new Metadata(), - }; - } + constructor(status?: Partial) { + this.status = { + code: Status.UNAVAILABLE, + details: 'No connection established', + metadata: new Metadata(), + ...status, + }; } pick(pickArgs: PickArgs): TransientFailurePickResult { return { diff --git a/packages/grpc-js/src/subchannel-interface.ts b/packages/grpc-js/src/subchannel-interface.ts index 9b947ad3..cc19c22c 100644 --- a/packages/grpc-js/src/subchannel-interface.ts +++ b/packages/grpc-js/src/subchannel-interface.ts @@ -23,7 +23,8 @@ export type ConnectivityStateListener = ( subchannel: SubchannelInterface, previousState: ConnectivityState, newState: ConnectivityState, - keepaliveTime: number + keepaliveTime: number, + errorMessage?: string ) => void; /** diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 0f884472..03bbf035 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -250,7 +250,8 @@ export class Subchannel { error => { this.transitionToState( [ConnectivityState.CONNECTING], - ConnectivityState.TRANSIENT_FAILURE + ConnectivityState.TRANSIENT_FAILURE, + `${error}` ); } ); @@ -265,7 +266,8 @@ export class Subchannel { */ private transitionToState( oldStates: ConnectivityState[], - newState: ConnectivityState + newState: ConnectivityState, + errorMessage?: string ): boolean { if (oldStates.indexOf(this.connectivityState) === -1) { return false; @@ -318,7 +320,7 @@ export class Subchannel { throw new Error(`Invalid state: unknown ConnectivityState ${newState}`); } for (const listener of this.stateListeners) { - listener(this, previousState, newState, this.keepaliveTime); + listener(this, previousState, newState, this.keepaliveTime, errorMessage); } return true; } diff --git a/packages/grpc-js/src/transport.ts b/packages/grpc-js/src/transport.ts index 49ec01dd..8dc4e2de 100644 --- a/packages/grpc-js/src/transport.ts +++ b/packages/grpc-js/src/transport.ts @@ -741,6 +741,7 @@ export class Http2SubchannelConnector implements SubchannelConnector { connectionOptions ); this.session = session; + let errorMessage = 'Failed to connect'; session.unref(); session.once('connect', () => { session.removeAllListeners(); @@ -749,10 +750,14 @@ export class Http2SubchannelConnector implements SubchannelConnector { }); session.once('close', () => { this.session = null; - reject(); + // Leave time for error event to happen before rejecting + setImmediate(() => { + reject(`${errorMessage} (${new Date().toISOString()})`); + }); }); session.once('error', error => { - this.trace('connection failed with error ' + (error as Error).message); + errorMessage = (error as Error).message; + this.trace('connection failed with error ' + errorMessage); }); }); }