Detecting Exceptions In Interceptors

[mike-thompson-day8]

trafficstars

The following was raised via Google groups and transferred to here for discussion ...

Hello, I recently updated to Re-frame version 0.8.0. I have transitioned the majority of my custom middleware to interceptors but am having trouble with one. I created an application specific logging utility that sends information to Elasticsearch. The error logging middleware looked as follows

(defn log-error
  [handler]
  (fn log-error-handler
    [db v]
    (try
        (handler db v)
        (catch :default e
          (do
            (error (.-message e) (.-stack e))
            (throw e))))))

This middleware was being included with a few others: (def standard-middlewares [log-error log-params trim-v])

But now that I am not calling the handler function directly I am unsure how to catch and log errors from an interceptor. Could anyone please point me in the right direction?

[mike-thompson-day8]

At the most basic level, you can simply hook window.onerror to know when exceptions happen during the execution of an interceptor chain.

If we were to adopt the approach used in Pedestal, it would look like this:

• re-frame catches any exception thrown during the execution of an interceptor chain, including the handler.
• the exception is assoc-ed into context with the key :ex
• Interceptors can have a :before, :after and :error fn.
• when an exception occurs, the further execution of interceptors in the chain is stopped, and the interceptor stack is unwound in reverse order, but instead of :after being called on each interceptor :error is called with the thrown exception.
• :error fns take one exception argument, and return a new, replacement exception. They have the option of returning the given argument.

I've avoided going all-in on exceptions because it does make the process more complicated. I wanted to better understand the usecases. If you have one, can you add it to this ticket please.

[lwhorton]

I've considered something like a per-interceptor error scheme, but it does seem rather ... inconvenient. My particular use case is simply for reporting js-errors to a third party, such as sentry. Previously it was simple to capture any errors at any point during the handler-chain because the top most handler could be wrapped in a try/catch. I suppose the minimal amount of work would be an interceptor that looks at :after errors, but the question remains who is responsible for putting an :after :error on the effects? It seems out of the hands of user-land at the moment.

[mike-thompson-day8]

I'm wondering if this approach might be good enough (untested):

(def log-error
  (let [old-onerror-fn (atom nil)         ;; what js/window.onerror used to be
        new-onerror-fn (fn [e]            ;; our alternative for js/window.onerror
                         (error e)        ;; call my function 
                         (@old-onerror-fn e)    ;; do whatever else needs to be done
                         (set!  (.onerror  js/window) @old-onerror-fn))]
     (->interceptor 
       :before  (fn [context]
                   (reset! old-onerror-fn js/windows.onerror)
                   (set!  (.onerror  js/window)  new-onerror-fn)
                   context)
       :after  (fn [context]
                  (set!  (.onerror  js/window) @old-onerror-fn)
                  context)))

The above really is just a sketch because js/window.onerror doesn't just take one argument (which I've assumed for simplicity above), but hopefully you get the idea.
https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror.html

[lwhorton]

This is a clever solution, and I don't see any immediate unintended side-effects that might occur by set/resetting.

[mike-thompson-day8]

I'm tempted to close this ticket.

If possible, I'd like to avoid the complexity of exception handling in the Interceptor execution code. Pedestal goes to a lot of effort around exception handling because it must allow for the cleanup of stateful Interceptors - I'm not sure we have the same needs in re-frame.

If anyone truly wants to capture exceptions, and log them, they can just permanently hook window.onerror. Surely they want to capture all exceptions and not just those arising from event processing?

And, in case I'm wrong, and someone does, indeed, only want to capture exceptions in events, then they can use the slightly tricky approach I sketched out above.

[lwhorton]

Perhaps a quick note somewhere in the appropriate docs explaining above? "We don't do error handling, and interceptors aren't going to do it either."

[mike-thompson-day8]

This has come up a couple of times now.
I'm open to a PR which adds this as an FAQ in here ... https://github.com/Day8/re-frame/tree/develop/docs/FAQs

[mike-thompson-day8]

I have added a very brief FAQ entry which points back to this issue. The FAQ entry is still only in develop.

[nberger]

I had a similar need in a project, while trying to upgrade it to 0.8.0: we used to have a middleware that posts exceptions in handlers to Sentry. We would like to keep it at the interceptor level: window.onerror is farther from the point of failure, some context is missing at that point, and also it doesn't offer us a nice way to recover (this middleware/interceptor could potentially handle some errors in such a way that execution of that interceptor chain can continue "normally".

So I went ahead and did the exercise of implementing the exception handling as part of the interceptors chain, following what @mike-thompson-day8 outlined above (and also looking into the pedestal code).

If anyone is interested the code is here: https://github.com/Day8/re-frame/compare/master...nberger:add-interceptor-error-handling?expand=1. I'd love to send a PR if there's interest from the maintainers. To me, this is really a missing piece and a blocking issue for us to upgrade to 0.8.0.

[danielcompton]

Hey this is really interesting. Definitely open up a PR and we can review it closer. I like that you've written tests already :) Can't promise we'll merge it, and work is pretty busy at the moment, but we're very interested.

[nberger]

@danielcompton: Great! Just opened #330 with a few more tests :)

[antonmos]

We have a similar use-case - ability to provide a graceful experience to users when any error occurs - "Sorry, error has occured, email us and include this reference id". So in addition to logging the error, I would want to render a different view. Doubt that affects the solution but wanted to make sure this is considered. Thanks!

[danielcompton]

I've been looking at #423 and am weighing up the pros and cons of adopting it. The current solution we have in re-frame, while suboptimal from the point of view of giving context, does have the advantage that stack frames are preserved correctly.

So far the different use cases I have seen are:

• Sending unhandled exceptions to an exception tracker like Sentry
• Popping up an error dialog box to users when an error occurs
• Handling some kinds of errors and recovering from them within your application logic

I don't think we're too keen on trying to support the 3rd option (see https://github.com/Day8/re-frame/pull/330#issuecomment-289636085 for more explanation), are there any other use cases that we should be considering?