-
Notifications
You must be signed in to change notification settings - Fork 337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add error handler to rpc interface #965
feat: add error handler to rpc interface #965
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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=true
rpcAfterResponse=true
rpcErrorHandler=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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
rpc
interface 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 thehandleError
method, consumers of the service may leverage this functionality to:Other noteworthy additions to this PR:
tryCatchBlock
helper to theutils.ts
file to make it easier to create try-catch blocks in the future.outputBeforeRequest
oroutputAfterResponse
totrue
caused you to only be able to output thedefault
service - and thus, not have access to the servicegeneric-definitions
.service: string
andmethod: string
as parameters to thebeforeRequest
andafterResponse
methods in therpc
interface. These can be used to handle specific services or methods in a special way.package.json
to allow the VSCode-Jest debugger to attach correctly. Note: Happy to revert this if there is a reason why it's set.Testing
outputErrorHandler
by itself and the interaction withoutputAfterResponse
before-after-request
to includegeneric-definitions
outputBeforeRequest
,outputAfterResponse
, andoutputErrorHandler