Merge pull request #6322 from murgatroid99/node_service_default_method

Make Node servers warn instead of fail when a method is missing a handler

ext/server_credentials.cc

@@ -146,7 +146,9 @@ NAN_METHOD(ServerCredentials::CreateSsl) {
         "createSsl's second argument must be a list of objects");
   }
 
-  grpc_ssl_client_certificate_request_type client_certificate_request;
+  // Default to not requesting the client certificate
+  grpc_ssl_client_certificate_request_type client_certificate_request =
+      GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE;
   if (info[2]->IsBoolean()) {
     client_certificate_request =
         Nan::To<bool>(info[2]).FromJust()

src/server.js

@@ -684,6 +684,26 @@ Server.prototype.register = function(name, handler, serialize, deserialize,
   return true;
 };
 
+var unimplementedStatusResponse = {
+  code: grpc.status.UNIMPLEMENTED,
+  details: 'The server does not implement this method'
+};
+
+var defaultHandler = {
+  unary: function(call, callback) {
+    callback(unimplementedStatusResponse);
+  },
+  client_stream: function(call, callback) {
+    callback(unimplementedStatusResponse);
+  },
+  server_stream: function(call) {
+    call.emit('error', unimplementedStatusResponse);
+  },
+  bidi: function(call) {
+    call.emit('error', unimplementedStatusResponse);
+  }
+};
+
 /**
  * Add a service to the server, with a corresponding implementation. If you are
  * generating this from a proto file, you should instead use
@@ -713,16 +733,18 @@ Server.prototype.addService = function(service, implementation) {
         method_type = 'unary';
       }
     }
+    var impl;
     if (implementation[name] === undefined) {
-      throw new Error('Method handler for ' + attrs.path +
+      console.warn('Method handler for %s expected but not provided',
-          ' not provided.');
+                   attrs.path);
+      impl = defaultHandler[method_type];
+    } else {
+      impl = _.bind(implementation[name], implementation);
     }
     var serialize = attrs.responseSerialize;
     var deserialize = attrs.requestDeserialize;
-    var register_success = self.register(attrs.path,
+    var register_success = self.register(attrs.path, impl, serialize,
-                                         _.bind(implementation[name],
+                                         deserialize, method_type);
-                                                implementation),
-                                         serialize, deserialize, method_type);
     if (!register_success) {
       throw new Error('Method handler for ' + attrs.path +
           ' already provided.');

test/surface_test.js

@@ -143,21 +143,59 @@ describe('Server.prototype.addProtoService', function() {
       server.addProtoService(mathService, dummyImpls);
     });
   });
-  it('Should fail with missing handlers', function() {
-    assert.throws(function() {
-      server.addProtoService(mathService, {
-        'div': function() {},
-        'divMany': function() {},
-        'fib': function() {}
-      });
-    }, /math.Math.Sum/);
-  });
   it('Should fail if the server has been started', function() {
     server.start();
     assert.throws(function() {
       server.addProtoService(mathService, dummyImpls);
     });
   });
+  describe('Default handlers', function() {
+    var client;
+    beforeEach(function() {
+      server.addProtoService(mathService, {});
+      var port = server.bind('localhost:0', server_insecure_creds);
+      var Client = surface_client.makeProtobufClientConstructor(mathService);
+      client = new Client('localhost:' + port,
+                          grpc.credentials.createInsecure());
+      server.start();
+    });
+    it('should respond to a unary call with UNIMPLEMENTED', function(done) {
+      client.div({divisor: 4, dividend: 3}, function(error, response) {
+        assert(error);
+        assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED);
+        done();
+      });
+    });
+    it('should respond to a client stream with UNIMPLEMENTED', function(done) {
+      var call = client.sum(function(error, respones) {
+        assert(error);
+        assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED);
+        done();
+      });
+      call.end();
+    });
+    it('should respond to a server stream with UNIMPLEMENTED', function(done) {
+      var call = client.fib({limit: 5});
+      call.on('data', function(value) {
+        assert.fail('No messages expected');
+      });
+      call.on('status', function(status) {
+        assert.strictEqual(status.code, grpc.status.UNIMPLEMENTED);
+        done();
+      });
+    });
+    it('should respond to a bidi call with UNIMPLEMENTED', function(done) {
+      var call = client.divMany();
+      call.on('data', function(value) {
+        assert.fail('No messages expected');
+      });
+      call.on('status', function(status) {
+        assert.strictEqual(status.code, grpc.status.UNIMPLEMENTED);
+        done();
+      });
+      call.end();
+    });
+  });
 });
 describe('Client constructor building', function() {
   var illegal_service_attrs = {