From b90dc81b732caa9bfc86298dbd8a46dd158f05ac Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 20 Feb 2020 12:46:59 -0800 Subject: [PATCH 1/3] Include method name in UNIMPLEMENTED details string --- packages/grpc-js/src/server.ts | 66 ++++++++++++++++--------- packages/grpc-native-core/src/server.js | 47 +++++++++++------- 2 files changed, 70 insertions(+), 43 deletions(-) diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index a2db30be..9cddb084 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -49,11 +49,15 @@ import { ChannelOptions } from './channel-options'; function noop(): void {} -const unimplementedStatusResponse: Partial = { - code: Status.UNIMPLEMENTED, - details: 'The server does not implement this method', - metadata: new Metadata(), -}; +function getUnimplementedStatusResponse( + methodName: string +): Partial { + return { + code: Status.UNIMPLEMENTED, + details: `The server does not implement the method ${methodName}`, + metadata: new Metadata(), + }; +} // tslint:disable:no-any type UntypedUnaryHandler = UnaryHandler; @@ -66,23 +70,37 @@ export interface UntypedServiceImplementation { [name: string]: UntypedHandleCall; } -const defaultHandler = { - unary(call: ServerUnaryCall, callback: sendUnaryData): void { - callback(unimplementedStatusResponse as ServiceError, null); - }, - clientStream( - call: ServerReadableStream, - callback: sendUnaryData - ): void { - callback(unimplementedStatusResponse as ServiceError, null); - }, - serverStream(call: ServerWritableStream): void { - call.emit('error', unimplementedStatusResponse); - }, - bidi(call: ServerDuplexStream): void { - call.emit('error', unimplementedStatusResponse); - }, -}; +function getDefaultHandler(handlerType: HandlerType, methodName: string) { + const unimplementedStatusResponse = getUnimplementedStatusResponse( + methodName + ); + switch (handlerType) { + case 'unary': + return ( + call: ServerUnaryCall, + callback: sendUnaryData + ) => { + callback(unimplementedStatusResponse as ServiceError, null); + }; + case 'clientStream': + return ( + call: ServerReadableStream, + callback: sendUnaryData + ) => { + callback(unimplementedStatusResponse as ServiceError, null); + }; + case 'serverStream': + return (call: ServerWritableStream) => { + call.emit('error', unimplementedStatusResponse); + }; + case 'bidi': + return (call: ServerDuplexStream) => { + call.emit('error', unimplementedStatusResponse); + }; + default: + throw new Error(`Invalid handlerType ${handlerType}`); + } +} // tslint:enable:no-any export class Server { @@ -157,7 +175,7 @@ export class Server { if (implFn !== undefined) { impl = implFn.bind(implementation); } else { - impl = defaultHandler[methodType]; + impl = getDefaultHandler(methodType, name); } const success = this.register( @@ -353,7 +371,7 @@ export class Server { const handler = this.handlers.get(path); if (handler === undefined) { - throw unimplementedStatusResponse; + throw getUnimplementedStatusResponse(path); } const call = new Http2ServerCallStream(stream, handler); diff --git a/packages/grpc-native-core/src/server.js b/packages/grpc-native-core/src/server.js index 737e4314..3471f271 100644 --- a/packages/grpc-native-core/src/server.js +++ b/packages/grpc-native-core/src/server.js @@ -843,25 +843,34 @@ Server.prototype.forceShutdown = function() { this._server.forceShutdown(); }; -var unimplementedStatusResponse = { - code: constants.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); +function getUnimplementedStatusResponse(methodName) { + return { + code: Status.UNIMPLEMENTED, + details: 'The server does not implement the method' + methodName } -}; +} + +function getDefaultHandler(handlerType, methodName) { + const unimplementedStatusResponse = getUnimplementedStatusResponse(methodName); + switch(handlerType) { + case 'unary': + return (call, callback) => { + callback(unimplementedStatusResponse, null); + }; + case 'clientStream': + return (call,callback) => { + callback(unimplementedStatusResponse, null); + }; + case 'serverStream': + return (call) => { + call.emit('error', unimplementedStatusResponse); + } + case 'bidi': + return (call) => { + call.emit('error', unimplementedStatusResponse); + } + } +} function isObject(thing) { return (typeof thing === 'object' || typeof thing === 'function') && thing !== null; @@ -908,7 +917,7 @@ Server.prototype.addService = function(service, implementation) { if (implementation[attrs.originalName] === undefined) { common.log(constants.logVerbosity.ERROR, 'Method handler ' + name + ' for ' + attrs.path + ' expected but not provided'); - impl = defaultHandler[method_type]; + impl = getDefaultHandler(method_type, name); } else { impl = implementation[attrs.originalName].bind(implementation); } From 38bab42f30dca4bb9958c930d3441963b02b326d Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 20 Feb 2020 14:10:13 -0800 Subject: [PATCH 2/3] Fix reference to status enum in native impl --- packages/grpc-native-core/src/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-native-core/src/server.js b/packages/grpc-native-core/src/server.js index 3471f271..2735d28e 100644 --- a/packages/grpc-native-core/src/server.js +++ b/packages/grpc-native-core/src/server.js @@ -845,7 +845,7 @@ Server.prototype.forceShutdown = function() { function getUnimplementedStatusResponse(methodName) { return { - code: Status.UNIMPLEMENTED, + code: constants.status.UNIMPLEMENTED, details: 'The server does not implement the method' + methodName } } From 0257c585ea074a5588167bca0d590e49ce9218d0 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 20 Feb 2020 15:06:43 -0800 Subject: [PATCH 3/3] Correct handler type name spelling --- packages/grpc-native-core/src/server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grpc-native-core/src/server.js b/packages/grpc-native-core/src/server.js index 2735d28e..81e56347 100644 --- a/packages/grpc-native-core/src/server.js +++ b/packages/grpc-native-core/src/server.js @@ -857,11 +857,11 @@ function getDefaultHandler(handlerType, methodName) { return (call, callback) => { callback(unimplementedStatusResponse, null); }; - case 'clientStream': + case 'client_stream': return (call,callback) => { callback(unimplementedStatusResponse, null); }; - case 'serverStream': + case 'server_stream': return (call) => { call.emit('error', unimplementedStatusResponse); }