Skip to content

Commit 1f5ef62

Browse files
fullstackjamclaude
andauthored
Improve error messages with context wrapping (#24)
* fix: wrap bare returned errors with context Add fmt.Errorf("context: %w", err) wrapping on previously bare `return err` sites in auth, shell, npm, and dotfiles. Per CLAUDE.md convention, callers should get contextual error chains. * test: skip filesystem-permission tests when running as root Tests that rely on chmod 0500/0000 to trigger permission denied silently succeed under root because root bypasses DAC checks, leaving `err == nil` where the test expected a non-nil error and panicking on the follow-up nil dereference. Guard these with `os.Geteuid() == 0` skips; they still run as a non-root user or in standard CI. * refactor: make brewInstallURL const The existing comment described it as a test seam, but no test reassigns it. A const prevents accidental mutation; if a test ever needs to override the URL, add a proper seam then. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 045ef77 commit 1f5ef62

9 files changed

Lines changed: 42 additions & 20 deletions

File tree

internal/auth/auth.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TokenPath() (string, error) {
2626
func LoadToken() (*StoredAuth, error) {
2727
path, err := TokenPath()
2828
if err != nil {
29-
return nil, err
29+
return nil, fmt.Errorf("load token: %w", err)
3030
}
3131
data, err := os.ReadFile(path)
3232
if err != nil {
@@ -51,7 +51,7 @@ func LoadToken() (*StoredAuth, error) {
5151
func SaveToken(auth *StoredAuth) error {
5252
path, err := TokenPath()
5353
if err != nil {
54-
return err
54+
return fmt.Errorf("save token: %w", err)
5555
}
5656

5757
dir := filepath.Dir(path)
@@ -74,7 +74,7 @@ func SaveToken(auth *StoredAuth) error {
7474
func DeleteToken() error {
7575
path, err := TokenPath()
7676
if err != nil {
77-
return err
77+
return fmt.Errorf("delete token: %w", err)
7878
}
7979
err = os.Remove(path)
8080
if err != nil && !os.IsNotExist(err) {

internal/auth/auth_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,10 @@ func TestDeleteToken_FileNotExist(t *testing.T) {
258258
}
259259

260260
func TestDeleteToken_PermissionError(t *testing.T) {
261+
if os.Geteuid() == 0 {
262+
t.Skip("root bypasses filesystem permission checks")
263+
}
264+
261265
tmpDir := t.TempDir()
262266
authDir := filepath.Join(tmpDir, ".openboot")
263267
authFile := filepath.Join(authDir, "auth.json")
@@ -378,6 +382,10 @@ func TestStoredAuth_JSONMarshaling(t *testing.T) {
378382
}
379383

380384
func TestLoadToken_ReadPermissionError(t *testing.T) {
385+
if os.Geteuid() == 0 {
386+
t.Skip("root bypasses filesystem permission checks")
387+
}
388+
381389
tmpDir := t.TempDir()
382390
authDir := filepath.Join(tmpDir, ".openboot")
383391
authFile := filepath.Join(authDir, "auth.json")

internal/auth/login_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,9 @@ func TestLoginInteractive_InvalidExpirationFormat(t *testing.T) {
521521
}
522522

523523
func TestLoginInteractive_SaveTokenError(t *testing.T) {
524+
if os.Geteuid() == 0 {
525+
t.Skip("root bypasses filesystem permission checks")
526+
}
524527
withFastPoll(t)
525528
withNoBrowser(t)
526529
tmpDir := t.TempDir()

internal/cli/login_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ func TestLogoutCmd_WhenNotAuthenticated(t *testing.T) {
9090
}
9191

9292
func TestLogoutCmd_DeleteError(t *testing.T) {
93+
if os.Geteuid() == 0 {
94+
t.Skip("root bypasses filesystem permission checks")
95+
}
9396
tmpDir := setupTestAuth(t, true)
9497
authDir := filepath.Join(tmpDir, ".openboot")
9598

internal/dotfiles/dotfiles.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,18 @@ func Clone(repoURL string, dryRun bool) error {
4343

4444
home, err := system.HomeDir()
4545
if err != nil {
46-
return err
46+
return fmt.Errorf("clone dotfiles: %w", err)
4747
}
4848
dotfilesPath := filepath.Join(home, defaultDotfilesDir)
4949

5050
if _, err := os.Stat(dotfilesPath); err == nil {
5151
// Dotfiles directory already exists — sync or re-clone as appropriate.
5252
needsClone, err := handleExistingDotfiles(dotfilesPath, repoURL, dryRun)
53-
if err != nil || !needsClone {
54-
return err
53+
if err != nil {
54+
return fmt.Errorf("handle existing dotfiles: %w", err)
55+
}
56+
if !needsClone {
57+
return nil
5558
}
5659
}
5760

@@ -210,7 +213,7 @@ func confirmResetIfDirty(dotfilesPath, branch string) bool {
210213
func Link(dryRun bool) error {
211214
home, err := system.HomeDir()
212215
if err != nil {
213-
return err
216+
return fmt.Errorf("link dotfiles: %w", err)
214217
}
215218
dotfilesPath := filepath.Join(home, defaultDotfilesDir)
216219

@@ -332,17 +335,17 @@ func backupConflicts(pkgDir, targetDir string) ([][2]string, error) {
332335

333336
func linkWithStow(dotfilesPath string, dryRun bool) error {
334337
if err := ensureStow(dryRun); err != nil {
335-
return err
338+
return fmt.Errorf("ensure stow: %w", err)
336339
}
337340

338341
entries, err := os.ReadDir(dotfilesPath)
339342
if err != nil {
340-
return err
343+
return fmt.Errorf("read dotfiles dir: %w", err)
341344
}
342345

343346
home, err := system.HomeDir()
344347
if err != nil {
345-
return err
348+
return fmt.Errorf("link with stow: %w", err)
346349
}
347350

348351
var errs []error
@@ -399,12 +402,12 @@ func linkWithStow(dotfilesPath string, dryRun bool) error {
399402
func linkDirect(dotfilesPath string, dryRun bool) error {
400403
home, err := system.HomeDir()
401404
if err != nil {
402-
return err
405+
return fmt.Errorf("link direct: %w", err)
403406
}
404407

405408
entries, err := os.ReadDir(dotfilesPath)
406409
if err != nil {
407-
return err
410+
return fmt.Errorf("read dotfiles dir: %w", err)
408411
}
409412

410413
for _, entry := range entries {

internal/npm/npm.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func Install(packages []string, dryRun bool) error {
124124

125125
failed, err := installBatch(toInstall)
126126
if err != nil {
127-
return err
127+
return fmt.Errorf("install npm packages: %w", err)
128128
}
129129

130130
if len(failed) > 0 {

internal/shell/shell.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func EnsureBrewShellenv(dryRun bool) error {
119119

120120
home, err := system.HomeDir()
121121
if err != nil {
122-
return err
122+
return fmt.Errorf("ensure brew shellenv: %w", err)
123123
}
124124
zshrcPath := filepath.Join(home, ".zshrc")
125125

@@ -129,7 +129,10 @@ func EnsureBrewShellenv(dryRun bool) error {
129129
fmt.Printf("[DRY-RUN] Would create %s with Homebrew shellenv\n", zshrcPath)
130130
return nil
131131
}
132-
return os.WriteFile(zshrcPath, []byte(brewShellenvLine+"\n"), 0600)
132+
if err := os.WriteFile(zshrcPath, []byte(brewShellenvLine+"\n"), 0600); err != nil {
133+
return fmt.Errorf("create .zshrc: %w", err)
134+
}
135+
return nil
133136
}
134137

135138
raw, err := os.ReadFile(zshrcPath)
@@ -262,7 +265,7 @@ func RestoreFromSnapshot(ohMyZsh bool, theme string, plugins []string, dryRun bo
262265

263266
home, err := system.HomeDir()
264267
if err != nil {
265-
return err
268+
return fmt.Errorf("configure zshrc: %w", err)
266269
}
267270
zshrcPath := filepath.Join(home, ".zshrc")
268271

@@ -272,11 +275,11 @@ func RestoreFromSnapshot(ohMyZsh bool, theme string, plugins []string, dryRun bo
272275
return nil
273276
}
274277
if err := validateShellIdentifier(theme, "ZSH_THEME"); err != nil {
275-
return err
278+
return fmt.Errorf("validate theme: %w", err)
276279
}
277280
for _, p := range plugins {
278281
if err := validateShellIdentifier(p, "plugin"); err != nil {
279-
return err
282+
return fmt.Errorf("validate plugin: %w", err)
280283
}
281284
}
282285
template := fmt.Sprintf(`export ZSH="$HOME/.oh-my-zsh"

internal/state/reminder_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ func TestRoundTrip_DefaultState(t *testing.T) {
176176
}
177177

178178
func TestLoadState_ReadError(t *testing.T) {
179+
if os.Geteuid() == 0 {
180+
t.Skip("root bypasses filesystem permission checks")
181+
}
179182
tmpDir := t.TempDir()
180183
statePath := filepath.Join(tmpDir, "state.json")
181184

internal/system/system.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,7 @@ func RunCommandOutput(name string, args ...string) (string, error) {
7777
// curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh | sha256sum
7878
const knownBrewInstallHash = "dfd5145fe2aa5956a600e35848765273f5798ce6def01bd08ecec088a1268d91"
7979

80-
// brewInstallURL is a var so tests can redirect it without a real server.
81-
var brewInstallURL = "https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh"
80+
const brewInstallURL = "https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh"
8281

8382
// brewHTTPClient is a var so tests can inject a mock transport.
8483
var brewHTTPClient *http.Client = http.DefaultClient

0 commit comments

Comments
 (0)