[rayapp] updating rayapp to handle errors correctly#490
Conversation
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
…ot-barn-update-rayapp-test-stderr-handling
There was a problem hiding this comment.
Code Review
This pull request updates the Anyscale CLI wrapper to correctly detect remote command failures that log 'exec failed with exit code' to stderr while exiting with a zero status code. It ensures both stdout and stderr are captured and included in the error message. Additionally, a new test case was added to verify this behavior for stderr. The reviewer suggested using a constant for the failure message string to improve maintainability and making the error message more descriptive.
| if strings.Contains(stdout, "exec failed with exit code") || | ||
| strings.Contains(stderr, "exec failed with exit code") { | ||
| return "", fmt.Errorf( | ||
| "anyscale error: command failed:\nstdout: %s\nstderr: %s", | ||
| stdout, stderr, | ||
| ) | ||
| } |
There was a problem hiding this comment.
The error message "command failed:" is somewhat generic. Since this block specifically handles a known CLI issue where it exits with 0 despite a remote failure, using a constant for the error string and including it in the error message improves both maintainability and error clarity for the user.
| if strings.Contains(stdout, "exec failed with exit code") || | |
| strings.Contains(stderr, "exec failed with exit code") { | |
| return "", fmt.Errorf( | |
| "anyscale error: command failed:\nstdout: %s\nstderr: %s", | |
| stdout, stderr, | |
| ) | |
| } | |
| const failureMsg = "exec failed with exit code" | |
| if strings.Contains(stdout, failureMsg) || | |
| strings.Contains(stderr, failureMsg) { | |
| return "", fmt.Errorf( | |
| "anyscale error: %s detected:\nstdout: %s\nstderr: %s", | |
| failureMsg, stdout, stderr, | |
| ) | |
| } |
Problem
The anyscale CLI sometimes exits with code 0 even when a remote
run_commandhas failed. In those cases it logs the failure via logrus to stderr, e.g.:
Previously,
runAnyscaleCLIinrayapp/anyscale_cli.goonly scanned stdoutfor the
exec failed with exit codemarker when the exit code was 0, so thesefailures were silently treated as success. (Non-zero exits were already handled
and did include stderr in the error message — the gap was specifically the
exit-0-with-stderr-marker case.)
Changes
rayapp/anyscale_cli.gostderrinto a local variable unconditionally (alongsidestdout)after
cmd.Run().stdoutandstderrfor"exec failed with exit code".for full diagnostic context.
rayapp/anyscale_cli_test.go"exec failed with exit code in stdout"."exec failed with exit code in stderr"that simulates thelogrus-style stderr output (
level=fatal msg="exec failed with exit code 1")with exit code 0, and asserts the error is still surfaced.
Uncaught failure: https://buildkite.com/anyscale/template-probe/builds/42/canvas?jid=019e1cb4-1046-4ee7-a113-790ea651edbf&tab=output#019e1cb4-1046-4ee7-a113-790ea651edbf/L712