L112: Node Server Interceptors#406
Conversation
| method handler | ||
| ``` | ||
|
|
||
| ## Rationale |
There was a problem hiding this comment.
Could you add a section on how closely (or not) the above APIs match the client-side interceptor design & apis?
There was a problem hiding this comment.
You can see the client interceptor APIs in https://github.com/grpc/proposal/blob/master/L5-node-client-interceptors.md. They are as close as possible, but they're not identical because there are inherent differences in client and server functionality. I said that the design "mirrors" the client interceptor design in the abstract, in the sense that client send operations generally correspond to server receive operations, and vice versa.
There was a problem hiding this comment.
I added a section to show the correspondence.
L112-node-server-interceptors.md
Outdated
|
|
||
| ### Single injection point | ||
|
|
||
| The client interceptor design has interceptor lists and interceptor provider lists, and interceptors specified at client construction vs interceptors specified for individual call invocations. In contrast, this design only specifies that an interceptor list can be provided at server construction. The primary purpose of this design is to solidify the interceptor part of the design. Additional injection points for interceptors could be specified in the future, and the way they interact with each other can be decided at that time. |
There was a problem hiding this comment.
This is probably one of the areas that is unique to server interceptors - which by design shouldn't support per-call inceptor injection as any inbound operations/events are supposed to be intercepted before any business logic is invoked (as part of the service method implementation).
There was a problem hiding this comment.
I updated this section to try to clarify it.
| The `Server` constructor's `options` parameter will accept a new option `interceptors`, which is an array of interceptor functions. | ||
|
|
||
|
|
||
| #### Interceptor nesting |
There was a problem hiding this comment.
Nit: chaining may be more commonly used for this term.
One of the reasons I asked about the client interceptor design is to find out if I need to review the interceptor chaining or nesting flow.
E.g. if interceptorA onReceiveMetadata() decides to respond a status immediately (e.g. an auth failure) will interceptorB/C onReceiveMetadata() be called by the framework or the call stack will jump to the outbound flow (for interceptorA).
There was a problem hiding this comment.
I used "nesting" because it's the term that the corresponding section of the client interceptors design used.
The next callback in each method allows the interceptor author to control that flow. In your particular auth failure example, the auth interceptor's onReceiveMetadata method can explicitly send a non-OK status and not call the next callback. As a result, the entire rest of the inbound/outbound flow will be skipped, and the handler will never be invoked. This is broadly the same as in the client interceptors design.
wenbozhu
left a comment
There was a problem hiding this comment.
LGTM
I left a comment on interceptor nesting. If any design changes are needed, they can be done separately.
|
LGTM. Thanks. |
|
I made a few changes to address issues that came up while writing the implementation. |
|
@murgatroid99 Is this proposal under implementation? |
|
The implementation of this proposal is available in grpc-js version 1.10.x |
|
@murgatroid99 Thanks for letting me know :) |
No description provided.