Add Opbeat.capture_rack_exception - #27
Conversation
There was a problem hiding this comment.
Thanks for the PR :) Can you explain why you don't send the event anymore?
There was a problem hiding this comment.
Becouse it is sent in the capture_rack_exception method. Also I think that original method didn't support async and also didn't support ignoring user defined errors. (note: it was also not checking if evt is nil)
There was a problem hiding this comment.
Sorry If you got confused about removing the expect(Opbeat).to receive(:send).with(@event) lines from tests. I put them back since the original issue that was getting the specs to fail was from elsewhere.
The async tests were letting the configuration have async callback, and since the middleware now supports async behaviour, the specs needed change. I fixed the async specs to cleanup after finish, so now that changes are not needed anymore.
There was a problem hiding this comment.
Also, maybe I didn't explain properly. This PR allows users to call the capture_rack_exception from their code (similar to capture_exception), allowing them to pass rack environment.
|
Hi, Did you have a chance to review this ? |
Currently I'm handling all exceptions at application level in order to properly format them and return proper response codes (400 vs 500).
https://github.com/intridea/grape#exception-handling
Since the
Opbeat::Rackmiddleware is put before myGrapemiddleware and since theGrapemiddleware is catching the errors and formatting them, the errors will never arrive inOpbeat::Rackmiddleware.This PR adds a method named
capture_rack_exception, which allows to also pass the rack environment.I also changed how
Opbeat::Racksends errors, passing them through this method, also allowing forasyncbehaviour.This method was extracted from our codebase, we are using this method in production as a monkey patch for a few months.