Skip to content
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

Add support for client hooks #42

Open
pelletiermaxime opened this issue Nov 2, 2020 · 3 comments
Open

Add support for client hooks #42

pelletiermaxime opened this issue Nov 2, 2020 · 3 comments
Milestone

Comments

@pelletiermaxime
Copy link

In all my code that calls the rpc services I end up wrapping the call in a try catch and doing the same error treatment. I was looking at if it was possible to add some kind of middleware/hooks/interceptor, but that would ideally need to be available in the generated code. Looking at the Go client I see that they now have client hooks (https://godoc.org/github.com/twitchtv/twirp#ClientHooks).

The implementation would probably look a lot like the server hooks, an argument to the client creation with a class that implements the RequestPrepared, ResponseReceived and Error functions.

What do you think ?

@sagikazarmark
Copy link
Member

What exactly would you like to do inside a client hooks?

Normally, errors (exceptions) are supposed to be handled for each call (wrapping the call in a try catch). That's how you retain control.

The error hook in the example does not stop the error from being returned. An example I'd use that hook is recording metrics.

But I don't think that hook is for "generic error handling".

tl;dr: I'm not against implementing client hooks, but I'm not sure it'll solve your issue.

@pelletiermaxime
Copy link
Author

Yes the example of my use case would be to reformat exceptions to send them to Sentry and then rethrow them.

@sagikazarmark
Copy link
Member

I see. I guess that makes some sense. But business errors (like not found) should probably not be reported to Sentry, but that's an application specific thing to decide.

I guess rethrowing would be done by the framework (at least that's how the Go client hook implemetation works).

Would you be open to submitting a PR?

@sagikazarmark sagikazarmark added this to the 1.1.0 milestone Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants