From 87a35414021f627f01591cade9b1f9a7dcaaf5d3 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 14 May 2024 14:47:53 -0700 Subject: [PATCH] grpc-js: Fix UDS channels not reconnecting after going idle --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/resolver-uds.ts | 2 +- packages/grpc-js/test/common.ts | 51 ++++++++++++++----- packages/grpc-js/test/test-idle-timer.ts | 41 +++++++++++++++ packages/grpc-js/test/test-pick-first.ts | 2 +- .../grpc-js/test/test-server-interceptors.ts | 8 +-- 6 files changed, 86 insertions(+), 20 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index fdfab0d21..f8b070353 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.10.7", + "version": "1.10.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/resolver-uds.ts b/packages/grpc-js/src/resolver-uds.ts index 3a42b18c4..4d84de9d5 100644 --- a/packages/grpc-js/src/resolver-uds.ts +++ b/packages/grpc-js/src/resolver-uds.ts @@ -50,7 +50,7 @@ class UdsResolver implements Resolver { } destroy() { - // This resolver owns no resources, so we do nothing here. + this.hasReturnedResult = false; } static getDefaultAuthority(target: GrpcUri): string { diff --git a/packages/grpc-js/test/common.ts b/packages/grpc-js/test/common.ts index fcdbb4500..5efbf9808 100644 --- a/packages/grpc-js/test/common.ts +++ b/packages/grpc-js/test/common.ts @@ -19,6 +19,8 @@ import * as loader from '@grpc/proto-loader'; import * as assert2 from './assert2'; import * as path from 'path'; import * as grpc from '../src'; +import * as fsPromises from 'fs/promises'; +import * as os from 'os'; import { GrpcObject, @@ -71,54 +73,77 @@ const serviceImpl = { export class TestServer { private server: grpc.Server; - public port: number | null = null; + private target: string | null = null; constructor(public useTls: boolean, options?: grpc.ServerOptions) { this.server = new grpc.Server(options); this.server.addService(echoService.service, serviceImpl); } - start(): Promise { - let credentials: grpc.ServerCredentials; + + private getCredentials(): grpc.ServerCredentials { if (this.useTls) { - credentials = grpc.ServerCredentials.createSsl(null, [ + return grpc.ServerCredentials.createSsl(null, [ { private_key: key, cert_chain: cert }, ]); } else { - credentials = grpc.ServerCredentials.createInsecure(); + return grpc.ServerCredentials.createInsecure(); } + } + + start(): Promise { return new Promise((resolve, reject) => { - this.server.bindAsync('localhost:0', credentials, (error, port) => { + this.server.bindAsync('localhost:0', this.getCredentials(), (error, port) => { if (error) { reject(error); return; } - this.port = port; + this.target = `localhost:${port}`; resolve(); }); }); } + startUds(): Promise { + return fsPromises.mkdtemp(path.join(os.tmpdir(), 'uds')).then(dir => { + return new Promise((resolve, reject) => { + const target = `unix://${dir}/socket`; + this.server.bindAsync(target, this.getCredentials(), (error, port) => { + if (error) { + reject(error); + return; + } + this.target = target; + resolve(); + }); + }); + }); + } + shutdown() { this.server.forceShutdown(); } + + getTarget() { + if (this.target === null) { + throw new Error('Server not yet started'); + } + return this.target; + } } export class TestClient { private client: ServiceClient; - constructor(port: number, useTls: boolean, options?: grpc.ChannelOptions) { + constructor(target: string, useTls: boolean, options?: grpc.ChannelOptions) { let credentials: grpc.ChannelCredentials; if (useTls) { credentials = grpc.credentials.createSsl(ca); } else { credentials = grpc.credentials.createInsecure(); } - this.client = new echoService(`localhost:${port}`, credentials, options); + this.client = new echoService(target, credentials, options); } static createFromServer(server: TestServer, options?: grpc.ChannelOptions) { - if (server.port === null) { - throw new Error('Cannot create client, server not started'); - } - return new TestClient(server.port, server.useTls, options); + return new TestClient(server.getTarget(), server.useTls, options); } waitForReady(deadline: grpc.Deadline, callback: (error?: Error) => void) { diff --git a/packages/grpc-js/test/test-idle-timer.ts b/packages/grpc-js/test/test-idle-timer.ts index a8f457e3f..3f2a8ed20 100644 --- a/packages/grpc-js/test/test-idle-timer.ts +++ b/packages/grpc-js/test/test-idle-timer.ts @@ -129,6 +129,47 @@ describe('Channel idle timer', () => { }); }); +describe('Channel idle timer with UDS', () => { + let server: TestServer; + let client: TestClient | null = null; + before(() => { + server = new TestServer(false); + return server.startUds(); + }); + afterEach(() => { + if (client) { + client.close(); + client = null; + } + }); + after(() => { + server.shutdown(); + }); + it('Should be able to make a request after going idle', function (done) { + this.timeout(5000); + client = TestClient.createFromServer(server, { + 'grpc.client_idle_timeout_ms': 1000, + }); + client.sendRequest(error => { + assert.ifError(error); + assert.strictEqual( + client!.getChannelState(), + grpc.connectivityState.READY + ); + setTimeout(() => { + assert.strictEqual( + client!.getChannelState(), + grpc.connectivityState.IDLE + ); + client!.sendRequest(error => { + assert.ifError(error); + done(); + }); + }, 1100); + }); + }); +}); + describe('Server idle timer', () => { let server: TestServer; let client: TestClient | null = null; diff --git a/packages/grpc-js/test/test-pick-first.ts b/packages/grpc-js/test/test-pick-first.ts index 4c2c319e1..9803a5853 100644 --- a/packages/grpc-js/test/test-pick-first.ts +++ b/packages/grpc-js/test/test-pick-first.ts @@ -811,7 +811,7 @@ describe('pick_first load balancing policy', () => { before(async () => { server = new TestServer(false); await server.start(); - client = new TestClient(server.port!, false, { + client = TestClient.createFromServer(server, { 'grpc.service_config': JSON.stringify(serviceConfig), }); }); diff --git a/packages/grpc-js/test/test-server-interceptors.ts b/packages/grpc-js/test/test-server-interceptors.ts index e94169721..5d4038599 100644 --- a/packages/grpc-js/test/test-server-interceptors.ts +++ b/packages/grpc-js/test/test-server-interceptors.ts @@ -153,7 +153,7 @@ describe('Server interceptors', () => { grpc.ServerCredentials.createInsecure(), (error, port) => { assert.ifError(error); - client = new TestClient(port, false); + client = new TestClient(`localhost:${port}`, false); done(); } ); @@ -195,7 +195,7 @@ describe('Server interceptors', () => { grpc.ServerCredentials.createInsecure(), (error, port) => { assert.ifError(error); - client = new TestClient(port, false); + client = new TestClient(`localhost:${port}`, false); done(); } ); @@ -246,7 +246,7 @@ describe('Server interceptors', () => { grpc.ServerCredentials.createInsecure(), (error, port) => { assert.ifError(error); - client = new TestClient(port, false); + client = new TestClient(`localhost:${port}`, false); done(); } ); @@ -292,7 +292,7 @@ describe('Server interceptors', () => { grpc.ServerCredentials.createInsecure(), (error, port) => { assert.ifError(error); - client = new TestClient(port, false); + client = new TestClient(`localhost:${port}`, false); done(); } );