search

framed-data / overseer

Overseer is a library for building and running data pipelines in Clojure.
Eclipse Public License 1.0
97 stars 10 forks source link

Errors: Catch/support java.util.concurrent.ExecutionException #57

Closed andrewberls closed 8 years ago

andrewberls commented 9 years ago

If an exception is thrown within a future, it will die with a j.u.c.ExecutionException 'wrapping' the original exception. This means that if e.g. overseer/abort is used within a future, it will noisily log to Sentry and follow the normal rules because the system is unable to see that it's actually a controlled exception with ex-data. This modifies the error handling to recognize j.u.c.ExecutionExceptions as we want any thread to be able to control job execution, and supports the normal rules.

There is ongoing discussion about whether or not exceptions are a suitable means to 'communicate' known statuses with the Overseer harness, vs a system oriented around return values and tagged data structures or similar. Regardless of the conclusion of this discussion, this is a hotfix of sorts for a current production situation.

elliot42 commented 9 years ago
  1. I don't think silencing sentry errors messages is a big enough purpose to put in a framework-level contract change, 2. especially not for a framework-level contract change I happen to disagree with
elliot42 commented 9 years ago

i.e. if you just want the hotfix, (again, I don't think live hotfixes are necessary for like, logging/exception noise) then it can totally go in the app, not the framework

elliot42 commented 9 years ago

if the change is going in the framework then I think it deserves to be ironed out

elliot42 commented 9 years ago

And I even think there are ways to iron out the non-local control flow story that create a standardized contract. Erlang uses universal exit signals and UNIX uses exit statuses/codes--if we actually had UNIX-process-level-isolation/management then we could always use (System/exit), and uncaught exceptions would always auto-propagate until they culminated in the equivalent of a System/exit--it's a unified system.

If it's truly necessary to have non-local control flow, and it can't be done through a more universal systems then you could still have a more uniform/explicit system, e.g. if we declared a well-known interface/protocol for Overseer then the contract could be that Overseer knows how to introspect IOverseerControl exceptions, they would all implement (control/silent?), and then ExecutionExceptions could include specific logic for unwrapping. I still think arbitrarily deep unwrapping in custom code for ExecutionException seems like a mess, but at the very least it would all be behind a unified interface (IOverseerControl) and the special case recursive logic for ExecutionException would be isolated from the otherwise straightforward logic of all other non-wrapped exceptions (which should normally be the case, because that's the normal contract for exceptions which is to propagate up the call stack, not get recursively wrapped at every layer up the call stack).

Even if we want to do something like this I think there's more ways to make it standardized and not a this-or-that contract, which is like the definition of not successfully defining a single common interface that covers/declares how a thing works, and core of why things get hard to reason about, because there is no single rule at all--the rule itself is defined as "this in this case and that in that case", which is what a unifying abstraction (even if the abstraction underneath delegates to multiple cases) intended to resolve--it can isolate the addition of casewise complexity instead of propagating it.