Conversation
…lemetry errors Signed-off-by: jorgee <jorge.ejarque@seqera.io>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
| if( abortOnError ) { | ||
| throw new AbortRunException("Invalid Seqera Platform API response - Missing workflow Id") | ||
| } |
There was a problem hiding this comment.
Why adding this condition if an exception was already thrown?
There was a problem hiding this comment.
The same as the previous comment, notifyEvent was catching the exceptions but not re-throwing to produce an error in the execution.
| if( abortOnError ) { | ||
| throw new AbortRunException(resp.message) | ||
| } |
There was a problem hiding this comment.
Not getting what this is adding?
There was a problem hiding this comment.
notifyEvent was catching all exceptions and printing a log message, but not re-throwing the exception. So, no error was produced with normal exceptions. I changed notifyEvent to catch and rethrow the AbortRunException. I didn't want to rethrow all exceptions or use the AbortOperationException because it is widely used and it could be very likely to introduce side effects. The AbortRunException was only used at ScriptRunner and Launcher. It was safe to use and matches with the meaning of the exception
I initially implemented it by invoking session.abort from here, but it was not working because the execution has not started yet. Abort was executed and threw the exception, but as it was caught at notifyEvent, the execution continued and produced failures later when trying to run the script in the aborted session. I think it could be fixed by inspecting the session just after invoking start, and throwing an exception if it is aborted.
Both are easy to implement; I just selected this one because it works in the same way for all the events, and it could be reused for other observers. If you think something is wrong or it's better to abort directly, I can try with the other approach.
pditommaso
force-pushed
the
master
branch
2 times, most recently
from
d9fa5cd to
d752bc2
Compare
2 participants
This pull request introduces a new mechanism to control error handling behavior in the
TowerClientclass by adding anabortOnErrorflag, which can be set via the environment variableTOWER_ABORT_ON_ERROR. When enabled, critical errors encountered while communicating with Seqera Platform will cause the workflow to abort immediately using theAbortRunException. The changes also include improved error propagation and additional tests to verify this behavior.Error Handling Improvements:
abortOnErrorflag toTowerClient, defaulting totrue, and made it configurable via theTOWER_ABORT_ON_ERRORenvironment variable. This determines whether critical errors abort the workflow or are handled as warnings. [1] [2]TowerClientmethods (logHttpResponse,parseTowerResponse, and others) to throwAbortRunExceptionwhenabortOnErroris enabled, ensuring immediate workflow termination on critical errors. [1] [2] [3] [4] [5]Session and Exception Propagation:
Sessionclass to specifically catch and logAbortRunExceptionduring observer notification, ensuring these exceptions propagate and abort the workflow as intended. [1] [2]Tests:
TowerClientTestto verify the correct detection of theabortOnErrorsetting and to ensure that the workflow aborts as expected when errors occur andabortOnErroris enabled. [1] [2]