From a188ae1681ce7bdc30ef437a66e251b16b585531 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 7 Mar 2025 14:46:12 -0800 Subject: [PATCH 01/11] grpc-js: Add backoff timer trace logging --- .../interop/test-client.Dockerfile | 2 +- packages/grpc-js/src/backoff-timeout.ts | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/interop/test-client.Dockerfile b/packages/grpc-js-xds/interop/test-client.Dockerfile index db608f95b..438b39bf2 100644 --- a/packages/grpc-js-xds/interop/test-client.Dockerfile +++ b/packages/grpc-js-xds/interop/test-client.Dockerfile @@ -42,7 +42,7 @@ COPY --from=build /node/src/grpc-node/packages/grpc-js ./packages/grpc-js/ COPY --from=build /node/src/grpc-node/packages/grpc-js-xds ./packages/grpc-js-xds/ ENV GRPC_VERBOSITY="DEBUG" -ENV GRPC_TRACE=xds_client,xds_resolver,xds_cluster_manager,cds_balancer,xds_cluster_resolver,xds_cluster_impl,priority,weighted_target,round_robin,resolving_load_balancer,subchannel,keepalive,dns_resolver,fault_injection,http_filter,csds,outlier_detection,server,server_call,ring_hash,transport,certificate_provider,xds_channel_credentials +ENV GRPC_TRACE=xds_client,xds_resolver,xds_cluster_manager,cds_balancer,xds_cluster_resolver,xds_cluster_impl,priority,weighted_target,round_robin,resolving_load_balancer,subchannel,keepalive,dns_resolver,fault_injection,http_filter,csds,outlier_detection,server,server_call,ring_hash,transport,certificate_provider,xds_channel_credentials,backoff ENV NODE_XDS_INTEROP_VERBOSITY=1 ENTRYPOINT [ "/nodejs/bin/node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ] diff --git a/packages/grpc-js/src/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts index ae5a62cab..e2fd4a2c6 100644 --- a/packages/grpc-js/src/backoff-timeout.ts +++ b/packages/grpc-js/src/backoff-timeout.ts @@ -15,6 +15,11 @@ * */ +import { LogVerbosity } from './constants'; +import * as logging from './logging'; + +const TRACER_NAME = 'backoff'; + const INITIAL_BACKOFF_MS = 1000; const BACKOFF_MULTIPLIER = 1.6; const MAX_BACKOFF_MS = 120000; @@ -84,7 +89,12 @@ export class BackoffTimeout { */ private endTime: Date = new Date(); + private id: number; + + private static nextId = 0; + constructor(private callback: () => void, options?: BackoffOptions) { + this.id = BackoffTimeout.getNextId(); if (options) { if (options.initialDelay) { this.initialDelay = options.initialDelay; @@ -99,18 +109,29 @@ export class BackoffTimeout { this.maxDelay = options.maxDelay; } } + this.trace('constructed initialDelay=' + this.initialDelay + ' multiplier=' + this.multiplier + ' jitter=' + this.jitter + ' maxDelay=' + this.maxDelay); this.nextDelay = this.initialDelay; this.timerId = setTimeout(() => {}, 0); clearTimeout(this.timerId); } + private static getNextId() { + return this.nextId++; + } + + private trace(text: string) { + logging.trace(LogVerbosity.DEBUG, TRACER_NAME, '{' + this.id + '} ' + text); + } + private runTimer(delay: number) { + this.trace('runTimer(delay=' + delay + ')'); this.endTime = this.startTime; this.endTime.setMilliseconds( this.endTime.getMilliseconds() + delay ); clearTimeout(this.timerId); this.timerId = setTimeout(() => { + this.trace('timer fired'); this.callback(); this.running = false; }, delay); @@ -123,6 +144,7 @@ export class BackoffTimeout { * Call the callback after the current amount of delay time */ runOnce() { + this.trace('runOnce()'); this.running = true; this.startTime = new Date(); this.runTimer(this.nextDelay); @@ -140,6 +162,7 @@ export class BackoffTimeout { * again. */ stop() { + this.trace('stop()'); clearTimeout(this.timerId); this.running = false; } @@ -149,6 +172,7 @@ export class BackoffTimeout { * retroactively apply that reset to the current timer. */ reset() { + this.trace('reset() running=' + this.running); this.nextDelay = this.initialDelay; if (this.running) { const now = new Date(); From bd3bbe12945c48944b5e92fa6e3ad7bb11c82963 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 11 Mar 2025 13:54:42 -0700 Subject: [PATCH 02/11] grpc-js: Fix reentrancy problem in backoff timer callback --- packages/grpc-js/src/backoff-timeout.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/src/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts index e2fd4a2c6..8be560da1 100644 --- a/packages/grpc-js/src/backoff-timeout.ts +++ b/packages/grpc-js/src/backoff-timeout.ts @@ -132,8 +132,8 @@ export class BackoffTimeout { clearTimeout(this.timerId); this.timerId = setTimeout(() => { this.trace('timer fired'); - this.callback(); this.running = false; + this.callback(); }, delay); if (!this.hasRef) { this.timerId.unref?.(); From 18fddad85e1840460d39645c4aa0104d8c085f48 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 20 Mar 2025 17:17:17 -0700 Subject: [PATCH 03/11] grpc-js: Don't check authorized when rejectUnauthorized is false --- packages/grpc-js/src/channel-credentials.ts | 4 ++-- packages/grpc-js/test/test-channel-credentials.ts | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/src/channel-credentials.ts b/packages/grpc-js/src/channel-credentials.ts index 16739a639..5eec92258 100644 --- a/packages/grpc-js/src/channel-credentials.ts +++ b/packages/grpc-js/src/channel-credentials.ts @@ -268,7 +268,7 @@ class SecureConnectorImpl implements SecureConnector { }; return new Promise((resolve, reject) => { const tlsSocket = tlsConnect(tlsConnectOptions, () => { - if (!tlsSocket.authorized) { + if ((this.connectionOptions.rejectUnauthorized ?? true) && !tlsSocket.authorized) { reject(tlsSocket.authorizationError); return; } @@ -364,7 +364,7 @@ class CertificateProviderChannelCredentialsImpl extends ChannelCredentials { const tlsSocket = tlsConnect(tlsConnectOptions, () => { tlsSocket.removeListener('close', closeCallback); tlsSocket.removeListener('error', errorCallback); - if (!tlsSocket.authorized) { + if ((this.parent.verifyOptions.rejectUnauthorized ?? true) && !tlsSocket.authorized) { reject(tlsSocket.authorizationError); return; } diff --git a/packages/grpc-js/test/test-channel-credentials.ts b/packages/grpc-js/test/test-channel-credentials.ts index a03ec41b2..b40d9400d 100644 --- a/packages/grpc-js/test/test-channel-credentials.ts +++ b/packages/grpc-js/test/test-channel-credentials.ts @@ -218,6 +218,15 @@ describe('ChannelCredentials usage', () => { } ); }); + it('Should accept self-signed certs with acceptUnauthorized', done => { + const client = new echoService(`localhost:${portNum}`, grpc.credentials.createSsl(null, null, null, {rejectUnauthorized: false})); + client.echo({ value: 'test value', value2: 3 }, (error: ServiceError | null, response: any) => { + client.close(); + assert.ifError(error); + assert.deepStrictEqual(response, { value: 'test value', value2: 3 }); + done(); + }); + }); }); describe('Channel credentials mtls', () => { From 26d26d7b0a5dca5e58af743e3c0cac878f308999 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 20 Mar 2025 17:47:10 -0700 Subject: [PATCH 04/11] grpc-js: Fix min/max switch in retry throttler --- packages/grpc-js/src/retrying-call.ts | 17 ++++++++++------- packages/grpc-js/test/test-retry.ts | 4 ++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js/src/retrying-call.ts b/packages/grpc-js/src/retrying-call.ts index a4d63e92f..c51aed1c2 100644 --- a/packages/grpc-js/src/retrying-call.ts +++ b/packages/grpc-js/src/retrying-call.ts @@ -57,15 +57,15 @@ export class RetryThrottler { } addCallSucceeded() { - this.tokens = Math.max(this.tokens + this.tokenRatio, this.maxTokens); + this.tokens = Math.min(this.tokens + this.tokenRatio, this.maxTokens); } addCallFailed() { - this.tokens = Math.min(this.tokens - 1, 0); + this.tokens = Math.max(this.tokens - 1, 0); } canRetryCall() { - return this.tokens > this.maxTokens / 2; + return this.tokens > (this.maxTokens / 2); } } @@ -217,7 +217,10 @@ export class RetryingCall implements Call, DeadlineInfoProvider { private readonly retryThrottler?: RetryThrottler ) { const maxAttemptsLimit = channel.getOptions()['grpc-node.retry_max_attempts_limit'] ?? DEFAULT_MAX_ATTEMPTS_LIMIT; - if (callConfig.methodConfig.retryPolicy) { + if (channel.getOptions()['grpc.enable_retries'] === 0) { + this.state = 'NO_RETRY'; + this.maxAttempts = 1; + } else if (callConfig.methodConfig.retryPolicy) { this.state = 'RETRY'; const retryPolicy = callConfig.methodConfig.retryPolicy; this.nextRetryBackoffSec = this.initialRetryBackoffSec = Number( @@ -230,9 +233,6 @@ export class RetryingCall implements Call, DeadlineInfoProvider { } else if (callConfig.methodConfig.hedgingPolicy) { this.state = 'HEDGING'; this.maxAttempts = Math.min(callConfig.methodConfig.hedgingPolicy.maxAttempts, maxAttemptsLimit); - } else if (channel.getOptions()['grpc.enable_retries'] === 0) { - this.state = 'NO_RETRY'; - this.maxAttempts = 1; } else { this.state = 'TRANSPARENT_ONLY'; this.maxAttempts = 1; @@ -459,6 +459,9 @@ export class RetryingCall implements Call, DeadlineInfoProvider { callback(true); this.attempts += 1; this.startNewAttempt(); + } else { + this.trace('Retry attempt denied by throttling policy'); + callback(false); } }, retryDelayMs); } diff --git a/packages/grpc-js/test/test-retry.ts b/packages/grpc-js/test/test-retry.ts index 26ad26c2a..d7e5a09cd 100644 --- a/packages/grpc-js/test/test-retry.ts +++ b/packages/grpc-js/test/test-retry.ts @@ -175,6 +175,10 @@ describe('Retries', () => { }, }, ], + retryThrottling: { + maxTokens: 1000, + tokenRatio: 0.1, + }, }; client = new EchoService( `localhost:${port}`, From 2bb7eae3c9beb5f03d56cbf757d4357304a59fbd Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 21 Mar 2025 10:27:11 -0700 Subject: [PATCH 05/11] grpc-js: Bump to 1.13.1 --- packages/grpc-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index b2dc54209..ff834be38 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.13.0", + "version": "1.13.1", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", From bdcbdf42326689f30ea859c02d2b248eb6095219 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 26 Mar 2025 13:16:01 -0700 Subject: [PATCH 06/11] grpc-js: Consistently reference the same options object in the channel constructor --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/internal-channel.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index ff834be38..d303926b7 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.13.1", + "version": "1.13.2", "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/internal-channel.ts b/packages/grpc-js/src/internal-channel.ts index f47f620a7..db3827f7b 100644 --- a/packages/grpc-js/src/internal-channel.ts +++ b/packages/grpc-js/src/internal-channel.ts @@ -297,16 +297,16 @@ export class InternalChannel { /* The global boolean parameter to getSubchannelPool has the inverse meaning to what * the grpc.use_local_subchannel_pool channel option means. */ this.subchannelPool = getSubchannelPool( - (options['grpc.use_local_subchannel_pool'] ?? 0) === 0 + (this.options['grpc.use_local_subchannel_pool'] ?? 0) === 0 ); this.retryBufferTracker = new MessageBufferTracker( - options['grpc.retry_buffer_size'] ?? DEFAULT_RETRY_BUFFER_SIZE_BYTES, - options['grpc.per_rpc_retry_buffer_size'] ?? + this.options['grpc.retry_buffer_size'] ?? DEFAULT_RETRY_BUFFER_SIZE_BYTES, + this.options['grpc.per_rpc_retry_buffer_size'] ?? DEFAULT_PER_RPC_RETRY_BUFFER_SIZE_BYTES ); - this.keepaliveTime = options['grpc.keepalive_time_ms'] ?? -1; + this.keepaliveTime = this.options['grpc.keepalive_time_ms'] ?? -1; this.idleTimeoutMs = Math.max( - options['grpc.client_idle_timeout_ms'] ?? DEFAULT_IDLE_TIMEOUT_MS, + this.options['grpc.client_idle_timeout_ms'] ?? DEFAULT_IDLE_TIMEOUT_MS, MIN_IDLE_TIMEOUT_MS ); const channelControlHelper: ChannelControlHelper = { @@ -372,7 +372,7 @@ export class InternalChannel { this.resolvingLoadBalancer = new ResolvingLoadBalancer( this.target, channelControlHelper, - options, + this.options, (serviceConfig, configSelector) => { if (serviceConfig.retryThrottling) { RETRY_THROTTLER_MAP.set( From 6168fe8197b62a3320741e9e588123ef720067ae Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 4 Apr 2025 10:52:26 -0700 Subject: [PATCH 07/11] grpc-js: Disable Nagle's Algorithm --- packages/grpc-js/src/transport.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/grpc-js/src/transport.ts b/packages/grpc-js/src/transport.ts index d14a22273..f2bd0ab14 100644 --- a/packages/grpc-js/src/transport.ts +++ b/packages/grpc-js/src/transport.ts @@ -763,6 +763,7 @@ export class Http2SubchannelConnector implements SubchannelConnector { await secureConnector.waitForReady(); this.trace(addressString + ' secureConnector is ready'); tcpConnection = await this.tcpConnect(address, options); + tcpConnection.setNoDelay(); this.trace(addressString + ' Established TCP connection'); secureConnectResult = await secureConnector.connect(tcpConnection); this.trace(addressString + ' Established secure connection'); From 482006e286172661a899e09ffe213fb373025dbc Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 4 Apr 2025 12:41:04 -0700 Subject: [PATCH 08/11] grpc-js: Avoid calling http2.getDefaultSettings --- packages/grpc-js/src/transport.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js/src/transport.ts b/packages/grpc-js/src/transport.ts index d14a22273..4d69b7f4e 100644 --- a/packages/grpc-js/src/transport.ts +++ b/packages/grpc-js/src/transport.ts @@ -695,16 +695,17 @@ export class Http2SubchannelConnector implements SubchannelConnector { reject(`${errorMessage} (${new Date().toISOString()})`); } }; - const session = http2.connect(`${scheme}://${targetPath}`, { + const sessionOptions: http2.ClientSessionOptions = { createConnection: (authority, option) => { return secureConnectResult.socket; - }, - settings: { - initialWindowSize: - options['grpc-node.flow_control_window'] ?? - http2.getDefaultSettings().initialWindowSize, } - }); + }; + if (options['grpc-node.flow_control_window'] !== undefined) { + sessionOptions.settings = { + initialWindowSize: options['grpc-node.flow_control_window'] + }; + } + const session = http2.connect(`${scheme}://${targetPath}`, sessionOptions); this.session = session; let errorMessage = 'Failed to connect'; let reportedError = false; From 75a96ecbae0900975e10a5cc30d2b3853aa073e7 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 10 Apr 2025 11:08:48 -0700 Subject: [PATCH 09/11] grpc-js: Bump to 1.13.3 --- packages/grpc-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index d303926b7..2f07286a1 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.13.2", + "version": "1.13.3", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", From a1aff9d1bca5709e0a13cf3499668f4fc61cd7a0 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 15 May 2025 14:09:32 -0700 Subject: [PATCH 10/11] grpc-js: Fix ability to set SNI with ssl_target_name_override option --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/channel-credentials.ts | 39 +++++++-------------- 2 files changed, 14 insertions(+), 27 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 2f07286a1..b05a2cde1 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.13.3", + "version": "1.13.4", "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/channel-credentials.ts b/packages/grpc-js/src/channel-credentials.ts index 5eec92258..a6ded81ea 100644 --- a/packages/grpc-js/src/channel-credentials.ts +++ b/packages/grpc-js/src/channel-credentials.ts @@ -206,6 +206,18 @@ function getConnectionOptions(secureContext: SecureContext, verifyOptions: Verif const connectionOptions: ConnectionOptions = { secureContext: secureContext }; + let realTarget: GrpcUri = channelTarget; + if ('grpc.http_connect_target' in options) { + const parsedTarget = parseUri(options['grpc.http_connect_target']!); + if (parsedTarget) { + realTarget = parsedTarget; + } + } + const targetPath = getDefaultAuthority(realTarget); + const hostPort = splitHostPort(targetPath); + const remoteHost = hostPort?.host ?? targetPath; + connectionOptions.host = remoteHost; + if (verifyOptions.checkServerIdentity) { connectionOptions.checkServerIdentity = verifyOptions.checkServerIdentity; } @@ -225,36 +237,11 @@ function getConnectionOptions(secureContext: SecureContext, verifyOptions: Verif }; connectionOptions.servername = sslTargetNameOverride; } else { - if ('grpc.http_connect_target' in options) { - /* This is more or less how servername will be set in createSession - * if a connection is successfully established through the proxy. - * If the proxy is not used, these connectionOptions are discarded - * anyway */ - const targetPath = getDefaultAuthority( - parseUri(options['grpc.http_connect_target'] as string) ?? { - path: 'localhost', - } - ); - const hostPort = splitHostPort(targetPath); - connectionOptions.servername = hostPort?.host ?? targetPath; - } + connectionOptions.servername = remoteHost; } if (options['grpc-node.tls_enable_trace']) { connectionOptions.enableTrace = true; } - - let realTarget: GrpcUri = channelTarget; - if ('grpc.http_connect_target' in options) { - const parsedTarget = parseUri(options['grpc.http_connect_target']!); - if (parsedTarget) { - realTarget = parsedTarget; - } - } - const targetPath = getDefaultAuthority(realTarget); - const hostPort = splitHostPort(targetPath); - const remoteHost = hostPort?.host ?? targetPath; - connectionOptions.host = remoteHost; - connectionOptions.servername = remoteHost; return connectionOptions; } From 73c5e7af127191f37b8b7fe5c52203030114350b Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 1 Aug 2025 09:50:14 -0700 Subject: [PATCH 11/11] grpc-js: Declare buffer type to avoid build error --- packages/grpc-js/src/compression-filter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/src/compression-filter.ts b/packages/grpc-js/src/compression-filter.ts index 189749f03..e4428a1fb 100644 --- a/packages/grpc-js/src/compression-filter.ts +++ b/packages/grpc-js/src/compression-filter.ts @@ -65,7 +65,7 @@ abstract class CompressionHandler { */ async readMessage(data: Buffer): Promise { const compressed = data.readUInt8(0) === 1; - let messageBuffer = data.slice(5); + let messageBuffer: Buffer = data.slice(5); if (compressed) { messageBuffer = await this.decompressMessage(messageBuffer); }