From 259e00b86623bc8a30fda6cf4cee86a293135935 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 3 Feb 2021 11:54:13 -0800 Subject: [PATCH 01/15] grpc-js: Loosen dependency on @types/node --- packages/grpc-js/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 01a9d418..7e7f4587 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.5", + "version": "1.2.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", @@ -57,7 +57,7 @@ "posttest": "npm run check" }, "dependencies": { - "@types/node": "^12.12.47", + "@types/node": ">=12.12.47", "google-auth-library": "^6.1.1", "semver": "^6.2.0" }, From cd14345cb429432e341a068c8bc3f940ccc6672c Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 11 Feb 2021 09:55:24 -0800 Subject: [PATCH 02/15] grpc-js: Ref and unref backoff timer --- packages/grpc-js-xds/package.json | 4 ++-- packages/grpc-js-xds/src/xds-client.ts | 4 +++- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/backoff-timeout.ts | 14 ++++++++++++++ packages/grpc-js/src/resolving-load-balancer.ts | 1 + packages/grpc-js/src/subchannel.ts | 2 ++ 6 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json index f2b32a77..5ff82e49 100644 --- a/packages/grpc-js-xds/package.json +++ b/packages/grpc-js-xds/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js-xds", - "version": "1.2.1", + "version": "1.2.2", "description": "Plugin for @grpc/grpc-js. Adds the xds:// URL scheme and associated features.", "main": "build/src/index.js", "scripts": { @@ -45,7 +45,7 @@ "@grpc/proto-loader": "^0.6.0-pre14" }, "peerDependencies": { - "@grpc/grpc-js": "~1.2.2" + "@grpc/grpc-js": "~1.2.7" }, "engines": { "node": ">=10.10.0" diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index ae67960d..7f658657 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -774,9 +774,11 @@ export class XdsClient { this.adsBackoff = new BackoffTimeout(() => { this.maybeStartAdsStream(); }); + this.adsBackoff.unref(); this.lrsBackoff = new BackoffTimeout(() => { this.maybeStartLrsStream(); - }) + }); + this.lrsBackoff.unref(); Promise.all([loadBootstrapInfo(), loadAdsProtos()]).then( ([bootstrapInfo, protoDefinitions]) => { diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 7e7f4587..6b84fd9e 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.6", + "version": "1.2.7", "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/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts index 8cad14f7..49a5b1e2 100644 --- a/packages/grpc-js/src/backoff-timeout.ts +++ b/packages/grpc-js/src/backoff-timeout.ts @@ -44,6 +44,7 @@ export class BackoffTimeout { private nextDelay: number; private timerId: NodeJS.Timer; private running = false; + private hasRef = true; constructor(private callback: () => void, options?: BackoffOptions) { if (options) { @@ -74,6 +75,9 @@ export class BackoffTimeout { this.callback(); this.running = false; }, this.nextDelay); + if (!this.hasRef) { + this.timerId.unref(); + } const nextBackoff = Math.min( this.nextDelay * this.multiplier, this.maxDelay @@ -102,4 +106,14 @@ export class BackoffTimeout { isRunning() { return this.running; } + + ref() { + this.hasRef = true; + this.timerId.ref(); + } + + unref() { + this.hasRef = false; + this.timerId.unref(); + } } diff --git a/packages/grpc-js/src/resolving-load-balancer.ts b/packages/grpc-js/src/resolving-load-balancer.ts index 5a4c62f5..ca6d0721 100644 --- a/packages/grpc-js/src/resolving-load-balancer.ts +++ b/packages/grpc-js/src/resolving-load-balancer.ts @@ -196,6 +196,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { this.updateState(this.latestChildState, this.latestChildPicker); } }); + this.backoffTimeout.unref(); } private updateResolution() { diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index c32ee43e..30959122 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -615,6 +615,7 @@ export class Subchannel { if (this.session) { this.session.ref(); } + this.backoffTimeout.ref(); if (!this.keepaliveWithoutCalls) { this.startKeepalivePings(); } @@ -635,6 +636,7 @@ export class Subchannel { if (this.session) { this.session.unref(); } + this.backoffTimeout.unref(); if (!this.keepaliveWithoutCalls) { this.stopKeepalivePings(); } From 097d63b14b9621ddd8ccfb180c4eaefe144a8851 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 12 Feb 2021 11:03:00 -0800 Subject: [PATCH 03/15] grpc-js: Add more details to 'Failed to start HTTP/2 stream' error --- packages/grpc-js/src/channel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index e1a76c09..ad24876e 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -362,7 +362,7 @@ export class ChannelImplementation implements Channel { ); callStream.cancelWithStatus( Status.INTERNAL, - 'Failed to start HTTP/2 stream' + `Failed to start HTTP/2 stream with error: ${(error as Error).message}` ); } } From 40c19ea28be67d05f11a431a4279505dfedba265 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 16 Feb 2021 12:42:10 -0800 Subject: [PATCH 04/15] grpc-js: Don't propagate non-numeric errors from auth plugins --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/call-credentials-filter.ts | 12 +++++++++++- packages/grpc-js/src/channel.ts | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 6b84fd9e..638c1920 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.7", + "version": "1.2.8", "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/call-credentials-filter.ts b/packages/grpc-js/src/call-credentials-filter.ts index 5263d97f..53bdba2f 100644 --- a/packages/grpc-js/src/call-credentials-filter.ts +++ b/packages/grpc-js/src/call-credentials-filter.ts @@ -21,6 +21,7 @@ import { BaseFilter, Filter, FilterFactory } from './filter'; import { Metadata } from './metadata'; import { Status } from './constants'; import { splitHostPort } from './uri-parser'; +import { ServiceError } from './call'; export class CallCredentialsFilter extends BaseFilter implements Filter { private serviceUrl: string; @@ -51,12 +52,21 @@ export class CallCredentialsFilter extends BaseFilter implements Filter { service_url: this.serviceUrl, }); const resultMetadata = await metadata; - resultMetadata.merge(await credsMetadata); + try { + resultMetadata.merge(await credsMetadata); + } catch (error) { + this.stream.cancelWithStatus( + Status.UNAUTHENTICATED, + `Failed to retrieve auth metadata with error: ${error.message}` + ); + return Promise.reject('Failed to retrieve auth metadata'); + } if (resultMetadata.get('authorization').length > 1) { this.stream.cancelWithStatus( Status.INTERNAL, '"authorization" metadata cannot have multiple values' ); + return Promise.reject('"authorization" metadata cannot have multiple values'); } return resultMetadata; } diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index ad24876e..0f055d1e 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -384,7 +384,7 @@ export class ChannelImplementation implements Channel { (error: Error & { code: number }) => { // We assume the error code isn't 0 (Status.OK) callStream.cancelWithStatus( - error.code || Status.UNKNOWN, + (typeof error.code === 'number') ? error.code : Status.UNKNOWN, `Getting metadata from plugin failed with error: ${error.message}` ); } From c5cc8b2652b4ec8ccde978f1ef4b954a80e92df0 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 2 Mar 2021 11:55:42 -0800 Subject: [PATCH 05/15] grpc-js: Speculative fix for ECONNRESET errors --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/call-stream.ts | 53 +++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 638c1920..8fffe4be 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.8", + "version": "1.2.9", "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/call-stream.ts b/packages/grpc-js/src/call-stream.ts index 9c27aeb1..a51c1da2 100644 --- a/packages/grpc-js/src/call-stream.ts +++ b/packages/grpc-js/src/call-stream.ts @@ -16,6 +16,7 @@ */ import * as http2 from 'http2'; +import * as os from 'os'; import { CallCredentials } from './call-credentials'; import { Propagate, Status } from './constants'; @@ -37,8 +38,34 @@ const { NGHTTP2_CANCEL, } = http2.constants; -interface NodeError extends Error { +/** + * https://nodejs.org/api/errors.html#errors_class_systemerror + */ +interface SystemError extends Error { + address?: string; code: string; + dest?: string; + errno: number; + info?: object; + message: string; + path?: string; + port?: number; + syscall: string; +} + +/** + * Should do approximately the same thing as util.getSystemErrorName but the + * TypeScript types don't have that function for some reason so I just made my + * own. + * @param errno + */ +function getSystemErrorName(errno: number): string { + for (const [name, num] of Object.entries(os.constants.errno)) { + if (num === errno) { + return name; + } + } + return 'Unknown system error ' + errno; } export type Deadline = Date | number; @@ -206,7 +233,7 @@ export class Http2CallStream implements Call { private listener: InterceptingListener | null = null; - private internalErrorMessage: string | null = null; + private internalError: SystemError | null = null; constructor( private readonly methodName: string, @@ -567,7 +594,7 @@ export class Http2CallStream implements Call { break; case http2.constants.NGHTTP2_INTERNAL_ERROR: code = Status.INTERNAL; - if (this.internalErrorMessage === null) { + if (this.internalError === null) { /* This error code was previously handled in the default case, and * there are several instances of it online, so I wanted to * preserve the original error message so that people find existing @@ -575,11 +602,16 @@ export class Http2CallStream implements Call { * "Internal server error" message. */ details = `Received RST_STREAM with code ${stream.rstCode} (Internal server error)`; } else { - /* The "Received RST_STREAM with code ..." error is preserved - * here for continuity with errors reported online, but the - * error message at the end will probably be more relevant in - * most cases. */ - details = `Received RST_STREAM with code ${stream.rstCode} triggered by internal client error: ${this.internalErrorMessage}`; + if (this.internalError.errno === os.constants.errno.ECONNRESET) { + code = Status.UNAVAILABLE; + details = this.internalError.message; + } else { + /* The "Received RST_STREAM with code ..." error is preserved + * here for continuity with errors reported online, but the + * error message at the end will probably be more relevant in + * most cases. */ + details = `Received RST_STREAM with code ${stream.rstCode} triggered by internal client error: ${this.internalError.message}`; + } } break; default: @@ -593,7 +625,7 @@ export class Http2CallStream implements Call { this.endCall({ code, details, metadata: new Metadata() }); }); }); - stream.on('error', (err: NodeError) => { + stream.on('error', (err: SystemError) => { /* We need an error handler here to stop "Uncaught Error" exceptions * from bubbling up. However, errors here should all correspond to * "close" events, where we will handle the error more granularly */ @@ -602,7 +634,8 @@ export class Http2CallStream implements Call { * https://github.com/nodejs/node/blob/8b8620d580314050175983402dfddf2674e8e22a/lib/internal/http2/core.js#L2267 */ if (err.code !== 'ERR_HTTP2_STREAM_ERROR') { - this.internalErrorMessage = err.message; + this.trace('Node error event: message=' + err.message + ' code=' + err.code + ' errno=' + getSystemErrorName(err.errno) + ' syscall=' + err.syscall); + this.internalError = err; } }); if (!this.pendingRead) { From 356518a212355f90dd1248a2f6c7565771078ecc Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Mar 2021 13:56:57 -0800 Subject: [PATCH 06/15] grpc-js-xds: Fix handling of empty LRS server names --- packages/grpc-js-xds/package.json | 2 +- packages/grpc-js-xds/scripts/xds.sh | 2 +- packages/grpc-js-xds/src/load-balancer-eds.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json index 5ff82e49..b5c9d3f1 100644 --- a/packages/grpc-js-xds/package.json +++ b/packages/grpc-js-xds/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js-xds", - "version": "1.2.2", + "version": "1.2.3", "description": "Plugin for @grpc/grpc-js. Adds the xds:// URL scheme and associated features.", "main": "build/src/index.js", "scripts": { diff --git a/packages/grpc-js-xds/scripts/xds.sh b/packages/grpc-js-xds/scripts/xds.sh index bbfc3056..4629f42a 100644 --- a/packages/grpc-js-xds/scripts/xds.sh +++ b/packages/grpc-js-xds/scripts/xds.sh @@ -52,7 +52,7 @@ GRPC_NODE_TRACE=xds_client,xds_resolver,cds_balancer,eds_balancer,priority,weigh GRPC_NODE_VERBOSITY=DEBUG \ NODE_XDS_INTEROP_VERBOSITY=1 \ python3 grpc/tools/run_tests/run_xds_tests.py \ - --test_case="backends_restart,change_backend_service,gentle_failover,ping_pong,remove_instance_group,round_robin,secondary_locality_gets_no_requests_on_partial_primary_failure,secondary_locality_gets_requests_on_primary_failure" \ + --test_case="backends_restart,change_backend_service,gentle_failover,ping_pong,remove_instance_group,round_robin,secondary_locality_gets_no_requests_on_partial_primary_failure,secondary_locality_gets_requests_on_primary_failure,load_report_based_failover" \ --project_id=grpc-testing \ --source_image=projects/grpc-testing/global/images/xds-test-server-2 \ --path_to_server_binary=/java_server/grpc-java/interop-testing/build/install/grpc-interop-testing/bin/xds-test-server \ diff --git a/packages/grpc-js-xds/src/load-balancer-eds.ts b/packages/grpc-js-xds/src/load-balancer-eds.ts index 8919f317..93cc6421 100644 --- a/packages/grpc-js-xds/src/load-balancer-eds.ts +++ b/packages/grpc-js-xds/src/load-balancer-eds.ts @@ -377,7 +377,7 @@ export class EdsLoadBalancer implements LoadBalancer { validateLoadBalancingConfig({ round_robin: {} }), ]; let childPolicy: LoadBalancingConfig[]; - if (this.lastestConfig.getLrsLoadReportingServerName()) { + if (this.lastestConfig.getLrsLoadReportingServerName() !== undefined) { childPolicy = [new LrsLoadBalancingConfig(this.lastestConfig.getCluster(), this.lastestConfig.getEdsServiceName() ?? '', this.lastestConfig.getLrsLoadReportingServerName()!, localityObj.locality, endpointPickingPolicy)]; } else { childPolicy = endpointPickingPolicy; From 1702014385d00ec1c58fd6f91d35184414a6064d Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Mar 2021 14:01:59 -0800 Subject: [PATCH 07/15] Borrow Linux Kokoro job for xDS testing --- test/kokoro/linux.cfg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/kokoro/linux.cfg b/test/kokoro/linux.cfg index f40e6db4..16a7dc7d 100644 --- a/test/kokoro/linux.cfg +++ b/test/kokoro/linux.cfg @@ -15,10 +15,10 @@ # Config file for Kokoro (in protobuf text format) # Location of the continuous shell script in repository. -build_file: "grpc-node/test/kokoro.sh" -timeout_mins: 60 +build_file: "grpc-node/packages/grpc-js-xds/scripts/xds.sh" +timeout_mins: 120 action { define_artifacts { - regex: "github/grpc-node/reports/**/sponge_log.xml" + regex: "github/grpc/reports/**" } } From b85c70839f4494371d586d463cd2f877c448d0d9 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Mar 2021 11:50:30 -0800 Subject: [PATCH 08/15] Add more detailed LRS tracing --- packages/grpc-js-xds/src/xds-client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index 7f658657..f147da0e 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -1068,7 +1068,6 @@ export class XdsClient { if (!this.lrsCall) { return; } - trace('Sending LRS stats'); const clusterStats: ClusterStats[] = []; for (const [ { clusterName, edsServiceName }, @@ -1129,6 +1128,7 @@ export class XdsClient { } } } + trace('Sending LRS stats ' + JSON.stringify(clusterStats, undefined, 2)); this.lrsCall.write({ node: this.lrsNode!, cluster_stats: clusterStats, From e3b35505a0aff40b8a1adabafb5a5443bf7ce7d9 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Mar 2021 13:53:02 -0800 Subject: [PATCH 09/15] Fix handling of LRS server name in EDS child config generation Also add more LRS logging --- packages/grpc-js-xds/src/load-balancer-lrs.ts | 1 - packages/grpc-js-xds/src/xds-client.ts | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/src/load-balancer-lrs.ts b/packages/grpc-js-xds/src/load-balancer-lrs.ts index b6fa6809..5f860b17 100644 --- a/packages/grpc-js-xds/src/load-balancer-lrs.ts +++ b/packages/grpc-js-xds/src/load-balancer-lrs.ts @@ -16,7 +16,6 @@ */ import { connectivityState as ConnectivityState, StatusObject, status as Status, experimental } from '@grpc/grpc-js'; -import { type } from 'os'; import { Locality__Output } from './generated/envoy/api/v2/core/Locality'; import { XdsClusterLocalityStats, XdsClient } from './xds-client'; import LoadBalancer = experimental.LoadBalancer; diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index f147da0e..56ddddf9 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -1174,6 +1174,7 @@ export class XdsClient { clusterName: string, edsServiceName: string ): XdsClusterDropStats { + trace('addClusterDropStats(lrsServer=' + lrsServer + ', clusterName=' + clusterName + ', edsServiceName=' + edsServiceName + ')'); if (lrsServer !== '') { return { addCallDropped: (category) => {}, @@ -1197,6 +1198,7 @@ export class XdsClient { edsServiceName: string, locality: Locality__Output ): XdsClusterLocalityStats { + trace('addClusterLocalityStats(lrsServer=' + lrsServer + ', clusterName=' + clusterName + ', edsServiceName=' + edsServiceName + ', locality=' + JSON.stringify(locality) + ')'); if (lrsServer !== '') { return { addCallStarted: () => {}, From efc9a0f05c9e2dbbfe2758e6df8cb5527903cc9f Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Mar 2021 16:34:53 -0800 Subject: [PATCH 10/15] Don't send status through the filter stack twice when receiving trailers --- packages/grpc-js-xds/package.json | 2 +- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/call-stream.ts | 15 +-------------- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json index b5c9d3f1..6ff85816 100644 --- a/packages/grpc-js-xds/package.json +++ b/packages/grpc-js-xds/package.json @@ -45,7 +45,7 @@ "@grpc/proto-loader": "^0.6.0-pre14" }, "peerDependencies": { - "@grpc/grpc-js": "~1.2.7" + "@grpc/grpc-js": "~1.2.10" }, "engines": { "node": ">=10.10.0" diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 8fffe4be..939827b2 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.9", + "version": "1.2.10", "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/call-stream.ts b/packages/grpc-js/src/call-stream.ts index a51c1da2..8fcc7065 100644 --- a/packages/grpc-js/src/call-stream.ts +++ b/packages/grpc-js/src/call-stream.ts @@ -442,21 +442,8 @@ export class Http2CallStream implements Call { ); } const status: StatusObject = { code, details, metadata }; - let finalStatus; - try { - // Attempt to assign final status. - finalStatus = this.filterStack.receiveTrailers(status); - } catch (error) { - // This is a no-op if the call was already ended when handling headers. - this.endCall({ - code: Status.INTERNAL, - details: 'Failed to process received status', - metadata: new Metadata(), - }); - return; - } // This is a no-op if the call was already ended when handling headers. - this.endCall(finalStatus); + this.endCall(status); } attachHttp2Stream( From 31074190ff01cc9c05ff103a702f9f550ee70ce5 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Mar 2021 18:31:18 -0800 Subject: [PATCH 11/15] Revert "Borrow Linux Kokoro job for xDS testing" This reverts commit 1702014385d00ec1c58fd6f91d35184414a6064d. --- test/kokoro/linux.cfg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/kokoro/linux.cfg b/test/kokoro/linux.cfg index 16a7dc7d..f40e6db4 100644 --- a/test/kokoro/linux.cfg +++ b/test/kokoro/linux.cfg @@ -15,10 +15,10 @@ # Config file for Kokoro (in protobuf text format) # Location of the continuous shell script in repository. -build_file: "grpc-node/packages/grpc-js-xds/scripts/xds.sh" -timeout_mins: 120 +build_file: "grpc-node/test/kokoro.sh" +timeout_mins: 60 action { define_artifacts { - regex: "github/grpc/reports/**" + regex: "github/grpc-node/reports/**/sponge_log.xml" } } From 231619fcaa7ddd46b23df25c699c271cb92bc67e Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 5 Mar 2021 14:18:50 -0800 Subject: [PATCH 12/15] grpc-js: Timer ref and unref might not exist --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/backoff-timeout.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 939827b2..4413d4d2 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.10", + "version": "1.2.11", "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/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts index 49a5b1e2..7f2ab5eb 100644 --- a/packages/grpc-js/src/backoff-timeout.ts +++ b/packages/grpc-js/src/backoff-timeout.ts @@ -76,7 +76,7 @@ export class BackoffTimeout { this.running = false; }, this.nextDelay); if (!this.hasRef) { - this.timerId.unref(); + this.timerId.unref?.(); } const nextBackoff = Math.min( this.nextDelay * this.multiplier, @@ -109,11 +109,11 @@ export class BackoffTimeout { ref() { this.hasRef = true; - this.timerId.ref(); + this.timerId.ref?.(); } unref() { this.hasRef = false; - this.timerId.unref(); + this.timerId.unref?.(); } } From 2aec366508e8bb434efd081bea2256b1aeb3dc84 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 9 Mar 2021 10:39:42 -0800 Subject: [PATCH 13/15] grpc-js-xds: Fix sending stats when reestablishing LRS stream --- packages/grpc-js-xds/package.json | 2 +- packages/grpc-js-xds/src/xds-client.ts | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json index 6ff85816..875c8645 100644 --- a/packages/grpc-js-xds/package.json +++ b/packages/grpc-js-xds/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js-xds", - "version": "1.2.3", + "version": "1.2.4", "description": "Plugin for @grpc/grpc-js. Adds the xds:// URL scheme and associated features.", "main": "build/src/index.js", "scripts": { diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index 56ddddf9..09420ba9 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -1020,12 +1020,14 @@ export class XdsClient { this.lrsBackoff.runOnce(); this.lrsCall = this.lrsClient.streamLoadStats(); + let receivedSettingsForThisStream = false; this.lrsCall.on('data', (message: LoadStatsResponse__Output) => { /* Once we get any response from the server, we assume that the stream is * in a good state, so we can reset the backoff timer. */ this.lrsBackoff.stop(); this.lrsBackoff.reset(); if ( + !receivedSettingsForThisStream || message.load_reporting_interval?.seconds !== this.latestLrsSettings?.load_reporting_interval?.seconds || message.load_reporting_interval?.nanos !== @@ -1045,13 +1047,13 @@ export class XdsClient { }, loadReportingIntervalMs); } this.latestLrsSettings = message; + receivedSettingsForThisStream = true; }); this.lrsCall.on('error', (error: ServiceError) => { trace( 'LRS stream ended. code=' + error.code + ' details= ' + error.details ); this.lrsCall = null; - this.latestLrsSettings = null; clearInterval(this.statsTimer); /* If the backoff timer is no longer running, we do not need to wait any * more to start the new call. */ @@ -1068,14 +1070,20 @@ export class XdsClient { if (!this.lrsCall) { return; } + if (!this.latestLrsSettings) { + this.lrsCall.write({ + node: this.lrsNode!, + }); + return; + } const clusterStats: ClusterStats[] = []; for (const [ { clusterName, edsServiceName }, stats, ] of this.clusterStatsMap.entries()) { if ( - this.latestLrsSettings!.send_all_clusters || - this.latestLrsSettings!.clusters.indexOf(clusterName) > 0 + this.latestLrsSettings.send_all_clusters || + this.latestLrsSettings.clusters.indexOf(clusterName) > 0 ) { const upstreamLocalityStats: UpstreamLocalityStats[] = []; for (const localityStats of stats.localityStats) { From 602fcd23b4b39b3dba0d75af5ecb688a46b8c7a5 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 12 Mar 2021 09:39:50 -0800 Subject: [PATCH 14/15] grpc-js: Throw in watchConnectivityState if channel is closed --- packages/grpc-js/src/channel.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index 0f055d1e..416aaa08 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -480,6 +480,9 @@ export class ChannelImplementation implements Channel { deadline: Date | number, callback: (error?: Error) => void ): void { + if (this.connectivityState === ConnectivityState.SHUTDOWN) { + throw new Error('Channel has been shut down'); + } let timer = null; if(deadline !== Infinity) { const deadlineDate: Date = From 4623ecca4295dd240d4cd90cb1049a946a9601a1 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 22 Mar 2021 10:47:36 -0700 Subject: [PATCH 15/15] grpc-js: don't send accept-encoding: gzip --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/compression-filter.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 4413d4d2..b87f95f0 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.2.11", + "version": "1.2.12", "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/compression-filter.ts b/packages/grpc-js/src/compression-filter.ts index 6f40ee4b..330eb675 100644 --- a/packages/grpc-js/src/compression-filter.ts +++ b/packages/grpc-js/src/compression-filter.ts @@ -170,7 +170,7 @@ export class CompressionFilter extends BaseFilter implements Filter { async sendMetadata(metadata: Promise): Promise { const headers: Metadata = await metadata; headers.set('grpc-accept-encoding', 'identity,deflate,gzip'); - headers.set('accept-encoding', 'identity,gzip'); + headers.set('accept-encoding', 'identity'); return headers; }