Ability to provide listener for unhandled execution errors

[danveloper]

• Fixes Support for execution error handlers from user registry #1037

This commit provides the means by which users can add one or many ExecutionErrorListener instances to an execution registry. Furthermore it extends the capabilities of ExecStarter#onError to allow multiple error handlers to be specified in a forked execution.

Users will be able to use this feature to provide a handler that will capture any uncaught exceptions in an execution, and do further processing in a new execution.

---

This change is 

[danveloper]

this allows us to pause the originating execution while the ExecutionErrorListener instances are working; then once they have completed, this will properly close off the original execution.

This is necessary to preserve the functionality that error handlers are invoked prior to the execution's onComplete handler.

[ldaley]

We should try to avoid this. We already had the behaviour of executing the error handlers before the on complete handler. What broke about that?

[danveloper]

with this, multiple onError calls can be made and they will all be fired on an error. This may constitute a breaking behavior change.

[ldaley]

@danveloper why do we need to have this multiple error handler behaviour?

[johnrengelman]

failing specs:

ratpack.exec.ExecutionErrorHandlingSpec > error listeners are pulled from registry FAILED
    Condition not satisfied:

    result == "!"
    |      |
    ""     false
           1 difference (0% similarity)
           (-)
           (!)
        at ratpack.exec.ExecutionErrorHandlingSpec.error listeners are pulled from registry(ExecutionErrorHandlingSpec.groovy:77)

ratpack.exec.ExecutionErrorHandlingSpec > multiple error listeners will be invoked and in registry order FAILED
    Condition not satisfied:

    results == [2, 1]
    |       |
    |       false
    [2, 1] (java.util.ArrayList)
        at ratpack.exec.ExecutionErrorHandlingSpec.multiple error listeners will be invoked and in registry order(ExecutionErrorHandlingSpec.groovy:101)

ratpack.manual.JavadocCodeSnippetTests > ratpack.exec.ExecutionErrorListener:27 FAILED
    ratpack.manual.snippets.executer.CompileException: java.lang.AssertionError: ERROR: illegal start of expression (29:12)

        Caused by:
        java.lang.AssertionError: ERROR: illegal start of expression (29:12)

[ldaley]

I have merged master into this. Let's get this into 1.6.

[ldaley]

@danveloper I would like to make a couple of changes.

1. Revert the additive behaviour of ExecSpec.onError
2. Avoid forking a new execution to invoke the error handlers

Other than that, I think we are good to merge (once we fix the tests).

[danveloper]

For 1: how would you like to handle the addition of the error handlers? Via the registry?

For 2: how do we ensure the error handlers run inside an execution? The current execution is complete by the time the error handlers would be invoked.

[ldaley]

1. I think we should restrict the goal here to just being the ability to override the “no error handler” error handler. So these would work just like interceptors/initializers in that we pull all implementations from the server registry. Add an impl to an execution while running or when bootstrapping would have no effect.

2. It should be possible to do this without forking a new execution.

I'll make both changes today.

[gschrader]

Is this still queued up for 1.6?

[beckje01]

@gschrader it's set to be reviewed this week and included if seems reasonable

[beckje01]

I'll look at removing the additive behavior next then I think this will have all its tests passing.

[michaelschlies]

@beckje01 is there outstanding things on this PR? running locally it isn't failing, and I've been seeing some strange timing issues out of circleci lately so it might be worth just re-triggering this to see if you just hit a hiccup.