From c793925828df0d54bb83c96b13357da55209e697 Mon Sep 17 00:00:00 2001 From: Micheal Wilkinson Date: Sat, 21 Mar 2026 20:52:13 +0000 Subject: [PATCH] chore(go): wrap core filesystem errors with context --- internal/homesick/core/core.go | 116 ++++++++++++------------ internal/homesick/core/generate_test.go | 9 ++ internal/homesick/core/helpers_test.go | 34 +++++++ internal/homesick/core/list_test.go | 10 ++ internal/homesick/core/rc_test.go | 10 ++ internal/homesick/core/track_test.go | 12 +++ 6 files changed, 135 insertions(+), 56 deletions(-) diff --git a/internal/homesick/core/core.go b/internal/homesick/core/core.go index aef3dfc..7ad2183 100644 --- a/internal/homesick/core/core.go +++ b/internal/homesick/core/core.go @@ -99,7 +99,7 @@ func (a *App) Clone(uri string, destination string) error { func (a *App) List() error { if err := os.MkdirAll(a.ReposDir, 0o750); err != nil { - return err + return fmt.Errorf("ensure repos directory: %w", err) } var castles []string @@ -114,13 +114,13 @@ func (a *App) List() error { castleRoot := filepath.Dir(path) rel, err := filepath.Rel(a.ReposDir, castleRoot) if err != nil { - return err + return fmt.Errorf("resolve castle path %q: %w", castleRoot, err) } castles = append(castles, rel) return filepath.SkipDir }) if err != nil { - return err + return fmt.Errorf("scan repos directory: %w", err) } sort.Strings(castles) @@ -132,7 +132,7 @@ func (a *App) List() error { } _, writeErr := fmt.Fprintf(a.Stdout, "%s %s\n", castle, strings.TrimSpace(remote)) if writeErr != nil { - return writeErr + return fmt.Errorf("write castle listing: %w", writeErr) } } @@ -234,13 +234,13 @@ func (a *App) Destroy(castle string) error { if errors.Is(err, os.ErrNotExist) { return fmt.Errorf("castle %q not found", castle) } - return err + return fmt.Errorf("stat castle %q: %w", castle, err) } if !a.Force { confirmed, confirmErr := a.confirmDestroy(castle) if confirmErr != nil { - return confirmErr + return fmt.Errorf("confirm destroy for %q: %w", castle, confirmErr) } if !confirmed { return nil @@ -252,7 +252,7 @@ func (a *App) Destroy(castle string) error { castleHome := filepath.Join(castleRoot, "home") if info, statErr := os.Stat(castleHome); statErr == nil && info.IsDir() { if unlinkErr := a.UnlinkCastle(castle); unlinkErr != nil { - return unlinkErr + return fmt.Errorf("unlink castle %q before destroy: %w", castle, unlinkErr) } } } @@ -267,12 +267,12 @@ func (a *App) confirmDestroy(castle string) (bool, error) { } if _, err := fmt.Fprintf(a.Stdout, "Destroy castle %q? [y/N]: ", castle); err != nil { - return false, err + return false, fmt.Errorf("write destroy prompt: %w", err) } line, err := bufio.NewReader(reader).ReadString('\n') if err != nil && !errors.Is(err, io.EOF) { - return false, err + return false, fmt.Errorf("read destroy confirmation: %w", err) } return isAffirmativeResponse(line), nil @@ -392,15 +392,15 @@ func (a *App) Generate(castlePath string) error { absCastle, err := filepath.Abs(trimmed) if err != nil { - return err + return fmt.Errorf("resolve castle path %q: %w", trimmed, err) } if err := os.MkdirAll(absCastle, 0o750); err != nil { - return err + return fmt.Errorf("create castle path %q: %w", absCastle, err) } if err := a.runGit(absCastle, "init"); err != nil { - return err + return fmt.Errorf("initialize git repository %q: %w", absCastle, err) } githubUser := "" @@ -412,11 +412,15 @@ func (a *App) Generate(castlePath string) error { repoName := filepath.Base(absCastle) url := fmt.Sprintf("git@github.com:%s/%s.git", githubUser, repoName) if err := a.runGit(absCastle, "remote", "add", "origin", url); err != nil { - return err + return fmt.Errorf("add origin remote for %q: %w", absCastle, err) } } - return os.MkdirAll(filepath.Join(absCastle, "home"), 0o750) + if err := os.MkdirAll(filepath.Join(absCastle, "home"), 0o750); err != nil { + return fmt.Errorf("create home directory for %q: %w", absCastle, err) + } + + return nil } func (a *App) Link(castle string) error { @@ -435,11 +439,11 @@ func (a *App) LinkCastle(castle string) error { subdirs, err := readSubdirs(filepath.Join(a.ReposDir, castle, ".homesick_subdir")) if err != nil { - return err + return fmt.Errorf("read subdirs for castle %q: %w", castle, err) } if err := a.linkEach(castleHome, castleHome, subdirs); err != nil { - return err + return fmt.Errorf("link castle %q: %w", castle, err) } for _, subdir := range subdirs { @@ -448,11 +452,11 @@ func (a *App) LinkCastle(castle string) error { if errors.Is(err, os.ErrNotExist) { continue } - return err + return fmt.Errorf("stat subdir %q for castle %q: %w", base, castle, err) } if err := a.linkEach(castleHome, base, subdirs); err != nil { - return err + return fmt.Errorf("link subdir %q for castle %q: %w", subdir, castle, err) } } @@ -475,11 +479,11 @@ func (a *App) UnlinkCastle(castle string) error { subdirs, err := readSubdirs(filepath.Join(a.ReposDir, castle, ".homesick_subdir")) if err != nil { - return err + return fmt.Errorf("read subdirs for castle %q: %w", castle, err) } if err := a.unlinkEach(castleHome, castleHome, subdirs); err != nil { - return err + return fmt.Errorf("unlink castle %q: %w", castle, err) } for _, subdir := range subdirs { @@ -488,11 +492,11 @@ func (a *App) UnlinkCastle(castle string) error { if errors.Is(err, os.ErrNotExist) { continue } - return err + return fmt.Errorf("stat subdir %q for castle %q: %w", base, castle, err) } if err := a.unlinkEach(castleHome, base, subdirs); err != nil { - return err + return fmt.Errorf("unlink subdir %q for castle %q: %w", subdir, castle, err) } } @@ -522,15 +526,15 @@ func (a *App) TrackPath(filePath string, castle string) error { absolutePath, err := filepath.Abs(strings.TrimRight(trimmedFile, string(filepath.Separator))) if err != nil { - return err + return fmt.Errorf("resolve tracked file %q: %w", trimmedFile, err) } if _, err := os.Lstat(absolutePath); err != nil { - return err + return fmt.Errorf("stat tracked file %q: %w", absolutePath, err) } relativeDir, err := filepath.Rel(a.HomeDir, filepath.Dir(absolutePath)) if err != nil { - return err + return fmt.Errorf("resolve tracked file directory for %q: %w", absolutePath, err) } if relativeDir == ".." || strings.HasPrefix(relativeDir, ".."+string(filepath.Separator)) { return fmt.Errorf("track requires file under %s", a.HomeDir) @@ -541,18 +545,18 @@ func (a *App) TrackPath(filePath string, castle string) error { castleTargetDir = castleHome } if err := os.MkdirAll(castleTargetDir, 0o750); err != nil { - return err + return fmt.Errorf("create tracked file directory %q: %w", castleTargetDir, err) } trackedPath := filepath.Join(castleTargetDir, filepath.Base(absolutePath)) if _, err := os.Lstat(trackedPath); err == nil { return fmt.Errorf("%s already exists", trackedPath) } else if !errors.Is(err, os.ErrNotExist) { - return err + return fmt.Errorf("stat tracked destination %q: %w", trackedPath, err) } if err := os.Rename(absolutePath, trackedPath); err != nil { - return err + return fmt.Errorf("move tracked file into castle %q: %w", trackedPath, err) } subdirChanged := false @@ -560,21 +564,21 @@ func (a *App) TrackPath(filePath string, castle string) error { subdirPath := filepath.Join(castleRoot, ".homesick_subdir") subdirChanged, err = appendUniqueSubdir(subdirPath, relativeDir) if err != nil { - return err + return fmt.Errorf("record tracked subdir %q: %w", relativeDir, err) } } if err := a.linkPath(trackedPath, absolutePath); err != nil { - return err + return fmt.Errorf("relink tracked file %q: %w", absolutePath, err) } repo, err := git.PlainOpen(castleRoot) if err != nil { - return err + return fmt.Errorf("open git repository for castle %q: %w", castle, err) } worktree, err := repo.Worktree() if err != nil { - return err + return fmt.Errorf("open worktree for castle %q: %w", castle, err) } trackedRelativePath := filepath.Join("home", relativeDir, filepath.Base(absolutePath)) @@ -582,12 +586,12 @@ func (a *App) TrackPath(filePath string, castle string) error { trackedRelativePath = filepath.Join("home", filepath.Base(absolutePath)) } if _, err := worktree.Add(filepath.ToSlash(filepath.Clean(trackedRelativePath))); err != nil { - return err + return fmt.Errorf("stage tracked file %q: %w", trackedRelativePath, err) } if subdirChanged { if _, err := worktree.Add(".homesick_subdir"); err != nil { - return err + return fmt.Errorf("stage subdir metadata: %w", err) } } @@ -597,7 +601,7 @@ func (a *App) TrackPath(filePath string, castle string) error { func appendUniqueSubdir(path string, subdir string) (bool, error) { existing, err := readSubdirs(path) if err != nil { - return false, err + return false, fmt.Errorf("load subdir metadata %q: %w", path, err) } cleanSubdir := filepath.Clean(subdir) @@ -609,12 +613,12 @@ func appendUniqueSubdir(path string, subdir string) (bool, error) { file, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) // #nosec G304 — internal metadata file if err != nil { - return false, err + return false, fmt.Errorf("open subdir metadata %q: %w", path, err) } defer file.Close() if _, err := file.WriteString(cleanSubdir + "\n"); err != nil { - return false, err + return false, fmt.Errorf("write subdir metadata %q: %w", path, err) } return true, nil @@ -623,7 +627,7 @@ func appendUniqueSubdir(path string, subdir string) (bool, error) { func (a *App) linkEach(castleHome string, baseDir string, subdirs []string) error { entries, err := os.ReadDir(baseDir) if err != nil { - return err + return fmt.Errorf("read castle directory %q: %w", baseDir, err) } for _, entry := range entries { @@ -635,7 +639,7 @@ func (a *App) linkEach(castleHome string, baseDir string, subdirs []string) erro source := filepath.Join(baseDir, name) ignore, err := matchesIgnoredDir(castleHome, source, subdirs) if err != nil { - return err + return fmt.Errorf("check ignored directory %q: %w", source, err) } if ignore { continue @@ -643,7 +647,7 @@ func (a *App) linkEach(castleHome string, baseDir string, subdirs []string) erro relDir, err := filepath.Rel(castleHome, baseDir) if err != nil { - return err + return fmt.Errorf("resolve castle relative directory %q: %w", baseDir, err) } destination := filepath.Join(a.HomeDir, relDir, name) @@ -652,7 +656,7 @@ func (a *App) linkEach(castleHome string, baseDir string, subdirs []string) erro } if err := a.linkPath(source, destination); err != nil { - return err + return fmt.Errorf("link %q to %q: %w", source, destination, err) } } @@ -662,7 +666,7 @@ func (a *App) linkEach(castleHome string, baseDir string, subdirs []string) erro func (a *App) unlinkEach(castleHome string, baseDir string, subdirs []string) error { entries, err := os.ReadDir(baseDir) if err != nil { - return err + return fmt.Errorf("read castle directory %q: %w", baseDir, err) } for _, entry := range entries { @@ -674,7 +678,7 @@ func (a *App) unlinkEach(castleHome string, baseDir string, subdirs []string) er source := filepath.Join(baseDir, name) ignore, err := matchesIgnoredDir(castleHome, source, subdirs) if err != nil { - return err + return fmt.Errorf("check ignored directory %q: %w", source, err) } if ignore { continue @@ -682,7 +686,7 @@ func (a *App) unlinkEach(castleHome string, baseDir string, subdirs []string) er relDir, err := filepath.Rel(castleHome, baseDir) if err != nil { - return err + return fmt.Errorf("resolve castle relative directory %q: %w", baseDir, err) } destination := filepath.Join(a.HomeDir, relDir, name) @@ -691,7 +695,7 @@ func (a *App) unlinkEach(castleHome string, baseDir string, subdirs []string) er } if err := unlinkPath(destination); err != nil { - return err + return fmt.Errorf("unlink %q: %w", destination, err) } } @@ -717,11 +721,11 @@ func unlinkPath(destination string) error { func (a *App) linkPath(source string, destination string) error { absSource, err := filepath.Abs(source) if err != nil { - return err + return fmt.Errorf("resolve link source %q: %w", source, err) } if err := os.MkdirAll(filepath.Dir(destination), 0o750); err != nil { - return err + return fmt.Errorf("create destination parent %q: %w", filepath.Dir(destination), err) } info, err := os.Lstat(destination) @@ -738,14 +742,14 @@ func (a *App) linkPath(source string, destination string) error { } if rmErr := os.RemoveAll(destination); rmErr != nil { - return rmErr + return fmt.Errorf("remove existing destination %q: %w", destination, rmErr) } } else if !errors.Is(err, os.ErrNotExist) { - return err + return fmt.Errorf("stat destination %q: %w", destination, err) } if err := os.Symlink(absSource, destination); err != nil { - return err + return fmt.Errorf("create symlink %q -> %q: %w", destination, absSource, err) } return nil @@ -757,7 +761,7 @@ func readSubdirs(path string) ([]string, error) { if errors.Is(err, os.ErrNotExist) { return []string{}, nil } - return nil, err + return nil, fmt.Errorf("read subdirs %q: %w", path, err) } lines := strings.Split(string(data), "\n") @@ -776,7 +780,7 @@ func readSubdirs(path string) ([]string, error) { func matchesIgnoredDir(castleHome string, candidate string, subdirs []string) (bool, error) { absCandidate, err := filepath.Abs(candidate) if err != nil { - return false, err + return false, fmt.Errorf("resolve candidate path %q: %w", candidate, err) } ignoreSet := map[string]struct{}{} @@ -853,7 +857,7 @@ func (a *App) Rc(castle string, force bool) error { if errors.Is(err, os.ErrNotExist) { return fmt.Errorf("castle %q not found", castle) } - return err + return fmt.Errorf("stat castle %q: %w", castle, err) } homesickD := filepath.Join(castleRoot, ".homesick.d") @@ -890,12 +894,12 @@ func (a *App) Rc(castle string, force bool) error { if errors.Is(err, os.ErrNotExist) { return nil } - return err + return fmt.Errorf("stat rc hooks directory %q: %w", homesickD, err) } entries, err := os.ReadDir(homesickD) if err != nil { - return err + return fmt.Errorf("read rc hooks %q: %w", homesickD, err) } // ReadDir returns entries in sorted order already. @@ -905,7 +909,7 @@ func (a *App) Rc(castle string, force bool) error { } info, infoErr := entry.Info() if infoErr != nil { - return infoErr + return fmt.Errorf("read rc hook metadata %q: %w", entry.Name(), infoErr) } if info.Mode()&0o111 == 0 { // Not executable — skip. diff --git a/internal/homesick/core/generate_test.go b/internal/homesick/core/generate_test.go index 76b4ad6..344222e 100644 --- a/internal/homesick/core/generate_test.go +++ b/internal/homesick/core/generate_test.go @@ -67,3 +67,12 @@ func (s *GenerateSuite) TestGenerate_DoesNotAddOriginWhenGitHubUserMissing() { require.NoError(s.T(), err) require.NotContains(s.T(), string(content), "[remote \"origin\"]") } + +func (s *GenerateSuite) TestGenerate_WrapsCastlePathCreationError() { + blocker := filepath.Join(s.tmpDir, "blocker") + require.NoError(s.T(), os.WriteFile(blocker, []byte("x"), 0o644)) + + err := s.app.Generate(filepath.Join(blocker, "castle")) + require.Error(s.T(), err) + require.Contains(s.T(), err.Error(), "create castle path") +} diff --git a/internal/homesick/core/helpers_test.go b/internal/homesick/core/helpers_test.go index a2db849..cea2397 100644 --- a/internal/homesick/core/helpers_test.go +++ b/internal/homesick/core/helpers_test.go @@ -17,6 +17,12 @@ func (errReader) Read(_ []byte) (int, error) { return 0, errors.New("boom") } +type errWriter struct{} + +func (errWriter) Write(_ []byte) (int, error) { + return 0, errors.New("boom") +} + func TestRunGitPretendWritesStatus(t *testing.T) { stdout := &bytes.Buffer{} app := &App{Stdout: stdout, Stderr: bytes.NewBuffer(nil), Pretend: true} @@ -119,6 +125,19 @@ func TestLinkPath(t *testing.T) { require.NoError(t, statErr) require.True(t, info.Mode()&os.ModeSymlink != 0) }) + + t.Run("create destination parent error includes context", func(t *testing.T) { + dir := t.TempDir() + source := filepath.Join(dir, "source") + blocker := filepath.Join(dir, "blocker") + require.NoError(t, os.WriteFile(source, []byte("x"), 0o644)) + require.NoError(t, os.WriteFile(blocker, []byte("x"), 0o644)) + + app := &App{Stdout: bytes.NewBuffer(nil), Stderr: bytes.NewBuffer(nil)} + err := app.linkPath(source, filepath.Join(blocker, "dest")) + require.Error(t, err) + require.Contains(t, err.Error(), "create destination parent") + }) } func TestReadSubdirsAndMatchesIgnoredDir(t *testing.T) { @@ -141,6 +160,12 @@ func TestReadSubdirsAndMatchesIgnoredDir(t *testing.T) { require.False(t, notIgnored) } +func TestReadSubdirsReadErrorIncludesContext(t *testing.T) { + _, err := readSubdirs(t.TempDir()) + require.Error(t, err) + require.Contains(t, err.Error(), "read subdirs") +} + func TestPullAndPushDefaultCastlePretend(t *testing.T) { dir := t.TempDir() stdout := &bytes.Buffer{} @@ -230,6 +255,15 @@ func TestConfirmDestroyReadError(t *testing.T) { ok, err := app.confirmDestroy("dotfiles") require.Error(t, err) require.False(t, ok) + require.Contains(t, err.Error(), "read destroy confirmation") +} + +func TestConfirmDestroyWriteError(t *testing.T) { + app := &App{Stdout: errWriter{}, Stdin: strings.NewReader("yes\n")} + ok, err := app.confirmDestroy("dotfiles") + require.Error(t, err) + require.False(t, ok) + require.Contains(t, err.Error(), "write destroy prompt") } func TestExecAllWrapsCastleError(t *testing.T) { diff --git a/internal/homesick/core/list_test.go b/internal/homesick/core/list_test.go index b7ac242..69cf225 100644 --- a/internal/homesick/core/list_test.go +++ b/internal/homesick/core/list_test.go @@ -70,3 +70,13 @@ func (s *ListSuite) TestList_OutputsSortedCastlesWithRemoteURLs() { s.stdout.String(), ) } + +func (s *ListSuite) TestList_WrapsReposDirCreationError() { + blocker := filepath.Join(s.tmpDir, "repos-blocker") + require.NoError(s.T(), os.WriteFile(blocker, []byte("x"), 0o644)) + s.app.ReposDir = filepath.Join(blocker, "repos") + + err := s.app.List() + require.Error(s.T(), err) + require.Contains(s.T(), err.Error(), "ensure repos directory") +} diff --git a/internal/homesick/core/rc_test.go b/internal/homesick/core/rc_test.go index ba54ed8..b10a877 100644 --- a/internal/homesick/core/rc_test.go +++ b/internal/homesick/core/rc_test.go @@ -230,3 +230,13 @@ func (s *RcSuite) TestRc_ScriptsRunWithCwdSetToCastleRoot() { require.NoError(s.T(), s.app.Rc("dotfiles", false)) require.Contains(s.T(), s.stdout.String(), castleRoot) } + +func (s *RcSuite) TestRc_ReadHooksErrorIncludesContext() { + castleRoot := s.createCastle("dotfiles") + homesickD := filepath.Join(castleRoot, ".homesick.d") + require.NoError(s.T(), os.WriteFile(homesickD, []byte("x"), 0o644)) + + err := s.app.Rc("dotfiles", false) + require.Error(s.T(), err) + require.Contains(s.T(), err.Error(), "read rc hooks") +} diff --git a/internal/homesick/core/track_test.go b/internal/homesick/core/track_test.go index ccf68c1..4ee914f 100644 --- a/internal/homesick/core/track_test.go +++ b/internal/homesick/core/track_test.go @@ -99,3 +99,15 @@ func (s *TrackSuite) TestTrack_DefaultCastleName() { require.NoError(s.T(), err) require.Equal(s.T(), expectedTarget, linkTarget) } + +func (s *TrackSuite) TestTrack_WrapsSubdirRecordingError() { + castleRoot := s.createCastleRepo("dotfiles") + require.NoError(s.T(), os.MkdirAll(filepath.Join(castleRoot, ".homesick_subdir"), 0o755)) + + filePath := filepath.Join(s.homeDir, ".config", "myapp", "config.toml") + s.writeFile(filePath, "ok=true\n") + + err := s.app.Track(filePath, "dotfiles") + require.Error(s.T(), err) + require.Contains(s.T(), err.Error(), "record tracked subdir") +}