fix(exec): stop failing asynchronous hooks with an empty error message - #81
loicsapone wants to merge 1 commit into
Conversation
|
Nice catch — I replayed your tests against main and they do fail with an empty exception message, so this is a real one. The quiet: true path in upload() is where it hurts most: a failing tar xzvf currently aborts the deploy with no information at all. One thing before I merge: we still don't know which server failed. That's the whole point of the async path, and in quiet mode the exception is the only output the user gets. Keying $process by server name and prefixing the message would be a few lines. Minor, take it or leave it: now that ExecCommand writes to stderr, the stdout fallback will mostly pick up PHP deprecations (CLI sends those to stdout), so it may occasionally present noise as the cause. And since Ssh::exec() throws with the command's full output, a failing composer install will render its entire log inside the [ERROR] block — right after the streaming callback already printed it. Truncating to the last few lines could be a separate PR. |
When a hook runs on several servers, each one is executed by a child
automate execprocess, and
Context::execAsync()reported the failure from that process' erroroutput:
But
ExecCommandwrites the reason on the standard output:So the error output was always empty and the deployment stopped on a blank
[ERROR]block with no clue about what went wrong.
ExecCommandnow reports on the error output, and the parent falls back on thestandard output, then on the exit code, so the message is never empty whatever the
child does.
Tests
New
ContextTest, 3 cases (reported on stderr, reported on stdout, not reported atall). The last two fail on
mainwith an empty exception message.ExecCommanditself is not unit tested: it builds itsSshinternally and callslogin()outside thetry, so it cannot be exercised without a refactoring thatfelt out of scope here.