feat: add error handler to rpc interface #965
Conversation
There was a problem hiding this comment.
Hi @lukealvoeiro , these changes look good/make sense, few questions but otherwise yep will merge after you respond back. Thanks!
| if (error instanceof Error && this.rpc.handleError) { | ||
| throw this.rpc.handleError(this.service, "GetBasic", error); | ||
| } | ||
| throw error; |
There was a problem hiding this comment.
@lukealvoeiro wdyt of instead of throw error, we make this return Promise.reject(error)?
My rationale is that, as-is, because this GetBasic method isn't using the async keyword, errors that are thrown won't actually turn into rejected promises, which is typically what callers except.
There was a problem hiding this comment.
ah true, good suggestion - will make this change!
README.markdown
Outdated
| @@ -446,9 +446,11 @@ Generated code will be placed in the Gradle build directory. | |||
|
|
|||
| - With `--ts_proto_opt=outputServices=false`, or `=none`, ts-proto will output NO service definitions. | |||
|
|
|||
| - With `--ts_proto_opt=outputBeforeRequest=true`, ts-proto will add a function definition to the Rpc interface definition with the signature: `beforeRequest(request: <RequestType>)`. It will will also automatically set `outputTypeRegistry=true` and `outputServices=true`. Each of the Service's methods will call `beforeRequest` before performing it's request. | |||
| - With `--ts_proto_opt=outputBeforeRequest=true`, ts-proto will add a function definition to the Rpc interface definition with the signature: `beforeRequest(service: string, message: string, request: <RequestType>)`. It will will also automatically set `outputServices=default`. Each of the Service's methods will call `beforeRequest` before performing it's request. | |||
There was a problem hiding this comment.
@lukealvoeiro This is admittedly a little late, since we've already released these outputBefore/After flags, but wdyt of renaming all of these to:
rpcBeforeRequest=truerpcAfterResponse=truerpcErrorHandler=true
The disclaimer/mea culpa is that I've not been great at having a super-consistent naming convention for flags, we just kind of make them up as we go, but given these are all related to the Rpc interface, that seems like a good prefix?
There was a problem hiding this comment.
No worries, those names are definitely better. Thanks for the quick 👀 on this PR :)
|
Looks great, thanks @lukealvoeiro ! |
# [1.165.0](v1.164.2...v1.165.0) (2023-11-28) ### Features * add error handler to rpc interface ([#965](#965)) ([47cd16e](47cd16e))
|
🎉 This PR is included in version 1.165.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Extending off of my previous PR, I have added in additional 'middleware' to the default service
rpcinterface in the form of a method callederrorHandler. This method catches any errors that may have occurred during the entire lifecycle of a RPC request. While ultimately you will still need to return some Error from thehandleErrormethod, consumers of the service may leverage this functionality to:Other noteworthy additions to this PR:
tryCatchBlockhelper to theutils.tsfile to make it easier to create try-catch blocks in the future.outputBeforeRequestoroutputAfterResponsetotruecaused you to only be able to output thedefaultservice - and thus, not have access to the servicegeneric-definitions.service: stringandmethod: stringas parameters to thebeforeRequestandafterResponsemethods in therpcinterface. These can be used to handle specific services or methods in a special way.package.jsonto allow the VSCode-Jest debugger to attach correctly. Note: Happy to revert this if there is a reason why it's set.Testing
outputErrorHandlerby itself and the interaction withoutputAfterResponsebefore-after-requestto includegeneric-definitionsoutputBeforeRequest,outputAfterResponse, andoutputErrorHandler