From 6b3e61732a510aa3b796fd1390aa43a6e87e382a Mon Sep 17 00:00:00 2001 From: Snider Date: Mon, 2 Feb 2026 05:34:10 +0000 Subject: [PATCH] fix(io): add contextual error handling with E() helper Address CodeRabbit review feedback by wrapping raw errors with the errors.E() helper to provide service/action context for debugging: - pkg/cache: wrap cache.New, Get, Set, Delete, Clear errors - pkg/devops/test: wrap LoadTestConfig path/read/parse errors - pkg/cli/daemon: wrap PIDFile.Release path resolution error - pkg/container/state: wrap LoadState/SaveState errors - pkg/container/templates: wrap GetTemplate embedded/user read errors Co-Authored-By: Claude Opus 4.5 --- pkg/cache/cache.go | 34 ++++++++++++++++++++++------------ pkg/cli/daemon.go | 7 +++++-- pkg/container/state.go | 16 ++++++++++------ pkg/container/templates.go | 7 ++++--- pkg/devops/test.go | 7 ++++--- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index f660e421..6c8c8cea 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -7,6 +7,7 @@ import ( "path/filepath" "time" + "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/io" ) @@ -33,7 +34,7 @@ func New(baseDir string, ttl time.Duration) (*Cache, error) { // Use .core/cache in current working directory cwd, err := os.Getwd() if err != nil { - return nil, err + return nil, errors.E("cache.New", "failed to get working directory", err) } baseDir = filepath.Join(cwd, ".core", "cache") } @@ -45,12 +46,12 @@ func New(baseDir string, ttl time.Duration) (*Cache, error) { // Convert to absolute path for io.Local absBaseDir, err := filepath.Abs(baseDir) if err != nil { - return nil, err + return nil, errors.E("cache.New", "failed to resolve absolute path", err) } // Ensure cache directory exists if err := io.Local.EnsureDir(absBaseDir); err != nil { - return nil, err + return nil, errors.E("cache.New", "failed to create cache directory", err) } baseDir = absBaseDir @@ -75,7 +76,7 @@ func (c *Cache) Get(key string, dest interface{}) (bool, error) { if os.IsNotExist(err) { return false, nil } - return false, err + return false, errors.E("cache.Get", "failed to read cache file", err) } data := []byte(content) @@ -92,7 +93,7 @@ func (c *Cache) Get(key string, dest interface{}) (bool, error) { // Unmarshal the actual data if err := json.Unmarshal(entry.Data, dest); err != nil { - return false, err + return false, errors.E("cache.Get", "failed to unmarshal cache data", err) } return true, nil @@ -105,7 +106,7 @@ func (c *Cache) Set(key string, data interface{}) error { // Marshal the data dataBytes, err := json.Marshal(data) if err != nil { - return err + return errors.E("cache.Set", "failed to marshal data", err) } entry := Entry{ @@ -116,26 +117,35 @@ func (c *Cache) Set(key string, data interface{}) error { entryBytes, err := json.MarshalIndent(entry, "", " ") if err != nil { - return err + return errors.E("cache.Set", "failed to marshal cache entry", err) } // io.Local.Write creates parent directories automatically - return io.Local.Write(path, string(entryBytes)) + if err := io.Local.Write(path, string(entryBytes)); err != nil { + return errors.E("cache.Set", "failed to write cache file", err) + } + return nil } // Delete removes an item from the cache. func (c *Cache) Delete(key string) error { path := c.Path(key) err := io.Local.Delete(path) - if os.IsNotExist(err) { - return nil + if err != nil { + if os.IsNotExist(err) { + return nil + } + return errors.E("cache.Delete", "failed to delete cache file", err) } - return err + return nil } // Clear removes all cached items. func (c *Cache) Clear() error { - return io.Local.DeleteAll(c.baseDir) + if err := io.Local.DeleteAll(c.baseDir); err != nil { + return errors.E("cache.Clear", "failed to clear cache directory", err) + } + return nil } // Age returns how old a cached item is, or -1 if not cached. diff --git a/pkg/cli/daemon.go b/pkg/cli/daemon.go index bcee03c1..c6aa575a 100644 --- a/pkg/cli/daemon.go +++ b/pkg/cli/daemon.go @@ -124,9 +124,12 @@ func (p *PIDFile) Release() error { defer p.mu.Unlock() absPath, err := filepath.Abs(p.path) if err != nil { - return err + return fmt.Errorf("failed to resolve PID file path: %w", err) } - return io.Local.Delete(absPath) + if err := io.Local.Delete(absPath); err != nil { + return fmt.Errorf("failed to delete PID file: %w", err) + } + return nil } // Path returns the PID file path. diff --git a/pkg/container/state.go b/pkg/container/state.go index e99bb051..a5a60c32 100644 --- a/pkg/container/state.go +++ b/pkg/container/state.go @@ -6,6 +6,7 @@ import ( "path/filepath" "sync" + "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/io" ) @@ -60,7 +61,7 @@ func LoadState(filePath string) (*State, error) { absPath, err := filepath.Abs(filePath) if err != nil { - return nil, err + return nil, errors.E("container.LoadState", "failed to resolve state file path", err) } content, err := io.Local.Read(absPath) @@ -68,11 +69,11 @@ func LoadState(filePath string) (*State, error) { if os.IsNotExist(err) { return state, nil } - return nil, err + return nil, errors.E("container.LoadState", "failed to read state file", err) } if err := json.Unmarshal([]byte(content), state); err != nil { - return nil, err + return nil, errors.E("container.LoadState", "failed to parse state file", err) } return state, nil @@ -85,16 +86,19 @@ func (s *State) SaveState() error { absPath, err := filepath.Abs(s.filePath) if err != nil { - return err + return errors.E("container.SaveState", "failed to resolve state file path", err) } data, err := json.MarshalIndent(s, "", " ") if err != nil { - return err + return errors.E("container.SaveState", "failed to marshal state", err) } // io.Local.Write creates parent directories automatically - return io.Local.Write(absPath, string(data)) + if err := io.Local.Write(absPath, string(data)); err != nil { + return errors.E("container.SaveState", "failed to write state file", err) + } + return nil } // Add adds a container to the state and persists it. diff --git a/pkg/container/templates.go b/pkg/container/templates.go index 80ec3005..b553fce3 100644 --- a/pkg/container/templates.go +++ b/pkg/container/templates.go @@ -8,6 +8,7 @@ import ( "regexp" "strings" + "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/io" ) @@ -63,7 +64,7 @@ func GetTemplate(name string) (string, error) { if t.Name == name { content, err := embeddedTemplates.ReadFile(t.Path) if err != nil { - return "", fmt.Errorf("failed to read embedded template %s: %w", name, err) + return "", errors.E("container.GetTemplate", "failed to read embedded template", err) } return string(content), nil } @@ -76,13 +77,13 @@ func GetTemplate(name string) (string, error) { if io.Local.IsFile(templatePath) { content, err := io.Local.Read(templatePath) if err != nil { - return "", fmt.Errorf("failed to read user template %s: %w", name, err) + return "", errors.E("container.GetTemplate", "failed to read user template", err) } return content, nil } } - return "", fmt.Errorf("template not found: %s", name) + return "", errors.E("container.GetTemplate", "template not found: "+name, nil) } // ApplyTemplate applies variable substitution to a template. diff --git a/pkg/devops/test.go b/pkg/devops/test.go index e424472e..d1a6834c 100644 --- a/pkg/devops/test.go +++ b/pkg/devops/test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/io" "gopkg.in/yaml.v3" ) @@ -116,17 +117,17 @@ func LoadTestConfig(projectDir string) (*TestConfig, error) { path := filepath.Join(projectDir, ".core", "test.yaml") absPath, err := filepath.Abs(path) if err != nil { - return nil, err + return nil, errors.E("devops.LoadTestConfig", "failed to resolve path", err) } content, err := io.Local.Read(absPath) if err != nil { - return nil, err + return nil, errors.E("devops.LoadTestConfig", "failed to read test config", err) } var cfg TestConfig if err := yaml.Unmarshal([]byte(content), &cfg); err != nil { - return nil, err + return nil, errors.E("devops.LoadTestConfig", "failed to parse test config", err) } return &cfg, nil