From 96b55cfeb36b81d4b10548bcd864ef838ae5aafa Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Mon, 10 Aug 2026 21:54:02 +0200 Subject: [PATCH] Surface the real error when magic-mode flashing fails Two bugs combined to make every magic-mode failure look like a generic "unknown error": 1. flash-from-url ran the download/decompress pipeline as (wget | pv | xz) 2> /tmp/recore-flash-progress - redirecting the whole pipeline's stderr into the progress file swallowed wget's and xz's real error output along with it, leaving nothing useful anywhere. 2. Even what little did get logged never reached the UI: goMagic() only ever set state.Error to the generic "An error was encountered during magic. Check log for details" - regular users can't get to that log anyway. Fixed by capturing each pipeline stage's stderr separately (redirected per-command, not for the whole subshell) and using PIPESTATUS to report whichever stage actually failed, and by having goMagic() surface that specific message to the UI instead of the generic one. Also dropped wget's -q flag, which turned out to suppress its error output along with its progress noise - needed stdout quiet (that's the image data) but not stderr. Live-tested on real hardware via the actual /api/start_magic call: - 404 (real file missing) -> "Download failed: ... ERROR 404: Not Found." - unresolvable host (no internet/DNS) -> "Download failed: wget: unable to resolve host address '...'" Closes #59 --- bin/prod/flash-from-url | 33 ++++++++++++++++++++++++++++++++- reflash/server.go | 9 ++++++++- test/bats/flash-from-url.bats | 10 +++------- test/bats/helper.bash | 21 +++++++++++++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/bin/prod/flash-from-url b/bin/prod/flash-from-url index 9a27fd8..c74687d 100755 --- a/bin/prod/flash-from-url +++ b/bin/prod/flash-from-url @@ -31,7 +31,38 @@ info "Overwriting block device $OUTFILE" info "Umounting /dev/mmcblk*" mount | grep '/dev/mmcblk' | cut -f 1 -d' ' | xargs -t -r -n 1 umount || true info "Starting flashing..." -(wget -q -O - "$URL" | pv -f -n -b | xz -T 0 -d -c > "$OUTFILE") 2> /tmp/recore-flash-progress +# pv's progress goes to /tmp/recore-flash-progress for the UI to poll - but +# redirecting the whole pipeline's stderr there swallowed wget's and xz's +# real error output too, leaving only "an unknown error" for the operator +# (#59). Capture each stage's stderr separately instead, and report whichever +# stage actually failed via PIPESTATUS. +WGET_ERR="$(mktemp)" +XZ_ERR="$(mktemp)" +# -q also suppresses wget's error output, not just its progress noise - +# needs to be silent on stdout (that's the actual image data) but not +# on stderr, where the real error message would otherwise go missing. +if wget -O - "$URL" 2>"$WGET_ERR" | pv -f -n -b 2> /tmp/recore-flash-progress | xz -T 0 -d -c 2>"$XZ_ERR" > "$OUTFILE"; then + PIPE_STATUS=("${PIPESTATUS[@]}") + PIPE_OK=1 +else + PIPE_STATUS=("${PIPESTATUS[@]}") + PIPE_OK=0 +fi + +if [ "$PIPE_OK" -eq 0 ]; then + if [ "${PIPE_STATUS[0]}" -ne 0 ]; then + # tail -1 would grab wget's trailing blank line rather than the actual + # "ERROR 404: Not Found." line before it - take the last non-empty line. + info "Download failed: $(awk 'NF{line=$0} END{print line}' "$WGET_ERR")" + elif [ "${PIPE_STATUS[2]}" -ne 0 ]; then + info "Decompression failed: $(awk 'NF{line=$0} END{print line}' "$XZ_ERR")" + else + info "Flashing pipeline failed (exit codes: ${PIPE_STATUS[*]})" + fi + rm -f "$WGET_ERR" "$XZ_ERR" + exit 1 +fi +rm -f "$WGET_ERR" "$XZ_ERR" info "Flashing done" sync diff --git a/reflash/server.go b/reflash/server.go index 296b3f8..8b3c616 100644 --- a/reflash/server.go +++ b/reflash/server.go @@ -770,7 +770,14 @@ func goMagic(url string) { if err != nil { logError("Error encountered during magic: \n" + stdout) state.State = ERROR - state.Error = "An error was encountered during magic. Check log for details" + // flash-from-url prints the specific reason (e.g. the real download + // error, not just "an unknown error" - #59) as its last line before + // exiting non-zero. Surface that instead of a generic message. + state.Error = "An error was encountered during magic" + lines := strings.Split(strings.TrimSpace(stdout), "\n") + if lastLine := lines[len(lines)-1]; lastLine != "" { + state.Error = lastLine + } return } diff --git a/test/bats/flash-from-url.bats b/test/bats/flash-from-url.bats index 301bd65..fcaa1d7 100644 --- a/test/bats/flash-from-url.bats +++ b/test/bats/flash-from-url.bats @@ -40,16 +40,12 @@ teardown() { teardown_sandbox; } [ "$status" -ne 0 ] } -# Issue #59: the operator currently sees only "unknown error" because the -# download pipeline's stderr is redirected into /tmp/recore-flash-progress -# instead of being surfaced. Un-skip this once flash-from-url reports the real -# cause on stdout/in the log. -@test "flash-from-url: surfaces the real download error (#59 — pending fix)" { - skip "flash-from-url still redirects pipeline stderr to /tmp; un-skip when #59 is fixed" - stub wget 1 <<'ERR' +@test "flash-from-url: surfaces the real download error (#59)" { + stub_stderr wget 1 <<'ERR' wget: unable to resolve host address 'example' ERR stub_silent xz 0 run "$PROD_BIN/flash-from-url" http://example/image.img.xz + [ "$status" -ne 0 ] [[ "$output" == *"unable to resolve host"* ]] } diff --git a/test/bats/helper.bash b/test/bats/helper.bash index de3c5f7..97b253e 100644 --- a/test/bats/helper.bash +++ b/test/bats/helper.bash @@ -47,6 +47,27 @@ stub() { chmod +x "$SHIMDIR/$name" } +# stub_stderr NAME [EXIT_CODE] (canned stderr read from this function's stdin) +# Same as stub, but the heredoc body goes to stderr - for commands like wget +# that write their real diagnostics there, not stdout. +# +# stub_stderr wget 1 <<'ERR' +# wget: unable to resolve host address 'example' +# ERR +stub_stderr() { + local name="$1" code="${2:-0}" body + body="$(cat)" + { + echo '#!/usr/bin/env bash' + echo "echo \"$name \$*\" >> \"$CALLS\"" + echo "cat <<'__STUB_ERR__' >&2" + printf '%s\n' "$body" + echo '__STUB_ERR__' + echo "exit $code" + } > "$SHIMDIR/$name" + chmod +x "$SHIMDIR/$name" +} + # stub_silent NAME [EXIT_CODE] — a fake command with no stdout (records argv). # Handy for no-op system tools (mount, sleep, partprobe, ...) and for making a # command "fail" with a chosen exit code.