fix: surface update notice even when a command fails#39
Merged
Conversation
The update notifier ran only at the end of main(), so any command that exited via fatal() (e.g. `asobi health` against a down backend) called os.Exit before the notice - and os.Exit skips deferred work, so a main-level defer wouldn't help either. New users, whose early commands often fail, never got nudged to upgrade. Route every exit through a single exit() chokepoint that emits the notice first; fatal() and the direct os.Exit sites now go through it. Add a writer seam (notifyUpdateTo) and a hermetic test driving the failing- command path off a seeded cache.
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
1 participant
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.
Problem
update.Notifyran only at the tail ofmain(). Commands that fail exit viafatal()→os.Exit(1)before reaching it, andos.Exitskips deferred functions, so adeferinmainwouldn't fix it. Net effect: a brand-new user whose early commands fail (asobi healthwith no backend → connection refused, exactly what surfaced this) is never nudged to upgrade.Fix
Single exit chokepoint:
fatal()and the directos.Exit(1)sites now route throughexit(), so every termination path surfaces the notice. Skip rules (version/help/upgrade, CI,ASOBI_NO_UPDATE_CHECK, dev builds) are unchanged — they live inupdate.shouldCheck.Test
Added
notifyUpdateTo(io.Writer)seam +TestNotifyUpdateWiring: seeds a fresh cache with a newer release and asserts the failing-command path emits the notice, hermetically (no network).Verified end-to-end with a
-ldflags -X main.version=v0.0.1build: failinghealthprints the error thenA new asobi is available: v9.9.9;versionstays silent.