Merge pull request #2521 from murgatroid99/v1.8.x_upmerge_3

Merge 1.8.x into master

Author: murgatroid99

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",

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<any, any> {
-    const path = headers[HTTP2_HEADER_PATH] as string;
-
+  private _retrieveHandler(path: string): Handler<any, any> | 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<any, any>;
-    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<any, any>;
-    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
       );

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 {

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();
       });