gruf
gruf copied to clipboard
Controller unhandled errors should be passed as BadStatus to interceptors
While using Gruf::Interceptors::Instrumentation::RequestLogging::Interceptor
, I noticed that in the event of unhandled errors raised in my controller (errors that are not of type GRPC::BadStatus
or raised via fail!
), I was not getting any request logging in my log.
In a successful controller call I would get something like:
[GRPC::Ok] (list_units) [5.24ms] [GRPC::Ok] (rpc.units.list_units) Parameters: {:is_active=>false, :check_can_delete=>false}
If an unhandled error was raised I would get nothing.
My assumption is that I should still get logging in that case. Request info is useful to have, including when an unhandled error occurs.
Investingating why I wasn't getting any, I observed that in Gruf::Interceptors::Instrumentation::RequestLogging::Interceptor
, the interceptor yields to the controller via this line:
result = Gruf::Interceptors::Timer.time(&block)
Gruf::Interceptors::Timer.time
knows to rescue
and handle errors, but only of type GRPC::BadStatus
. I realized that unhandled errors would not be caught by it, hence the error logging interceptor would get exited right there and never log.
I noticed that the base controller code turns unhandled errors in the GRPC::BadStatus
errors. But it only does so after all the interceptors have run (or been tripped as above). I thought why not just do this before the interceptors. Seeing how the logging interceptor is designed, maybe it was even meant to be that way. So this is what I propose here.
It works well for me so far.
API changes:
- Now interceptors will get a
GRPC::BadStatus
error in the case of unhandled errors, instead of the original unhandled error. To reach the original error,GRPC::BadStatus#cause
can be used.
wondering if interceptors like gruf-sentry
should be updated to log cause
in the case of internal
errors