From e5c6635982169ab39d51138618cc5dab23f1a5da Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 28 Sep 2018 10:50:41 -0700 Subject: [PATCH 1/4] L41: Asynchronous port binding method in Node Server API --- L41-node-server-async-bind.md | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 L41-node-server-async-bind.md diff --git a/L41-node-server-async-bind.md b/L41-node-server-async-bind.md new file mode 100644 index 000000000..eaf714c4a --- /dev/null +++ b/L41-node-server-async-bind.md @@ -0,0 +1,46 @@ +Asynchronous port binding method in Node Server API +---- +* Author(s): murgatroid99 +* Approver: wenbozhu +* Status: Draft +* Implemented in: Node +* Last updated: 28-09-2018 +* Discussion at: (filled after thread exists) + +## Abstract + +Add a method to the Node gRPC `Server` class that binds a port as an asynchronous operation. + +## Background + +In the initial development of the Node gRPC library, the `Server` class method to bind a port, called `bind`, was implemented as a synchronous operation because it was available as an asynchronous operation in the underlying core library. In contrast, in the Node built in `net` module, the `Server` class's `listen` method, which performs a similar function, is asynchronous. In the new pure JavaScript Node gRPC library, the underlying network operations use the `net` module, so it is impossible to implement the `bind` method as a synchronous operation. + + +## Proposal + +We would add a method called `bindAsync` to the `Server` class that implements the same functionality as `bind`, but provides the result asynchronously. The specific function signature would be as follows: + +```ts +class Server { + + // ... + + /** + * @param port The port that the server should bind on, in the format "address:port" + * @param creds Server credentials object to be used for SSL. + * @param callback Callback to be called asynchronously with the result of binding the port + */ + bindAsync(port: string, creds: ServerCredentials, callback: (port: number)=>void): void; +} +``` + +The semantics of the arguments are exactly the same as with the existing `bind` method, and the semantics of the port passed to the callback are identical to the semantics of the return value of the existing `bind` method: a negative number indicates failure, and a positive number indicates success binding to that port number. + +## Rationale + +The sematics of `bindAsync` would be identical to the semantics of `bind` except it is asynchronous, so the name communicates that. Keeping the semantics and API as similar as possible minimizes the work need to transition between the two. We could also implement the asynchrony using promises, but a callback based API is more consistent with the rest of the existing API. In the future, promise support could be added, perhaps by returning a promise when no callback is provided. + + +## Implementation + +I (@murgatroid99) will implement this in the grpc library as soon as this proposal is accpeted, and in the @grpc/grpc-js library as part of the server implementation. From 5c2ed70ab70f65d5a3dd5339dd7419430035c964 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 28 Sep 2018 10:57:18 -0700 Subject: [PATCH 2/4] L41: Add discussion thread --- L41-node-server-async-bind.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L41-node-server-async-bind.md b/L41-node-server-async-bind.md index eaf714c4a..a2d8d1f71 100644 --- a/L41-node-server-async-bind.md +++ b/L41-node-server-async-bind.md @@ -5,7 +5,7 @@ Asynchronous port binding method in Node Server API * Status: Draft * Implemented in: Node * Last updated: 28-09-2018 -* Discussion at: (filled after thread exists) +* Discussion at: https://groups.google.com/forum/#!topic/grpc-io/Uio1Ja6GRHI ## Abstract From dcd3f1e881c1d0d3737c2d146635bbfeddee416c Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 28 Sep 2018 11:41:49 -0700 Subject: [PATCH 3/4] L41: Correct some typos --- L41-node-server-async-bind.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/L41-node-server-async-bind.md b/L41-node-server-async-bind.md index a2d8d1f71..359cfc98a 100644 --- a/L41-node-server-async-bind.md +++ b/L41-node-server-async-bind.md @@ -13,7 +13,7 @@ Add a method to the Node gRPC `Server` class that binds a port as an asynchronou ## Background -In the initial development of the Node gRPC library, the `Server` class method to bind a port, called `bind`, was implemented as a synchronous operation because it was available as an asynchronous operation in the underlying core library. In contrast, in the Node built in `net` module, the `Server` class's `listen` method, which performs a similar function, is asynchronous. In the new pure JavaScript Node gRPC library, the underlying network operations use the `net` module, so it is impossible to implement the `bind` method as a synchronous operation. +In the initial development of the Node gRPC library, the `Server` class method to bind a port, called `bind`, was implemented as a synchronous operation because it was available as a synchronous operation in the underlying core library. In contrast, in the Node built in `net` module, the `Server` class's `listen` method, which performs a similar function, is asynchronous. In the new pure JavaScript Node gRPC library, the underlying network operations use the `net` module, so it is impossible to implement the `bind` method as a synchronous operation. ## Proposal @@ -38,9 +38,9 @@ The semantics of the arguments are exactly the same as with the existing `bind` ## Rationale -The sematics of `bindAsync` would be identical to the semantics of `bind` except it is asynchronous, so the name communicates that. Keeping the semantics and API as similar as possible minimizes the work need to transition between the two. We could also implement the asynchrony using promises, but a callback based API is more consistent with the rest of the existing API. In the future, promise support could be added, perhaps by returning a promise when no callback is provided. +The semantics of `bindAsync` would be identical to the semantics of `bind` except it is asynchronous, so the name communicates that. Keeping the semantics and API as similar as possible minimizes the work need to transition between the two. We could also implement the asynchrony using promises, but a callback based API is more consistent with the rest of the existing API. In the future, promise support could be added, perhaps by returning a promise when no callback is provided. ## Implementation -I (@murgatroid99) will implement this in the grpc library as soon as this proposal is accpeted, and in the @grpc/grpc-js library as part of the server implementation. +I (@murgatroid99) will implement this in the grpc library as soon as this proposal is accepted, and in the @grpc/grpc-js library as part of the server implementation. From a54de0906c84645e328a5ff30d8ce51b770ce82e Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 28 Sep 2018 12:27:46 -0700 Subject: [PATCH 4/4] L41: Add error to callback --- L41-node-server-async-bind.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/L41-node-server-async-bind.md b/L41-node-server-async-bind.md index 359cfc98a..37951ddc0 100644 --- a/L41-node-server-async-bind.md +++ b/L41-node-server-async-bind.md @@ -30,15 +30,15 @@ class Server { * @param creds Server credentials object to be used for SSL. * @param callback Callback to be called asynchronously with the result of binding the port */ - bindAsync(port: string, creds: ServerCredentials, callback: (port: number)=>void): void; + bindAsync(port: string, creds: ServerCredentials, callback: (error: Error, port: number)=>void): void; } ``` -The semantics of the arguments are exactly the same as with the existing `bind` method, and the semantics of the port passed to the callback are identical to the semantics of the return value of the existing `bind` method: a negative number indicates failure, and a positive number indicates success binding to that port number. +The semantics of the arguments are exactly the same as with the existing `bind` method, and the semantics of the port passed to the callback are identical to the semantics of the return value of the existing `bind` method: a negative number indicates failure, and a positive number indicates success binding to that port number. An error will also be passed to the callback if `port` is negative. ## Rationale -The semantics of `bindAsync` would be identical to the semantics of `bind` except it is asynchronous, so the name communicates that. Keeping the semantics and API as similar as possible minimizes the work need to transition between the two. We could also implement the asynchrony using promises, but a callback based API is more consistent with the rest of the existing API. In the future, promise support could be added, perhaps by returning a promise when no callback is provided. +The semantics of `bindAsync` would be identical to the semantics of `bind` except it is asynchronous, so the name communicates that. Keeping the semantics and API as similar as possible minimizes the work need to transition between the two. The semantically redundant `error` is passed to the callback for greater consistency with other Node APIs, so that it can be used with functions such as `util.promisify()`. We could also implement the asynchrony using promises, but a callback based API is more consistent with the rest of the existing API. In the future, promise support could be added, perhaps by returning a promise when no callback is provided. ## Implementation