Custom RPC opts mapper

[mupperton]

Motivation:
I want to be able to set a HTTP header on every RPC call, which I currently achieve by iterating over my handlers and wrapping them in a new function which modifies the ctx object to overwrite the RPC call methods and auto-inject the HTTP header into an existing ClientCallOptions object or create a new one, on each call - a bit of a hassle

Possible other use cases:

• Set a different serde by default for RPC ops
• logging
• any RPC hooks of interest

I considered other approaches to this problem, such as allowing it to be set as handler metadata, which would then become a property of the HandlerWrapper class, but this didn't feel logical to belong to an individual handler, and felt more global - of course the mapper function can be coded to opt-out of default behaviour if a handler actually provides some Opts or SendOpts

Happy to hear thoughts on this, and please feel free to enhance the JSDoc on the public API if not sufficient

Happy to shorten the property names (e.g. clientCallOptsMapper) if too verbose, but I struggled to think of a good short name

[mupperton]

In hindsight, this extraction is not necessary, can inline all 3 usages if preferred

[igalshilman]

Hey @mupperton, I can see how this can be useful indeed!
Some initial thoughts:

• One thing that you'd need to be aware of is that the input / output keys are used at discovery and they report the shape of the input and the output. I'm not sure how clear it is that these new endpoint options are not effecting this.
• Also, I'm wondering rather this is a bit too specific, as probably what is a bit more general than this is e is some sort of a request middle-ware/interceptor.
• Should this be configured per service as well, not only per endpoint? I can imagine that some services might have different requirements (different header's to set?)

[igalshilman]

One more thought, if I'd like to use that for auth for example, wouldn't I also need an access to the request as well?
(i.e. jwt style tokens)

[mupperton]

• One thing that you'd need to be aware of is that the input / output keys are used at discovery and they report the shape of the input and the output. I'm not sure how clear it is that these new endpoint options are not effecting this.

This isn't touching the serdes of the handlers, so no issues with discovery, this is about when doing an RPC call

• Also, I'm wondering rather this is a bit too specific, as probably what is a bit more general than this is e is some sort of a request middle-ware/interceptor.

Yeah that's fair, this could instead be done as an interceptor on genericCall and genericSend, would that be better?

• Should this be configured per service as well, not only per endpoint? I can imagine that some services might have different requirements (different header's to set?)

Potentially yes, although I'd imagine if you had a specific header for a specific use-case you would want to add that in-line in usages, this was meant more as a global setting, like you always use binary data in every handler, or always use your own custom JSON serde, so you don't want to overwrite the input/output serde of every RPC call/send in addition to the handler serde changes made for discovery

I think possibly the interceptor style approach on genericCall/genericSend might be the way to go though instead, and hopefully a cleaner implementation - let me know if you agree

[mupperton]

One more thought, if I'd like to use that for auth for example, wouldn't I also need an access to the request as well? (i.e. jwt style tokens)

Yes, so my use case is to send a custom tracing header between RPC call requests, so I need access to the incoming request headers, but I do this through AsyncLocalStorage (as I also need it elsewhere like in logger context, like the Discord discussion we had about traceparent) and set it at invocation start and then can access it in my equivalent custom implementation of this PR that way

So I was thinking that would be another change to propose to add a invocation middleware thing to be able to wrap an invocation in AsyncLocalStorage context (as an example use-case) which then completely removes my "hacks"

Maybe there is a way though to also give more "context" to the interceptor approach though, like providing the actual ctx value too