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.