search

twirphp / twirp

PHP port of Twitch's Twirp RPC framework
https://twirphp.github.io
MIT License
152 stars 19 forks source link

Add support for client hooks #42

Open pelletiermaxime opened 3 years ago

pelletiermaxime commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

sagikazarmark commented 3 years ago

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?