diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 1c4baede7..f9f5629d5 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.8.19", + "version": "1.8.21", "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/server.ts b/packages/grpc-js/src/server.ts index f7d580b92..c664e1824 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -73,7 +73,6 @@ import { unregisterChannelzRef, } from './channelz'; import { CipherNameAndProtocol, TLSSocket } from 'tls'; -import { getErrorCode, getErrorMessage } from './error'; const UNLIMITED_CONNECTION_AGE_MS = ~(1 << 31); const KEEPALIVE_MAX_TIME_MS = ~(1 << 31); @@ -846,11 +845,7 @@ export class Server { return true; } - private _retrieveHandler( - headers: http2.IncomingHttpHeaders - ): Handler { - const path = headers[HTTP2_HEADER_PATH] as string; - + private _retrieveHandler(path: string): Handler | null { this.trace( 'Received call to method ' + path + @@ -866,7 +861,7 @@ export class Server { path + '. Sending UNIMPLEMENTED status.' ); - throw getUnimplementedStatusResponse(path); + return null; } return handler; @@ -908,15 +903,12 @@ export class Server { return; } - let handler: Handler; - try { - handler = this._retrieveHandler(headers); - } catch (err) { + const path = headers[HTTP2_HEADER_PATH] as string; + + const handler = this._retrieveHandler(path); + if (!handler) { this._respondWithError( - { - details: getErrorMessage(err), - code: getErrorCode(err) ?? undefined, - }, + getUnimplementedStatusResponse(path), stream, channelzSessionInfo ); @@ -970,15 +962,12 @@ export class Server { return; } - let handler: Handler; - try { - handler = this._retrieveHandler(headers); - } catch (err) { + const path = headers[HTTP2_HEADER_PATH] as string; + + const handler = this._retrieveHandler(path); + if (!handler) { this._respondWithError( - { - details: getErrorMessage(err), - code: getErrorCode(err) ?? undefined, - }, + getUnimplementedStatusResponse(path), stream, null ); diff --git a/packages/grpc-js/src/transport.ts b/packages/grpc-js/src/transport.ts index aa17161fd..c55a170b8 100644 --- a/packages/grpc-js/src/transport.ts +++ b/packages/grpc-js/src/transport.ts @@ -153,6 +153,18 @@ class Http2Transport implements Transport { */ private remoteName: string | null ) { + /* Populate subchannelAddressString and channelzRef before doing anything + * else, because they are used in the trace methods. */ + this.subchannelAddressString = subchannelAddressToString(subchannelAddress); + + if (options['grpc.enable_channelz'] === 0) { + this.channelzEnabled = false; + } + this.channelzRef = registerChannelzSocket( + this.subchannelAddressString, + () => this.getChannelzInfo(), + this.channelzEnabled + ); // Build user-agent string. this.userAgent = [ options['grpc.primary_user_agent'], @@ -174,20 +186,6 @@ class Http2Transport implements Transport { } else { this.keepaliveWithoutCalls = false; } - if (this.keepaliveWithoutCalls) { - this.maybeStartKeepalivePingTimer(); - } - - this.subchannelAddressString = subchannelAddressToString(subchannelAddress); - - if (options['grpc.enable_channelz'] === 0) { - this.channelzEnabled = false; - } - this.channelzRef = registerChannelzSocket( - this.subchannelAddressString, - () => this.getChannelzInfo(), - this.channelzEnabled - ); session.once('close', () => { this.trace('session closed'); @@ -233,6 +231,11 @@ class Http2Transport implements Transport { ); }); } + /* Start the keepalive timer last, because this can trigger trace logs, + * which should only happen after everything else is set up. */ + if (this.keepaliveWithoutCalls) { + this.maybeStartKeepalivePingTimer(); + } } private getChannelzInfo(): SocketInfo { diff --git a/packages/grpc-js/test/test-server.ts b/packages/grpc-js/test/test-server.ts index b01712442..2a7a48181 100644 --- a/packages/grpc-js/test/test-server.ts +++ b/packages/grpc-js/test/test-server.ts @@ -430,6 +430,7 @@ describe('Server', () => { (error: ServiceError, response: any) => { assert(error); assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED); + assert.match(error.details, /does not implement the method.*Div/); done(); } ); @@ -439,6 +440,7 @@ describe('Server', () => { const call = client.sum((error: ServiceError, response: any) => { assert(error); assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED); + assert.match(error.details, /does not implement the method.*Sum/); done(); }); @@ -455,6 +457,7 @@ describe('Server', () => { call.on('error', (err: ServiceError) => { assert(err); assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED); + assert.match(err.details, /does not implement the method.*Fib/); done(); }); }); @@ -469,6 +472,93 @@ describe('Server', () => { call.on('error', (err: ServiceError) => { assert(err); assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED); + assert.match(err.details, /does not implement the method.*DivMany/); + done(); + }); + + call.end(); + }); + }); + + describe('Unregistered service', () => { + let server: Server; + let client: ServiceClient; + + const mathProtoFile = path.join(__dirname, 'fixtures', 'math.proto'); + const mathClient = (loadProtoFile(mathProtoFile).math as any).Math; + + before(done => { + server = new Server(); + // Don't register a service at all + server.bindAsync( + 'localhost:0', + ServerCredentials.createInsecure(), + (err, port) => { + assert.ifError(err); + client = new mathClient( + `localhost:${port}`, + grpc.credentials.createInsecure() + ); + server.start(); + done(); + } + ); + }); + + after(done => { + client.close(); + server.tryShutdown(done); + }); + + it('should respond to a unary call with UNIMPLEMENTED', done => { + client.div( + { divisor: 4, dividend: 3 }, + (error: ServiceError, response: any) => { + assert(error); + assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED); + assert.match(error.details, /does not implement the method.*Div/); + done(); + } + ); + }); + + it('should respond to a client stream with UNIMPLEMENTED', done => { + const call = client.sum((error: ServiceError, response: any) => { + assert(error); + assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED); + assert.match(error.details, /does not implement the method.*Sum/); + done(); + }); + + call.end(); + }); + + it('should respond to a server stream with UNIMPLEMENTED', done => { + const call = client.fib({ limit: 5 }); + + call.on('data', (value: any) => { + assert.fail('No messages expected'); + }); + + call.on('error', (err: ServiceError) => { + assert(err); + assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED); + assert.match(err.details, /does not implement the method.*Fib/); + done(); + }); + }); + + it('should respond to a bidi call with UNIMPLEMENTED', done => { + const call = client.divMany(); + + call.on('data', (value: any) => { + assert.fail('No messages expected'); + }); + + call.on('error', (err: ServiceError) => { + assert(err); + assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED); + assert.match(err.details, /does not implement the method.*DivMany/); done(); });