Surface the real error when magic-mode flashing fails - #101
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs combined to make every magic-mode failure look like a generic "unknown error":
flash-from-urlran 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.goMagic()only ever setstate.Errorto a generic "An error was encountered during magic. Check log for details" message, which regular users can't act on.Fixed by capturing each pipeline stage's stderr separately (per-command redirect, not for the whole subshell) and using
PIPESTATUSto report whichever stage actually failed, and by havinggoMagic()surface that specific message to the UI instead of the generic one.Also dropped wget's
-qflag, which turned out to suppress its error output along with its progress noise.Testing
make test-bats- un-skipped and fixed the existing (pending) test for this issue; added astub_stderrtest helper since the real fix needed to distinguish stdout (test's canned fake data) from stderr (where wget's real diagnostics go).go test ./...- no regressions./api/start_magiccall:"Download failed: ... ERROR 404: Not Found.""Download failed: wget: unable to resolve host address '...'"Closes #59