Context for Custom Exceptions

[philipp-spiess]

I think adding Context to a Custom Exception through the @__raven_context member variable (see here and here) is not ideal. To improve this, I suggest two changes:

1. Switch from a member variable to an actual public method. This allows us for a more straight forward API design and doesn't require the context to be set during initialisation.

   # custom_exception.rb
   class CustomException < StandardError
     def raven_context
       {
         extra: {
           foo: "bar"
         }
       } 
     end
   end

2. We should use deep_merge instead of a regular merge. This way we won't override global context options. See here

   Imagine the previous CustomException with the following Raven call:

   Raven.capute_exception(CustomException.new, extra: { bar: "foo"} )

   A regular #merge would look like this:

   ({extra: {foo: "bar"}}).merge({extra: {bar: "foo"}})
   # => {:extra=>{:bar=>"foo"}} 

   Compared to a #deep_merge implementation, which would preserve previous the context.

   ({extra: {foo: "bar"}}).deep_merge({extra: {bar: "foo"}})
   # => {:extra=>{:foo=>"bar", :bar=>"foo"}} 

   The problem with #deep_merge is that it would require another dependency as it's not part of the standard library. But simple implementations are available.

[dcramer]

While I realize patching isn't overly frowned upon in Ruby, it'd likely be better if we continue to provide an abstraction that is like Raven.annotate_exception(exc, context) which promotes a better programming style. I forget if that function already exists, but it can easily apply the private local store on the exception instance itself.

[philipp-spiess]

I not sure what you mean by "patching". I'm proposing that we add an API to provide context for custom exception classes. You could also do this in Java, for example, by implementing an Interface.

I don't like the Raven.annotate_exception(exc, context) approach - this requires me to take care of adding the annotations whenever I raise an exception. When I have to do this all the time I could also simply call Raven.capture_exception(exc, context). This would also tightly couple my library with Raven (*).

Another option is that we have something like the processor system, where you could register annotation handler for specific exception classes. But this would still require me to store the info inside the exception class and I would have to take care of an initializer, that registers this annotations for all my custom exception classes.

(*) We could completely decouple by calling my custom #raven_context method something like #additional_context which could, in theory, also be used by other error handling software or Ruby itself. Some research should be done if this is not already the case 😄

[dcramer]

@philipp-spiess are you just looking to throw up a bunch of metadata along with an exception? e.g. HttpError having status_code?

I think I understand the value, but it's a bit of an overload on what context is today. It's generally not exception specific, but its environment based. That is, context is often things like "whats the current state of foo". Raven.annotate_exception is intended that you can do it with other events, so if it were to get captured by Sentry it could associate the given attributes. I did misread before thinking you were suggesting we somehow patch existing exceptions (I'm not a Rubyist). I believe in Python we support a __sentry__ attribute to capture additional data, so I'm fine w/ this general concept.

I definitely understand the desire to not couple your code to Sentry, but quite frankly, once you use Sentry you don't stop using it. There is absolutely a line you'd need to draw that says "this kind of data is valuable". Once you do, the fact that you're coupled to something is insignificant. You could just as easily make an annotate_exception function which calls Raven so its easier to decouple.

[philipp-spiess]

HttpError having status_code?

This is exactly what we're looking for - metadata so we can recreate the conditions that lead to this exception. You can achieve this currently by setting a member variable and raven-ruby will do some nasty work to extract and use it (1). This can be improved by making the API less hidden and by deferring the creation of this hash to the last possible moment.

once you use Sentry you don't stop using it

I agree 😄

(1).

# raven-ruby/event.rb
exc.instance_variable_defined?(:@__raven_context) && exc.instance_variable_get(:@__raven_context)

[nateberkopec]

Take a look at #491 and tell me what you think.

[philipp-spiess]

This is amazing 👍

[nateberkopec]

Fixed by #491, available in 1.0.0+.