From 4e7b94ab3596179a9d161fbe4abf0f77e9ad2fc3 Mon Sep 17 00:00:00 2001 From: Richard Willis Date: Sat, 18 Apr 2020 09:16:41 +0100 Subject: [PATCH 1/3] grpc-js: Remove watcher from queue before calling watcher callback. Fixes #1352 In the case where a new watcher is synchronously added to the watcher queue via the watcher callback, this can result in the callback being called multiple times. To support this case, the watcher needs to be move removed from the queue before calling the watcher callback. --- 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 4d07dde03..2c8906c4d 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -381,9 +381,9 @@ export class ChannelImplementation implements Channel { const watchersCopy = this.connectivityStateWatchers.slice(); for (const watcherObject of watchersCopy) { if (newState !== watcherObject.currentState) { - watcherObject.callback(); clearTimeout(watcherObject.timer); this.removeConnectivityStateWatcher(watcherObject); + watcherObject.callback(); } } } From 615a3c65b14e965a1d130a02005807dd122c9091 Mon Sep 17 00:00:00 2001 From: Richard Willis Date: Sat, 18 Apr 2020 09:19:45 +0100 Subject: [PATCH 2/3] grpc-js: Add test for client.waitForReady. Refs #1352 --- packages/grpc-js/test/test-client.ts | 81 ++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 packages/grpc-js/test/test-client.ts diff --git a/packages/grpc-js/test/test-client.ts b/packages/grpc-js/test/test-client.ts new file mode 100644 index 000000000..e783b715c --- /dev/null +++ b/packages/grpc-js/test/test-client.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2019 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Allow `any` data type for testing runtime type checking. +// tslint:disable no-any +import * as assert from 'assert'; +import * as path from 'path'; + +import * as grpc from '../src'; +import { Server, ServerCredentials } from '../src'; +import { ServiceClient, ServiceClientConstructor } from '../src/make-client'; +import { sendUnaryData, ServerUnaryCall } from '../src/server-call'; + +import { loadProtoFile } from './common'; +import { ConnectivityState } from '../src/channel'; + +const clientInsecureCreds = grpc.credentials.createInsecure(); +const serverInsecureCreds = ServerCredentials.createInsecure(); + +describe('Client', () => { + let server: Server; + let client: ServiceClient; + + before(done => { + const protoFile = path.join(__dirname, 'fixtures', 'echo_service.proto'); + const echoService = loadProtoFile(protoFile) + .EchoService as ServiceClientConstructor; + + server = new Server(); + + server.bindAsync( + 'localhost:0', + serverInsecureCreds, + (err, port) => { + assert.ifError(err); + client = new echoService( + `localhost:${port}`, + clientInsecureCreds + ); + server.start(); + done(); + } + ); + }); + + after(done => { + client.close(); + server.tryShutdown(done); + }); + + it('should call the waitForReady callback only once when channel connectivity state is READY', done => { + const deadline = Date.now() + 100; + let calledTimes = 0; + client.waitForReady(deadline, err => { + assert.ifError(err); + assert.equal( + client.getChannel().getConnectivityState(true), + ConnectivityState.READY + ); + calledTimes += 1; + }); + setTimeout(() => { + assert.equal(calledTimes, 1); + done(); + }, deadline - Date.now()); + }); +}); From 7e381f7f2a6559e8fb3ef403de5b74cb000b6e12 Mon Sep 17 00:00:00 2001 From: Richard Willis Date: Mon, 20 Apr 2020 19:12:32 +0100 Subject: [PATCH 3/3] grpc-js: Simplify client.waitForReady tests. Refs #1352 No need to add a service to the server to test the client. --- packages/grpc-js/test/test-client.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/packages/grpc-js/test/test-client.ts b/packages/grpc-js/test/test-client.ts index e783b715c..da20c246b 100644 --- a/packages/grpc-js/test/test-client.ts +++ b/packages/grpc-js/test/test-client.ts @@ -15,17 +15,11 @@ * */ -// Allow `any` data type for testing runtime type checking. -// tslint:disable no-any import * as assert from 'assert'; -import * as path from 'path'; import * as grpc from '../src'; import { Server, ServerCredentials } from '../src'; -import { ServiceClient, ServiceClientConstructor } from '../src/make-client'; -import { sendUnaryData, ServerUnaryCall } from '../src/server-call'; - -import { loadProtoFile } from './common'; +import { Client } from '../src'; import { ConnectivityState } from '../src/channel'; const clientInsecureCreds = grpc.credentials.createInsecure(); @@ -33,13 +27,9 @@ const serverInsecureCreds = ServerCredentials.createInsecure(); describe('Client', () => { let server: Server; - let client: ServiceClient; + let client: Client; before(done => { - const protoFile = path.join(__dirname, 'fixtures', 'echo_service.proto'); - const echoService = loadProtoFile(protoFile) - .EchoService as ServiceClientConstructor; - server = new Server(); server.bindAsync( @@ -47,7 +37,7 @@ describe('Client', () => { serverInsecureCreds, (err, port) => { assert.ifError(err); - client = new echoService( + client = new Client( `localhost:${port}`, clientInsecureCreds ); @@ -62,7 +52,7 @@ describe('Client', () => { server.tryShutdown(done); }); - it('should call the waitForReady callback only once when channel connectivity state is READY', done => { + it('should call the waitForReady callback only once, when channel connectivity state is READY', done => { const deadline = Date.now() + 100; let calledTimes = 0; client.waitForReady(deadline, err => {