From 4014fd2dc3ec499235210d1ded55c2ec44663f0e Mon Sep 17 00:00:00 2001 From: Snider Date: Mon, 2 Feb 2026 06:48:40 +0000 Subject: [PATCH] feat(errors): Unify errors and logging (#180) * feat(help): Add CLI help command Fixes #136 * chore: remove binary * feat(mcp): Add TCP transport Fixes #126 * feat(io): Migrate pkg/mcp to use Medium abstraction Fixes #103 * feat(io): batch implementation placeholder Co-Authored-By: Claude Opus 4.5 * feat(errors): batch implementation placeholder Co-Authored-By: Claude Opus 4.5 * feat(log): batch implementation placeholder Co-Authored-By: Claude Opus 4.5 * chore(io): Migrate internal/cmd/docs/* to Medium abstraction Fixes #113 * chore(io): Migrate internal/cmd/dev/* to Medium abstraction Fixes #114 * chore(io): Migrate internal/cmd/setup/* to Medium abstraction * chore(io): Complete migration of internal/cmd/dev/* to Medium abstraction * feat(io): extend Medium interface with Delete, Rename, List, Stat operations Adds the following methods to the Medium interface: - Delete(path) - remove a file or empty directory - DeleteAll(path) - recursively remove a file or directory - Rename(old, new) - move/rename a file or directory - List(path) - list directory entries (returns []fs.DirEntry) - Stat(path) - get file information (returns fs.FileInfo) - Exists(path) - check if path exists - IsDir(path) - check if path is a directory Implements these methods in both local.Medium (using os package) and MockMedium (in-memory for testing). Includes FileInfo and DirEntry types for mock implementations. This enables migration of direct os.* calls to the Medium abstraction for consistent path validation and testability. Refs #101 Co-Authored-By: Claude Opus 4.5 * chore(io): Migrate internal/cmd/sdk, pkgcmd, and workspace to Medium abstraction * chore(io): migrate internal/cmd/docs and internal/cmd/dev to Medium - internal/cmd/docs: Replace os.Stat, os.ReadFile, os.WriteFile, os.MkdirAll, os.RemoveAll with io.Local equivalents - internal/cmd/dev: Replace os.Stat, os.ReadFile, os.WriteFile, os.MkdirAll, os.ReadDir with io.Local equivalents - Fix local.Medium to allow absolute paths when root is "/" for full filesystem access (io.Local use case) Refs #113, #114 Co-Authored-By: Claude Opus 4.5 * chore(io): migrate internal/cmd/setup to Medium abstraction Migrated all direct os.* filesystem calls to use io.Local: - cmd_repo.go: os.MkdirAll -> io.Local.EnsureDir, os.WriteFile -> io.Local.Write, os.Stat -> io.Local.IsFile - cmd_bootstrap.go: os.MkdirAll -> io.Local.EnsureDir, os.Stat -> io.Local.IsDir/Exists, os.ReadDir -> io.Local.List - cmd_registry.go: os.MkdirAll -> io.Local.EnsureDir, os.Stat -> io.Local.Exists - cmd_ci.go: os.ReadFile -> io.Local.Read - github_config.go: os.ReadFile -> io.Local.Read, os.Stat -> io.Local.Exists Refs #116 Co-Authored-By: Claude Opus 4.5 * feat(log): add error creation and log-and-return helpers Implements issues #129 and #132: - Add Err struct with Op, Msg, Err, Code fields for structured errors - Add E(), Wrap(), WrapCode(), NewCode() for error creation - Add Is(), As(), NewError(), Join() as stdlib wrappers - Add Op(), ErrCode(), Message(), Root() for introspection - Add LogError(), LogWarn(), Must() for combined log-and-return Closes #129 Closes #132 Co-Authored-By: Claude Opus 4.5 * chore(errors): create deprecation alias pointing to pkg/log Makes pkg/errors a thin compatibility layer that re-exports from pkg/log. All error handling functions now have canonical implementations in pkg/log. Migration guide in package documentation: - errors.Error -> log.Err - errors.E -> log.E - errors.Code -> log.NewCode - errors.New -> log.NewError Fixes behavior consistency: - E(op, msg, nil) now creates an error (for errors without cause) - Wrap(nil, op, msg) returns nil (for conditional wrapping) - WrapCode returns nil only when both err is nil AND code is empty Closes #128 Co-Authored-By: Claude Opus 4.5 * chore(log): migrate pkg/errors imports to pkg/log Migrates all internal packages from pkg/errors to pkg/log: - internal/cmd/monitor - internal/cmd/qa - internal/cmd/dev - pkg/agentic Closes #130 Co-Authored-By: Claude Opus 4.5 * fix(io): address Copilot review feedback - Fix MockMedium.Rename: collect keys before mutating maps during iteration - Fix .git checks to use Exists instead of List (handles worktrees/submodules) - Fix cmd_sync.go: use DeleteAll for recursive directory removal Files updated: - pkg/io/io.go: safe map iteration in Rename - internal/cmd/setup/cmd_bootstrap.go: Exists for .git checks - internal/cmd/setup/cmd_registry.go: Exists for .git checks - internal/cmd/pkgcmd/cmd_install.go: Exists for .git checks - internal/cmd/pkgcmd/cmd_manage.go: Exists for .git checks - internal/cmd/docs/cmd_sync.go: DeleteAll for recursive delete Co-Authored-By: Claude Opus 4.5 * fix(updater): resolve PkgVersion duplicate declaration Remove var PkgVersion from updater.go since go generate creates const PkgVersion in version.go. Track version.go in git to ensure builds work without running go generate first. Co-Authored-By: Claude Opus 4.5 * style: fix formatting in internal/variants Co-Authored-By: Claude Opus 4.5 * style: fix formatting across migrated files Co-Authored-By: Claude Opus 4.5 * refactor(io): simplify local Medium implementation Rewrote to match the simpler TypeScript pattern: - path() sanitizes and returns string directly - Each method calls path() once - No complex symlink validation - Less code, less attack surface Co-Authored-By: Claude Opus 4.5 * fix(io): remove duplicate method declarations Clean up the client.go file that had duplicate method declarations from a bad cherry-pick merge. Now has 127 lines of simple, clean code. Co-Authored-By: Claude Opus 4.5 * test(io): fix traversal test to match sanitization behavior The simplified path() sanitizes .. to . without returning errors. Update test to verify sanitization works correctly. Co-Authored-By: Claude Opus 4.5 * test(mcp): update sandboxing tests for simplified Medium The simplified io/local.Medium implementation: - Sanitizes .. to . (no error, path is cleaned) - Allows absolute paths through (caller validates if needed) - Follows symlinks (no traversal blocking) Update tests to match this simplified behavior. Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 --- internal/cmd/dev/cmd_file_sync.go | 10 +- internal/cmd/docs/cmd_sync.go | 4 +- internal/cmd/monitor/cmd_monitor.go | 26 +-- internal/cmd/pkgcmd/cmd_install.go | 2 +- internal/cmd/pkgcmd/cmd_manage.go | 7 +- internal/cmd/qa/cmd_health.go | 8 +- internal/cmd/qa/cmd_issues.go | 8 +- internal/cmd/qa/cmd_review.go | 10 +- internal/cmd/qa/cmd_watch.go | 14 +- internal/cmd/setup/cmd_bootstrap.go | 6 +- internal/cmd/setup/cmd_registry.go | 6 +- pkg/agentic/client.go | 58 +++--- pkg/agentic/completion.go | 34 ++-- pkg/agentic/config.go | 18 +- pkg/agentic/context.go | 10 +- pkg/errors/errors.go | 141 ++++++-------- pkg/io/client_test.go | 121 ++++++++++++ pkg/io/io.go | 288 ++++++++++++++++++++++++---- pkg/io/local/client_test.go | 198 +++++++++++++++++++ pkg/log/errors.go | 219 +++++++++++++++++++++ pkg/log/errors_test.go | 274 ++++++++++++++++++++++++++ 21 files changed, 1236 insertions(+), 226 deletions(-) create mode 100644 pkg/log/errors.go create mode 100644 pkg/log/errors_test.go diff --git a/internal/cmd/dev/cmd_file_sync.go b/internal/cmd/dev/cmd_file_sync.go index 4886683a..81d4b96e 100644 --- a/internal/cmd/dev/cmd_file_sync.go +++ b/internal/cmd/dev/cmd_file_sync.go @@ -15,10 +15,10 @@ import ( "strings" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/git" "github.com/host-uk/core/pkg/i18n" coreio "github.com/host-uk/core/pkg/io" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -59,7 +59,7 @@ func runFileSync(source string) error { // Security: Reject path traversal attempts if strings.Contains(source, "..") { - return errors.E("dev.sync", "path traversal not allowed", nil) + return log.E("dev.sync", "path traversal not allowed", nil) } // Validate source exists @@ -82,7 +82,7 @@ func runFileSync(source string) error { // Let's stick to os.Stat for source properties finding as typically allowed for CLI args. if err != nil { - return errors.E("dev.sync", i18n.T("cmd.dev.file_sync.error.source_not_found", map[string]interface{}{"Path": source}), err) + return log.E("dev.sync", i18n.T("cmd.dev.file_sync.error.source_not_found", map[string]interface{}{"Path": source}), err) } // Find target repos @@ -205,12 +205,12 @@ func resolveTargetRepos(pattern string) ([]*repos.Repo, error) { // Load registry registryPath, err := repos.FindRegistry() if err != nil { - return nil, errors.E("dev.sync", "failed to find registry", err) + return nil, log.E("dev.sync", "failed to find registry", err) } registry, err := repos.LoadRegistry(registryPath) if err != nil { - return nil, errors.E("dev.sync", "failed to load registry", err) + return nil, log.E("dev.sync", "failed to load registry", err) } // Match pattern against repo names diff --git a/internal/cmd/docs/cmd_sync.go b/internal/cmd/docs/cmd_sync.go index 2cbfb4dc..a1611056 100644 --- a/internal/cmd/docs/cmd_sync.go +++ b/internal/cmd/docs/cmd_sync.go @@ -126,8 +126,8 @@ func runDocsSync(registryPath string, outputDir string, dryRun bool) error { outName := packageOutputName(info.Name) repoOutDir := filepath.Join(outputDir, outName) - // Clear existing directory - io.Local.Delete(repoOutDir) // Recursive delete + // Clear existing directory (recursively) + _ = io.Local.DeleteAll(repoOutDir) if err := io.Local.EnsureDir(repoOutDir); err != nil { cli.Print(" %s %s: %s\n", errorStyle.Render("✗"), info.Name, err) diff --git a/internal/cmd/monitor/cmd_monitor.go b/internal/cmd/monitor/cmd_monitor.go index 62f4b680..847fc991 100644 --- a/internal/cmd/monitor/cmd_monitor.go +++ b/internal/cmd/monitor/cmd_monitor.go @@ -17,8 +17,8 @@ import ( "strings" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -107,7 +107,7 @@ type SecretScanningAlert struct { func runMonitor() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("monitor", i18n.T("error.gh_not_found"), err) + return log.E("monitor", i18n.T("error.gh_not_found"), err) } // Determine repos to scan @@ -117,7 +117,7 @@ func runMonitor() error { } if len(repoList) == 0 { - return errors.E("monitor", i18n.T("cmd.monitor.error.no_repos"), nil) + return log.E("monitor", i18n.T("cmd.monitor.error.no_repos"), nil) } // Collect all findings and errors @@ -179,12 +179,12 @@ func resolveRepos() ([]string, error) { // All repos from registry registry, err := repos.FindRegistry() if err != nil { - return nil, errors.E("monitor", "failed to find registry", err) + return nil, log.E("monitor", "failed to find registry", err) } loaded, err := repos.LoadRegistry(registry) if err != nil { - return nil, errors.E("monitor", "failed to load registry", err) + return nil, log.E("monitor", "failed to load registry", err) } var repoList []string @@ -253,12 +253,12 @@ func fetchCodeScanningAlerts(repoFullName string) ([]Finding, error) { return nil, nil } } - return nil, errors.E("monitor.fetchCodeScanning", "API request failed", err) + return nil, log.E("monitor.fetchCodeScanning", "API request failed", err) } var alerts []CodeScanningAlert if err := json.Unmarshal(output, &alerts); err != nil { - return nil, errors.E("monitor.fetchCodeScanning", "failed to parse response", err) + return nil, log.E("monitor.fetchCodeScanning", "failed to parse response", err) } repoName := strings.Split(repoFullName, "/")[1] @@ -307,12 +307,12 @@ func fetchDependabotAlerts(repoFullName string) ([]Finding, error) { return nil, nil } } - return nil, errors.E("monitor.fetchDependabot", "API request failed", err) + return nil, log.E("monitor.fetchDependabot", "API request failed", err) } var alerts []DependabotAlert if err := json.Unmarshal(output, &alerts); err != nil { - return nil, errors.E("monitor.fetchDependabot", "failed to parse response", err) + return nil, log.E("monitor.fetchDependabot", "failed to parse response", err) } repoName := strings.Split(repoFullName, "/")[1] @@ -358,12 +358,12 @@ func fetchSecretScanningAlerts(repoFullName string) ([]Finding, error) { return nil, nil } } - return nil, errors.E("monitor.fetchSecretScanning", "API request failed", err) + return nil, log.E("monitor.fetchSecretScanning", "API request failed", err) } var alerts []SecretScanningAlert if err := json.Unmarshal(output, &alerts); err != nil { - return nil, errors.E("monitor.fetchSecretScanning", "failed to parse response", err) + return nil, log.E("monitor.fetchSecretScanning", "failed to parse response", err) } repoName := strings.Split(repoFullName, "/")[1] @@ -447,7 +447,7 @@ func sortBySeverity(findings []Finding) { func outputJSON(findings []Finding) error { data, err := json.MarshalIndent(findings, "", " ") if err != nil { - return errors.E("monitor", "failed to marshal findings", err) + return log.E("monitor", "failed to marshal findings", err) } cli.Print("%s\n", string(data)) return nil @@ -547,7 +547,7 @@ func detectRepoFromGit() (string, error) { cmd := exec.Command("git", "remote", "get-url", "origin") output, err := cmd.Output() if err != nil { - return "", errors.E("monitor", i18n.T("cmd.monitor.error.not_git_repo"), err) + return "", log.E("monitor", i18n.T("cmd.monitor.error.not_git_repo"), err) } url := strings.TrimSpace(string(output)) diff --git a/internal/cmd/pkgcmd/cmd_install.go b/internal/cmd/pkgcmd/cmd_install.go index d3d0bf55..b5052325 100644 --- a/internal/cmd/pkgcmd/cmd_install.go +++ b/internal/cmd/pkgcmd/cmd_install.go @@ -74,7 +74,7 @@ func runPkgInstall(repoArg, targetDir string, addToRegistry bool) error { repoPath := filepath.Join(targetDir, repoName) - if _, err := coreio.Local.List(filepath.Join(repoPath, ".git")); err == nil { + if coreio.Local.Exists(filepath.Join(repoPath, ".git")) { fmt.Printf("%s %s\n", dimStyle.Render(i18n.Label("skip")), i18n.T("cmd.pkg.install.already_exists", map[string]string{"Name": repoName, "Path": repoPath})) return nil } diff --git a/internal/cmd/pkgcmd/cmd_manage.go b/internal/cmd/pkgcmd/cmd_manage.go index cabba86f..cabcbde1 100644 --- a/internal/cmd/pkgcmd/cmd_manage.go +++ b/internal/cmd/pkgcmd/cmd_manage.go @@ -57,9 +57,8 @@ func runPkgList() error { var installed, missing int for _, r := range allRepos { repoPath := filepath.Join(basePath, r.Name) - exists := false - if _, err := coreio.Local.List(filepath.Join(repoPath, ".git")); err == nil { - exists = true + exists := coreio.Local.Exists(filepath.Join(repoPath, ".git")) + if exists { installed++ } else { missing++ @@ -219,7 +218,7 @@ func runPkgOutdated() error { for _, r := range reg.List() { repoPath := filepath.Join(basePath, r.Name) - if _, err := coreio.Local.List(filepath.Join(repoPath, ".git")); err != nil { + if !coreio.Local.Exists(filepath.Join(repoPath, ".git")) { notInstalled++ continue } diff --git a/internal/cmd/qa/cmd_health.go b/internal/cmd/qa/cmd_health.go index 95dca54c..f34cee3d 100644 --- a/internal/cmd/qa/cmd_health.go +++ b/internal/cmd/qa/cmd_health.go @@ -13,8 +13,8 @@ import ( "strings" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -63,7 +63,7 @@ func addHealthCommand(parent *cli.Command) { func runHealth() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.health", i18n.T("error.gh_not_found"), nil) + return log.E("qa.health", i18n.T("error.gh_not_found"), nil) } // Load registry @@ -75,12 +75,12 @@ func runHealth() error { } else { registryPath, findErr := repos.FindRegistry() if findErr != nil { - return errors.E("qa.health", i18n.T("error.registry_not_found"), nil) + return log.E("qa.health", i18n.T("error.registry_not_found"), nil) } reg, err = repos.LoadRegistry(registryPath) } if err != nil { - return errors.E("qa.health", "failed to load registry", err) + return log.E("qa.health", "failed to load registry", err) } // Fetch CI status from all repos diff --git a/internal/cmd/qa/cmd_issues.go b/internal/cmd/qa/cmd_issues.go index d243fc03..2b038c6d 100644 --- a/internal/cmd/qa/cmd_issues.go +++ b/internal/cmd/qa/cmd_issues.go @@ -16,8 +16,8 @@ import ( "time" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/repos" ) @@ -92,7 +92,7 @@ func addIssuesCommand(parent *cli.Command) { func runQAIssues() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.issues", i18n.T("error.gh_not_found"), nil) + return log.E("qa.issues", i18n.T("error.gh_not_found"), nil) } // Load registry @@ -104,12 +104,12 @@ func runQAIssues() error { } else { registryPath, findErr := repos.FindRegistry() if findErr != nil { - return errors.E("qa.issues", i18n.T("error.registry_not_found"), nil) + return log.E("qa.issues", i18n.T("error.registry_not_found"), nil) } reg, err = repos.LoadRegistry(registryPath) } if err != nil { - return errors.E("qa.issues", "failed to load registry", err) + return log.E("qa.issues", "failed to load registry", err) } // Fetch issues from all repos diff --git a/internal/cmd/qa/cmd_review.go b/internal/cmd/qa/cmd_review.go index 30945856..7bae5e47 100644 --- a/internal/cmd/qa/cmd_review.go +++ b/internal/cmd/qa/cmd_review.go @@ -16,8 +16,8 @@ import ( "time" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" ) // Review command flags @@ -102,7 +102,7 @@ func addReviewCommand(parent *cli.Command) { func runReview() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.review", i18n.T("error.gh_not_found"), nil) + return log.E("qa.review", i18n.T("error.gh_not_found"), nil) } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -114,7 +114,7 @@ func runReview() error { var err error repoFullName, err = detectRepoFromGit() if err != nil { - return errors.E("qa.review", i18n.T("cmd.qa.review.error.no_repo"), nil) + return log.E("qa.review", i18n.T("cmd.qa.review.error.no_repo"), nil) } } @@ -144,7 +144,7 @@ func runReview() error { func showMyPRs(ctx context.Context, repo string) error { prs, err := fetchPRs(ctx, repo, "author:@me") if err != nil { - return errors.E("qa.review", "failed to fetch your PRs", err) + return log.E("qa.review", "failed to fetch your PRs", err) } if len(prs) == 0 { @@ -165,7 +165,7 @@ func showMyPRs(ctx context.Context, repo string) error { func showRequestedReviews(ctx context.Context, repo string) error { prs, err := fetchPRs(ctx, repo, "review-requested:@me") if err != nil { - return errors.E("qa.review", "failed to fetch review requests", err) + return log.E("qa.review", "failed to fetch review requests", err) } if len(prs) == 0 { diff --git a/internal/cmd/qa/cmd_watch.go b/internal/cmd/qa/cmd_watch.go index 2db17fe3..38ec20d7 100644 --- a/internal/cmd/qa/cmd_watch.go +++ b/internal/cmd/qa/cmd_watch.go @@ -17,8 +17,8 @@ import ( "time" "github.com/host-uk/core/pkg/cli" - "github.com/host-uk/core/pkg/errors" "github.com/host-uk/core/pkg/i18n" + "github.com/host-uk/core/pkg/log" ) // Watch command flags @@ -79,7 +79,7 @@ func addWatchCommand(parent *cli.Command) { func runWatch() error { // Check gh is available if _, err := exec.LookPath("gh"); err != nil { - return errors.E("qa.watch", i18n.T("error.gh_not_found"), nil) + return log.E("qa.watch", i18n.T("error.gh_not_found"), nil) } // Determine repo @@ -115,12 +115,12 @@ func runWatch() error { // Check if context deadline exceeded if ctx.Err() != nil { cli.Blank() - return errors.E("qa.watch", i18n.T("cmd.qa.watch.timeout", map[string]interface{}{"Duration": watchTimeout}), nil) + return log.E("qa.watch", i18n.T("cmd.qa.watch.timeout", map[string]interface{}{"Duration": watchTimeout}), nil) } runs, err := fetchWorkflowRunsForCommit(ctx, repoFullName, commitSha) if err != nil { - return errors.Wrap(err, "qa.watch", "failed to fetch workflow runs") + return log.Wrap(err, "qa.watch", "failed to fetch workflow runs") } if len(runs) == 0 { @@ -195,7 +195,7 @@ func resolveRepo(specified string) (string, error) { if org != "" { return org + "/" + specified, nil } - return "", errors.E("qa.watch", i18n.T("cmd.qa.watch.error.repo_format"), nil) + return "", log.E("qa.watch", i18n.T("cmd.qa.watch.error.repo_format"), nil) } // Detect from current directory @@ -212,7 +212,7 @@ func resolveCommit(specified string) (string, error) { cmd := exec.Command("git", "rev-parse", "HEAD") output, err := cmd.Output() if err != nil { - return "", errors.Wrap(err, "qa.watch", "failed to get HEAD commit") + return "", log.Wrap(err, "qa.watch", "failed to get HEAD commit") } return strings.TrimSpace(string(output)), nil @@ -223,7 +223,7 @@ func detectRepoFromGit() (string, error) { cmd := exec.Command("git", "remote", "get-url", "origin") output, err := cmd.Output() if err != nil { - return "", errors.E("qa.watch", i18n.T("cmd.qa.watch.error.not_git_repo"), nil) + return "", log.E("qa.watch", i18n.T("cmd.qa.watch.error.not_git_repo"), nil) } url := strings.TrimSpace(string(output)) diff --git a/internal/cmd/setup/cmd_bootstrap.go b/internal/cmd/setup/cmd_bootstrap.go index a7d354ec..4ea2839c 100644 --- a/internal/cmd/setup/cmd_bootstrap.go +++ b/internal/cmd/setup/cmd_bootstrap.go @@ -105,7 +105,7 @@ func runBootstrap(ctx context.Context, only string, dryRun, all bool, projectNam // Clone core-devops first devopsPath := filepath.Join(targetDir, devopsRepo) - if _, err := coreio.Local.List(filepath.Join(devopsPath, ".git")); err != nil { + if !coreio.Local.Exists(filepath.Join(devopsPath, ".git")) { fmt.Printf("%s %s %s...\n", dimStyle.Render(">>"), i18n.T("common.status.cloning"), devopsRepo) if !dryRun { @@ -148,9 +148,9 @@ func runBootstrap(ctx context.Context, only string, dryRun, all bool, projectNam } // isGitRepoRoot returns true if the directory is a git repository root. +// Handles both regular repos (.git is a directory) and worktrees (.git is a file). func isGitRepoRoot(path string) bool { - _, err := coreio.Local.List(filepath.Join(path, ".git")) - return err == nil + return coreio.Local.Exists(filepath.Join(path, ".git")) } // isDirEmpty returns true if the directory is empty or contains only hidden files. diff --git a/internal/cmd/setup/cmd_registry.go b/internal/cmd/setup/cmd_registry.go index 896a4e63..a06e10ef 100644 --- a/internal/cmd/setup/cmd_registry.go +++ b/internal/cmd/setup/cmd_registry.go @@ -117,8 +117,8 @@ func runRegistrySetupWithReg(ctx context.Context, reg *repos.Registry, registryP // Check if already exists repoPath := filepath.Join(basePath, repo.Name) - // Check .git dir existence via List - if _, err := coreio.Local.List(filepath.Join(repoPath, ".git")); err == nil { + // Check .git dir existence via Exists + if coreio.Local.Exists(filepath.Join(repoPath, ".git")) { exists++ continue } @@ -147,7 +147,7 @@ func runRegistrySetupWithReg(ctx context.Context, reg *repos.Registry, registryP // Check if already exists repoPath := filepath.Join(basePath, repo.Name) - if _, err := coreio.Local.List(filepath.Join(repoPath, ".git")); err == nil { + if coreio.Local.Exists(filepath.Join(repoPath, ".git")) { exists++ continue } diff --git a/pkg/agentic/client.go b/pkg/agentic/client.go index 72675a1a..fe77f937 100644 --- a/pkg/agentic/client.go +++ b/pkg/agentic/client.go @@ -12,7 +12,7 @@ import ( "strings" "time" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" ) // Client is the API client for the core-agentic service. @@ -77,24 +77,24 @@ func (c *Client) ListTasks(ctx context.Context, opts ListOptions) ([]Task, error req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return nil, errors.E(op, "failed to create request", err) + return nil, log.E(op, "failed to create request", err) } c.setHeaders(req) resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, errors.E(op, "request failed", err) + return nil, log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return nil, errors.E(op, "API error", err) + return nil, log.E(op, "API error", err) } var tasks []Task if err := json.NewDecoder(resp.Body).Decode(&tasks); err != nil { - return nil, errors.E(op, "failed to decode response", err) + return nil, log.E(op, "failed to decode response", err) } return tasks, nil @@ -105,31 +105,31 @@ func (c *Client) GetTask(ctx context.Context, id string) (*Task, error) { const op = "agentic.Client.GetTask" if id == "" { - return nil, errors.E(op, "task ID is required", nil) + return nil, log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s", c.BaseURL, url.PathEscape(id)) req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return nil, errors.E(op, "failed to create request", err) + return nil, log.E(op, "failed to create request", err) } c.setHeaders(req) resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, errors.E(op, "request failed", err) + return nil, log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return nil, errors.E(op, "API error", err) + return nil, log.E(op, "API error", err) } var task Task if err := json.NewDecoder(resp.Body).Decode(&task); err != nil { - return nil, errors.E(op, "failed to decode response", err) + return nil, log.E(op, "failed to decode response", err) } return &task, nil @@ -140,7 +140,7 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { const op = "agentic.Client.ClaimTask" if id == "" { - return nil, errors.E(op, "task ID is required", nil) + return nil, log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s/claim", c.BaseURL, url.PathEscape(id)) @@ -154,7 +154,7 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, body) if err != nil { - return nil, errors.E(op, "failed to create request", err) + return nil, log.E(op, "failed to create request", err) } c.setHeaders(req) @@ -164,18 +164,18 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, errors.E(op, "request failed", err) + return nil, log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return nil, errors.E(op, "API error", err) + return nil, log.E(op, "API error", err) } // Read body once to allow multiple decode attempts bodyData, err := io.ReadAll(resp.Body) if err != nil { - return nil, errors.E(op, "failed to read response", err) + return nil, log.E(op, "failed to read response", err) } // Try decoding as ClaimResponse first @@ -187,7 +187,7 @@ func (c *Client) ClaimTask(ctx context.Context, id string) (*Task, error) { // Try decoding as just a Task for simpler API responses var task Task if err := json.Unmarshal(bodyData, &task); err != nil { - return nil, errors.E(op, "failed to decode response", err) + return nil, log.E(op, "failed to decode response", err) } return &task, nil @@ -198,19 +198,19 @@ func (c *Client) UpdateTask(ctx context.Context, id string, update TaskUpdate) e const op = "agentic.Client.UpdateTask" if id == "" { - return errors.E(op, "task ID is required", nil) + return log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s", c.BaseURL, url.PathEscape(id)) data, err := json.Marshal(update) if err != nil { - return errors.E(op, "failed to marshal update", err) + return log.E(op, "failed to marshal update", err) } req, err := http.NewRequestWithContext(ctx, http.MethodPatch, endpoint, bytes.NewReader(data)) if err != nil { - return errors.E(op, "failed to create request", err) + return log.E(op, "failed to create request", err) } c.setHeaders(req) @@ -218,12 +218,12 @@ func (c *Client) UpdateTask(ctx context.Context, id string, update TaskUpdate) e resp, err := c.HTTPClient.Do(req) if err != nil { - return errors.E(op, "request failed", err) + return log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return errors.E(op, "API error", err) + return log.E(op, "API error", err) } return nil @@ -234,19 +234,19 @@ func (c *Client) CompleteTask(ctx context.Context, id string, result TaskResult) const op = "agentic.Client.CompleteTask" if id == "" { - return errors.E(op, "task ID is required", nil) + return log.E(op, "task ID is required", nil) } endpoint := fmt.Sprintf("%s/api/tasks/%s/complete", c.BaseURL, url.PathEscape(id)) data, err := json.Marshal(result) if err != nil { - return errors.E(op, "failed to marshal result", err) + return log.E(op, "failed to marshal result", err) } req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(data)) if err != nil { - return errors.E(op, "failed to create request", err) + return log.E(op, "failed to create request", err) } c.setHeaders(req) @@ -254,12 +254,12 @@ func (c *Client) CompleteTask(ctx context.Context, id string, result TaskResult) resp, err := c.HTTPClient.Do(req) if err != nil { - return errors.E(op, "request failed", err) + return log.E(op, "request failed", err) } defer resp.Body.Close() if err := c.checkResponse(resp); err != nil { - return errors.E(op, "API error", err) + return log.E(op, "API error", err) } return nil @@ -303,19 +303,19 @@ func (c *Client) Ping(ctx context.Context) error { req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return errors.E(op, "failed to create request", err) + return log.E(op, "failed to create request", err) } c.setHeaders(req) resp, err := c.HTTPClient.Do(req) if err != nil { - return errors.E(op, "request failed", err) + return log.E(op, "request failed", err) } defer resp.Body.Close() if resp.StatusCode >= 400 { - return errors.E(op, fmt.Sprintf("server returned status %d", resp.StatusCode), nil) + return log.E(op, fmt.Sprintf("server returned status %d", resp.StatusCode), nil) } return nil diff --git a/pkg/agentic/completion.go b/pkg/agentic/completion.go index 3107c876..4a5b58f8 100644 --- a/pkg/agentic/completion.go +++ b/pkg/agentic/completion.go @@ -8,7 +8,7 @@ import ( "os/exec" "strings" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" ) // PROptions contains options for creating a pull request. @@ -36,11 +36,11 @@ func AutoCommit(ctx context.Context, task *Task, dir string, message string) err const op = "agentic.AutoCommit" if task == nil { - return errors.E(op, "task is required", nil) + return log.E(op, "task is required", nil) } if message == "" { - return errors.E(op, "commit message is required", nil) + return log.E(op, "commit message is required", nil) } // Build full commit message @@ -48,12 +48,12 @@ func AutoCommit(ctx context.Context, task *Task, dir string, message string) err // Stage all changes if _, err := runGitCommandCtx(ctx, dir, "add", "-A"); err != nil { - return errors.E(op, "failed to stage changes", err) + return log.E(op, "failed to stage changes", err) } // Create commit if _, err := runGitCommandCtx(ctx, dir, "commit", "-m", fullMessage); err != nil { - return errors.E(op, "failed to create commit", err) + return log.E(op, "failed to create commit", err) } return nil @@ -83,7 +83,7 @@ func CreatePR(ctx context.Context, task *Task, dir string, opts PROptions) (stri const op = "agentic.CreatePR" if task == nil { - return "", errors.E(op, "task is required", nil) + return "", log.E(op, "task is required", nil) } // Build title if not provided @@ -116,7 +116,7 @@ func CreatePR(ctx context.Context, task *Task, dir string, opts PROptions) (stri // Run gh pr create output, err := runCommandCtx(ctx, dir, "gh", args...) if err != nil { - return "", errors.E(op, "failed to create PR", err) + return "", log.E(op, "failed to create PR", err) } // Extract PR URL from output @@ -158,11 +158,11 @@ func SyncStatus(ctx context.Context, client *Client, task *Task, update TaskUpda const op = "agentic.SyncStatus" if client == nil { - return errors.E(op, "client is required", nil) + return log.E(op, "client is required", nil) } if task == nil { - return errors.E(op, "task is required", nil) + return log.E(op, "task is required", nil) } return client.UpdateTask(ctx, task.ID, update) @@ -174,7 +174,7 @@ func CommitAndSync(ctx context.Context, client *Client, task *Task, dir string, // Create commit if err := AutoCommit(ctx, task, dir, message); err != nil { - return errors.E(op, "failed to commit", err) + return log.E(op, "failed to commit", err) } // Sync status if client provided @@ -187,7 +187,7 @@ func CommitAndSync(ctx context.Context, client *Client, task *Task, dir string, if err := SyncStatus(ctx, client, task, update); err != nil { // Log but don't fail on sync errors - return errors.E(op, "commit succeeded but sync failed", err) + return log.E(op, "commit succeeded but sync failed", err) } } @@ -200,7 +200,7 @@ func PushChanges(ctx context.Context, dir string) error { _, err := runGitCommandCtx(ctx, dir, "push") if err != nil { - return errors.E(op, "failed to push changes", err) + return log.E(op, "failed to push changes", err) } return nil @@ -211,7 +211,7 @@ func CreateBranch(ctx context.Context, task *Task, dir string) (string, error) { const op = "agentic.CreateBranch" if task == nil { - return "", errors.E(op, "task is required", nil) + return "", log.E(op, "task is required", nil) } // Generate branch name from task @@ -220,7 +220,7 @@ func CreateBranch(ctx context.Context, task *Task, dir string) (string, error) { // Create and checkout branch _, err := runGitCommandCtx(ctx, dir, "checkout", "-b", branchName) if err != nil { - return "", errors.E(op, "failed to create branch", err) + return "", log.E(op, "failed to create branch", err) } return branchName, nil @@ -302,7 +302,7 @@ func GetCurrentBranch(ctx context.Context, dir string) (string, error) { output, err := runGitCommandCtx(ctx, dir, "rev-parse", "--abbrev-ref", "HEAD") if err != nil { - return "", errors.E(op, "failed to get current branch", err) + return "", log.E(op, "failed to get current branch", err) } return strings.TrimSpace(output), nil @@ -314,7 +314,7 @@ func HasUncommittedChanges(ctx context.Context, dir string) (bool, error) { output, err := runGitCommandCtx(ctx, dir, "status", "--porcelain") if err != nil { - return false, errors.E(op, "failed to get git status", err) + return false, log.E(op, "failed to get git status", err) } return strings.TrimSpace(output) != "", nil @@ -331,7 +331,7 @@ func GetDiff(ctx context.Context, dir string, staged bool) (string, error) { output, err := runGitCommandCtx(ctx, dir, args...) if err != nil { - return "", errors.E(op, "failed to get diff", err) + return "", log.E(op, "failed to get diff", err) } return output, nil diff --git a/pkg/agentic/config.go b/pkg/agentic/config.go index 3ad088ad..c713de5f 100644 --- a/pkg/agentic/config.go +++ b/pkg/agentic/config.go @@ -6,7 +6,7 @@ import ( "path/filepath" "strings" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" "gopkg.in/yaml.v3" ) @@ -74,12 +74,12 @@ func LoadConfig(dir string) (*Config, error) { // Try loading from ~/.core/agentic.yaml homeDir, err := os.UserHomeDir() if err != nil { - return nil, errors.E("agentic.LoadConfig", "failed to get home directory", err) + return nil, log.E("agentic.LoadConfig", "failed to get home directory", err) } configPath := filepath.Join(homeDir, ".core", configFileName) if err := loadYAMLConfig(configPath, cfg); err != nil && !os.IsNotExist(err) { - return nil, errors.E("agentic.LoadConfig", "failed to load config", err) + return nil, log.E("agentic.LoadConfig", "failed to load config", err) } // Apply environment variable overrides @@ -87,7 +87,7 @@ func LoadConfig(dir string) (*Config, error) { // Validate configuration if cfg.Token == "" { - return nil, errors.E("agentic.LoadConfig", "no authentication token configured", nil) + return nil, log.E("agentic.LoadConfig", "no authentication token configured", nil) } return cfg, nil @@ -167,23 +167,23 @@ func applyEnvOverrides(cfg *Config) { func SaveConfig(cfg *Config) error { homeDir, err := os.UserHomeDir() if err != nil { - return errors.E("agentic.SaveConfig", "failed to get home directory", err) + return log.E("agentic.SaveConfig", "failed to get home directory", err) } configDir := filepath.Join(homeDir, ".core") if err := os.MkdirAll(configDir, 0755); err != nil { - return errors.E("agentic.SaveConfig", "failed to create config directory", err) + return log.E("agentic.SaveConfig", "failed to create config directory", err) } configPath := filepath.Join(configDir, configFileName) data, err := yaml.Marshal(cfg) if err != nil { - return errors.E("agentic.SaveConfig", "failed to marshal config", err) + return log.E("agentic.SaveConfig", "failed to marshal config", err) } if err := os.WriteFile(configPath, data, 0600); err != nil { - return errors.E("agentic.SaveConfig", "failed to write config file", err) + return log.E("agentic.SaveConfig", "failed to write config file", err) } return nil @@ -193,7 +193,7 @@ func SaveConfig(cfg *Config) error { func ConfigPath() (string, error) { homeDir, err := os.UserHomeDir() if err != nil { - return "", errors.E("agentic.ConfigPath", "failed to get home directory", err) + return "", log.E("agentic.ConfigPath", "failed to get home directory", err) } return filepath.Join(homeDir, ".core", configFileName), nil } diff --git a/pkg/agentic/context.go b/pkg/agentic/context.go index a31ba632..00951322 100644 --- a/pkg/agentic/context.go +++ b/pkg/agentic/context.go @@ -9,7 +9,7 @@ import ( "regexp" "strings" - "github.com/host-uk/core/pkg/errors" + "github.com/host-uk/core/pkg/log" ) // FileContent represents the content of a file for AI context. @@ -41,13 +41,13 @@ func BuildTaskContext(task *Task, dir string) (*TaskContext, error) { const op = "agentic.BuildTaskContext" if task == nil { - return nil, errors.E(op, "task is required", nil) + return nil, log.E(op, "task is required", nil) } if dir == "" { cwd, err := os.Getwd() if err != nil { - return nil, errors.E(op, "failed to get working directory", err) + return nil, log.E(op, "failed to get working directory", err) } dir = cwd } @@ -87,7 +87,7 @@ func GatherRelatedFiles(task *Task, dir string) ([]FileContent, error) { const op = "agentic.GatherRelatedFiles" if task == nil { - return nil, errors.E(op, "task is required", nil) + return nil, log.E(op, "task is required", nil) } var files []FileContent @@ -117,7 +117,7 @@ func findRelatedCode(task *Task, dir string) ([]FileContent, error) { const op = "agentic.findRelatedCode" if task == nil { - return nil, errors.E(op, "task is required", nil) + return nil, log.E(op, "task is required", nil) } // Extract keywords from title and description diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 19741d13..36295762 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -1,151 +1,128 @@ // Package errors provides structured error handling for Core applications. // -// Errors include operational context (what was being done) and support -// error wrapping for debugging while keeping user-facing messages clean: +// Deprecated: Use pkg/log instead. This package is maintained for backward +// compatibility and will be removed in a future version. All error handling +// functions are now available in pkg/log: // -// err := errors.E("user.Create", "email already exists", nil) -// err := errors.Wrap(dbErr, "user.Create", "failed to save user") +// // Instead of: +// import "github.com/host-uk/core/pkg/errors" +// err := errors.E("op", "msg", cause) // -// // Check error types -// if errors.Is(err, sql.ErrNoRows) { ... } +// // Use: +// import "github.com/host-uk/core/pkg/log" +// err := log.E("op", "msg", cause) // -// // Extract operation -// var e *errors.Error -// if errors.As(err, &e) { -// fmt.Println("Operation:", e.Op) -// } +// Migration guide: +// - errors.Error -> log.Err +// - errors.E -> log.E +// - errors.Wrap -> log.Wrap +// - errors.WrapCode -> log.WrapCode +// - errors.Code -> log.NewCode +// - errors.New -> log.NewError +// - errors.Is -> log.Is +// - errors.As -> log.As +// - errors.Join -> log.Join +// - errors.Op -> log.Op +// - errors.ErrCode -> log.ErrCode +// - errors.Message -> log.Message +// - errors.Root -> log.Root package errors import ( - stderrors "errors" - "fmt" + "github.com/host-uk/core/pkg/log" ) // Error represents a structured error with operational context. -type Error struct { - Op string // Operation being performed (e.g., "user.Create") - Msg string // Human-readable message - Err error // Underlying error (optional) - Code string // Error code for i18n/categorisation (optional) -} +// +// Deprecated: Use log.Err instead. +type Error = log.Err // E creates a new Error with operation context. // -// err := errors.E("config.Load", "file not found", os.ErrNotExist) -// err := errors.E("api.Call", "rate limited", nil) +// Deprecated: Use log.E instead. func E(op, msg string, err error) error { - return &Error{Op: op, Msg: msg, Err: err} + return log.E(op, msg, err) } // Wrap wraps an error with operation context. // Returns nil if err is nil. // -// return errors.Wrap(err, "db.Query", "failed to fetch user") +// Deprecated: Use log.Wrap instead. func Wrap(err error, op, msg string) error { - if err == nil { - return nil - } - return &Error{Op: op, Msg: msg, Err: err} + return log.Wrap(err, op, msg) } // WrapCode wraps an error with operation context and an error code. // -// return errors.WrapCode(err, "ERR_NOT_FOUND", "user.Get", "user not found") +// Deprecated: Use log.WrapCode instead. func WrapCode(err error, code, op, msg string) error { - if err == nil && code == "" { - return nil - } - return &Error{Op: op, Msg: msg, Err: err, Code: code} + return log.WrapCode(err, code, op, msg) } // Code creates an error with just a code and message. // -// return errors.Code("ERR_VALIDATION", "invalid email format") +// Deprecated: Use log.NewCode instead. func Code(code, msg string) error { - return &Error{Code: code, Msg: msg} -} - -// Error returns the error message. -func (e *Error) Error() string { - if e.Op != "" && e.Err != nil { - return fmt.Sprintf("%s: %s: %v", e.Op, e.Msg, e.Err) - } - if e.Op != "" { - return fmt.Sprintf("%s: %s", e.Op, e.Msg) - } - if e.Err != nil { - return fmt.Sprintf("%s: %v", e.Msg, e.Err) - } - return e.Msg -} - -// Unwrap returns the underlying error. -func (e *Error) Unwrap() error { - return e.Err + return log.NewCode(code, msg) } // --- Standard library wrappers --- // Is reports whether any error in err's tree matches target. +// +// Deprecated: Use log.Is instead. func Is(err, target error) bool { - return stderrors.Is(err, target) + return log.Is(err, target) } // As finds the first error in err's tree that matches target. +// +// Deprecated: Use log.As instead. func As(err error, target any) bool { - return stderrors.As(err, target) + return log.As(err, target) } // New returns an error with the given text. +// +// Deprecated: Use log.NewError instead. func New(text string) error { - return stderrors.New(text) + return log.NewError(text) } // Join returns an error that wraps the given errors. +// +// Deprecated: Use log.Join instead. func Join(errs ...error) error { - return stderrors.Join(errs...) + return log.Join(errs...) } // --- Helper functions --- // Op extracts the operation from an error, or empty string if not an Error. +// +// Deprecated: Use log.Op instead. func Op(err error) string { - var e *Error - if As(err, &e) { - return e.Op - } - return "" + return log.Op(err) } // ErrCode extracts the error code, or empty string if not set. +// +// Deprecated: Use log.ErrCode instead. func ErrCode(err error) string { - var e *Error - if As(err, &e) { - return e.Code - } - return "" + return log.ErrCode(err) } // Message extracts the message from an error. // For Error types, returns Msg; otherwise returns err.Error(). +// +// Deprecated: Use log.Message instead. func Message(err error) string { - if err == nil { - return "" - } - var e *Error - if As(err, &e) { - return e.Msg - } - return err.Error() + return log.Message(err) } // Root returns the deepest error in the chain. +// +// Deprecated: Use log.Root instead. func Root(err error) error { - for { - unwrapped := stderrors.Unwrap(err) - if unwrapped == nil { - return err - } - err = unwrapped - } + return log.Root(err) } diff --git a/pkg/io/client_test.go b/pkg/io/client_test.go index 1579460b..9d76d518 100644 --- a/pkg/io/client_test.go +++ b/pkg/io/client_test.go @@ -73,6 +73,127 @@ func TestMockMedium_FileSet_Good(t *testing.T) { assert.Equal(t, "content", m.Files["test.txt"]) } +func TestMockMedium_Delete_Good(t *testing.T) { + m := NewMockMedium() + m.Files["test.txt"] = "content" + + err := m.Delete("test.txt") + assert.NoError(t, err) + assert.False(t, m.IsFile("test.txt")) +} + +func TestMockMedium_Delete_Bad_NotFound(t *testing.T) { + m := NewMockMedium() + err := m.Delete("nonexistent.txt") + assert.Error(t, err) +} + +func TestMockMedium_Delete_Bad_DirNotEmpty(t *testing.T) { + m := NewMockMedium() + m.Dirs["mydir"] = true + m.Files["mydir/file.txt"] = "content" + + err := m.Delete("mydir") + assert.Error(t, err) +} + +func TestMockMedium_DeleteAll_Good(t *testing.T) { + m := NewMockMedium() + m.Dirs["mydir"] = true + m.Dirs["mydir/subdir"] = true + m.Files["mydir/file.txt"] = "content" + m.Files["mydir/subdir/nested.txt"] = "nested" + + err := m.DeleteAll("mydir") + assert.NoError(t, err) + assert.Empty(t, m.Dirs) + assert.Empty(t, m.Files) +} + +func TestMockMedium_Rename_Good(t *testing.T) { + m := NewMockMedium() + m.Files["old.txt"] = "content" + + err := m.Rename("old.txt", "new.txt") + assert.NoError(t, err) + assert.False(t, m.IsFile("old.txt")) + assert.True(t, m.IsFile("new.txt")) + assert.Equal(t, "content", m.Files["new.txt"]) +} + +func TestMockMedium_Rename_Good_Dir(t *testing.T) { + m := NewMockMedium() + m.Dirs["olddir"] = true + m.Files["olddir/file.txt"] = "content" + + err := m.Rename("olddir", "newdir") + assert.NoError(t, err) + assert.False(t, m.Dirs["olddir"]) + assert.True(t, m.Dirs["newdir"]) + assert.Equal(t, "content", m.Files["newdir/file.txt"]) +} + +func TestMockMedium_List_Good(t *testing.T) { + m := NewMockMedium() + m.Dirs["mydir"] = true + m.Files["mydir/file1.txt"] = "content1" + m.Files["mydir/file2.txt"] = "content2" + m.Dirs["mydir/subdir"] = true + + entries, err := m.List("mydir") + assert.NoError(t, err) + assert.Len(t, entries, 3) + + names := make(map[string]bool) + for _, e := range entries { + names[e.Name()] = true + } + assert.True(t, names["file1.txt"]) + assert.True(t, names["file2.txt"]) + assert.True(t, names["subdir"]) +} + +func TestMockMedium_Stat_Good(t *testing.T) { + m := NewMockMedium() + m.Files["test.txt"] = "hello world" + + info, err := m.Stat("test.txt") + assert.NoError(t, err) + assert.Equal(t, "test.txt", info.Name()) + assert.Equal(t, int64(11), info.Size()) + assert.False(t, info.IsDir()) +} + +func TestMockMedium_Stat_Good_Dir(t *testing.T) { + m := NewMockMedium() + m.Dirs["mydir"] = true + + info, err := m.Stat("mydir") + assert.NoError(t, err) + assert.Equal(t, "mydir", info.Name()) + assert.True(t, info.IsDir()) +} + +func TestMockMedium_Exists_Good(t *testing.T) { + m := NewMockMedium() + m.Files["file.txt"] = "content" + m.Dirs["mydir"] = true + + assert.True(t, m.Exists("file.txt")) + assert.True(t, m.Exists("mydir")) + assert.False(t, m.Exists("nonexistent")) +} + +func TestMockMedium_IsDir_Good(t *testing.T) { + m := NewMockMedium() + m.Files["file.txt"] = "content" + m.Dirs["mydir"] = true + + assert.False(t, m.IsDir("file.txt")) + assert.True(t, m.IsDir("mydir")) + assert.False(t, m.IsDir("nonexistent")) +} + // --- Wrapper Function Tests --- func TestRead_Good(t *testing.T) { diff --git a/pkg/io/io.go b/pkg/io/io.go index 4f199bd9..1e5020b7 100644 --- a/pkg/io/io.go +++ b/pkg/io/io.go @@ -3,7 +3,9 @@ package io import ( "io/fs" "os" + "path/filepath" "strings" + "time" coreerr "github.com/host-uk/core/pkg/framework/core" "github.com/host-uk/core/pkg/io/local" @@ -34,25 +36,54 @@ type Medium interface { // Delete removes a file or empty directory. Delete(path string) error - // DeleteAll removes a file or directory recursively. + // DeleteAll removes a file or directory and all its contents recursively. DeleteAll(path string) error - // Rename moves or renames a file or directory. + // Rename moves a file or directory from oldPath to newPath. Rename(oldPath, newPath string) error - // List returns directory entries. + // List returns the directory entries for the given path. List(path string) ([]fs.DirEntry, error) - // Stat returns file information. + // Stat returns file information for the given path. Stat(path string) (fs.FileInfo, error) - // Exists returns true if path exists. + // Exists checks if a path exists (file or directory). Exists(path string) bool - // IsDir returns true if path is a directory. + // IsDir checks if a path exists and is a directory. IsDir(path string) bool } +// FileInfo provides a simple implementation of fs.FileInfo for mock testing. +type FileInfo struct { + name string + size int64 + mode fs.FileMode + modTime time.Time + isDir bool +} + +func (fi FileInfo) Name() string { return fi.name } +func (fi FileInfo) Size() int64 { return fi.size } +func (fi FileInfo) Mode() fs.FileMode { return fi.mode } +func (fi FileInfo) ModTime() time.Time { return fi.modTime } +func (fi FileInfo) IsDir() bool { return fi.isDir } +func (fi FileInfo) Sys() any { return nil } + +// DirEntry provides a simple implementation of fs.DirEntry for mock testing. +type DirEntry struct { + name string + isDir bool + mode fs.FileMode + info fs.FileInfo +} + +func (de DirEntry) Name() string { return de.name } +func (de DirEntry) IsDir() bool { return de.isDir } +func (de DirEntry) Type() fs.FileMode { return de.mode.Type() } +func (de DirEntry) Info() (fs.FileInfo, error) { return de.info, nil } + // Local is a pre-initialized medium for the local filesystem. // It uses "/" as root, providing unsandboxed access to the filesystem. // For sandboxed access, use NewSandboxed with a specific root path. @@ -162,69 +193,260 @@ func (m *MockMedium) FileSet(path, content string) error { // Delete removes a file or empty directory from the mock filesystem. func (m *MockMedium) Delete(path string) error { - delete(m.Files, path) - delete(m.Dirs, path) - return nil + if _, ok := m.Files[path]; ok { + delete(m.Files, path) + return nil + } + if _, ok := m.Dirs[path]; ok { + // Check if directory is empty (no files or subdirs with this prefix) + prefix := path + if !strings.HasSuffix(prefix, "/") { + prefix += "/" + } + for f := range m.Files { + if strings.HasPrefix(f, prefix) { + return coreerr.E("io.MockMedium.Delete", "directory not empty: "+path, os.ErrExist) + } + } + for d := range m.Dirs { + if d != path && strings.HasPrefix(d, prefix) { + return coreerr.E("io.MockMedium.Delete", "directory not empty: "+path, os.ErrExist) + } + } + delete(m.Dirs, path) + return nil + } + return coreerr.E("io.MockMedium.Delete", "path not found: "+path, os.ErrNotExist) } -// DeleteAll removes a file or directory recursively from the mock filesystem. +// DeleteAll removes a file or directory and all contents from the mock filesystem. func (m *MockMedium) DeleteAll(path string) error { - delete(m.Files, path) - delete(m.Dirs, path) + found := false + if _, ok := m.Files[path]; ok { + delete(m.Files, path) + found = true + } + if _, ok := m.Dirs[path]; ok { + delete(m.Dirs, path) + found = true + } - prefix := path + "/" - for k := range m.Files { - if strings.HasPrefix(k, prefix) { - delete(m.Files, k) + // Delete all entries under this path + prefix := path + if !strings.HasSuffix(prefix, "/") { + prefix += "/" + } + for f := range m.Files { + if strings.HasPrefix(f, prefix) { + delete(m.Files, f) + found = true } } - for k := range m.Dirs { - if strings.HasPrefix(k, prefix) { - delete(m.Dirs, k) + for d := range m.Dirs { + if strings.HasPrefix(d, prefix) { + delete(m.Dirs, d) + found = true } } + + if !found { + return coreerr.E("io.MockMedium.DeleteAll", "path not found: "+path, os.ErrNotExist) + } return nil } -// Rename moves or renames a file in the mock filesystem. +// Rename moves a file or directory in the mock filesystem. func (m *MockMedium) Rename(oldPath, newPath string) error { if content, ok := m.Files[oldPath]; ok { m.Files[newPath] = content delete(m.Files, oldPath) + return nil } - if m.Dirs[oldPath] { + if _, ok := m.Dirs[oldPath]; ok { + // Move directory and all contents m.Dirs[newPath] = true delete(m.Dirs, oldPath) + + oldPrefix := oldPath + if !strings.HasSuffix(oldPrefix, "/") { + oldPrefix += "/" + } + newPrefix := newPath + if !strings.HasSuffix(newPrefix, "/") { + newPrefix += "/" + } + + // Collect files to move first (don't mutate during iteration) + filesToMove := make(map[string]string) + for f, content := range m.Files { + if strings.HasPrefix(f, oldPrefix) { + newF := newPrefix + strings.TrimPrefix(f, oldPrefix) + filesToMove[f] = newF + _ = content // content will be copied in next loop + } + } + for oldF, newF := range filesToMove { + m.Files[newF] = m.Files[oldF] + delete(m.Files, oldF) + } + + // Collect directories to move first + dirsToMove := make(map[string]string) + for d := range m.Dirs { + if strings.HasPrefix(d, oldPrefix) { + newD := newPrefix + strings.TrimPrefix(d, oldPrefix) + dirsToMove[d] = newD + } + } + for oldD, newD := range dirsToMove { + m.Dirs[newD] = true + delete(m.Dirs, oldD) + } + return nil } - return nil + return coreerr.E("io.MockMedium.Rename", "path not found: "+oldPath, os.ErrNotExist) } -// List returns directory entries from the mock filesystem. +// List returns directory entries for the mock filesystem. func (m *MockMedium) List(path string) ([]fs.DirEntry, error) { - return []fs.DirEntry{}, nil + if _, ok := m.Dirs[path]; !ok { + // Check if it's the root or has children + hasChildren := false + prefix := path + if path != "" && !strings.HasSuffix(prefix, "/") { + prefix += "/" + } + for f := range m.Files { + if strings.HasPrefix(f, prefix) { + hasChildren = true + break + } + } + if !hasChildren { + for d := range m.Dirs { + if strings.HasPrefix(d, prefix) { + hasChildren = true + break + } + } + } + if !hasChildren && path != "" { + return nil, coreerr.E("io.MockMedium.List", "directory not found: "+path, os.ErrNotExist) + } + } + + prefix := path + if path != "" && !strings.HasSuffix(prefix, "/") { + prefix += "/" + } + + seen := make(map[string]bool) + var entries []fs.DirEntry + + // Find immediate children (files) + for f, content := range m.Files { + if !strings.HasPrefix(f, prefix) { + continue + } + rest := strings.TrimPrefix(f, prefix) + if rest == "" || strings.Contains(rest, "/") { + // Skip if it's not an immediate child + if idx := strings.Index(rest, "/"); idx != -1 { + // This is a subdirectory + dirName := rest[:idx] + if !seen[dirName] { + seen[dirName] = true + entries = append(entries, DirEntry{ + name: dirName, + isDir: true, + mode: fs.ModeDir | 0755, + info: FileInfo{ + name: dirName, + isDir: true, + mode: fs.ModeDir | 0755, + }, + }) + } + } + continue + } + if !seen[rest] { + seen[rest] = true + entries = append(entries, DirEntry{ + name: rest, + isDir: false, + mode: 0644, + info: FileInfo{ + name: rest, + size: int64(len(content)), + mode: 0644, + }, + }) + } + } + + // Find immediate subdirectories + for d := range m.Dirs { + if !strings.HasPrefix(d, prefix) { + continue + } + rest := strings.TrimPrefix(d, prefix) + if rest == "" { + continue + } + // Get only immediate child + if idx := strings.Index(rest, "/"); idx != -1 { + rest = rest[:idx] + } + if !seen[rest] { + seen[rest] = true + entries = append(entries, DirEntry{ + name: rest, + isDir: true, + mode: fs.ModeDir | 0755, + info: FileInfo{ + name: rest, + isDir: true, + mode: fs.ModeDir | 0755, + }, + }) + } + } + + return entries, nil } -// Stat returns file information from the mock filesystem. +// Stat returns file information for the mock filesystem. func (m *MockMedium) Stat(path string) (fs.FileInfo, error) { - if _, ok := m.Files[path]; ok { - return nil, nil // Mock returns nil info for simplicity + if content, ok := m.Files[path]; ok { + return FileInfo{ + name: filepath.Base(path), + size: int64(len(content)), + mode: 0644, + }, nil } if _, ok := m.Dirs[path]; ok { - return nil, nil + return FileInfo{ + name: filepath.Base(path), + isDir: true, + mode: fs.ModeDir | 0755, + }, nil } - return nil, os.ErrNotExist + return nil, coreerr.E("io.MockMedium.Stat", "path not found: "+path, os.ErrNotExist) } -// Exists returns true if path exists in the mock filesystem. +// Exists checks if a path exists in the mock filesystem. func (m *MockMedium) Exists(path string) bool { if _, ok := m.Files[path]; ok { return true } - _, ok := m.Dirs[path] - return ok + if _, ok := m.Dirs[path]; ok { + return true + } + return false } -// IsDir returns true if path is a directory in the mock filesystem. +// IsDir checks if a path is a directory in the mock filesystem. func (m *MockMedium) IsDir(path string) bool { _, ok := m.Dirs[path] return ok diff --git a/pkg/io/local/client_test.go b/pkg/io/local/client_test.go index 19e3d9f9..3a197a49 100644 --- a/pkg/io/local/client_test.go +++ b/pkg/io/local/client_test.go @@ -177,3 +177,201 @@ func TestFileGetFileSet(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "value", val) } + +func TestDelete_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_delete_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + // Create and delete a file + err = medium.Write("file.txt", "content") + assert.NoError(t, err) + assert.True(t, medium.IsFile("file.txt")) + + err = medium.Delete("file.txt") + assert.NoError(t, err) + assert.False(t, medium.IsFile("file.txt")) + + // Create and delete an empty directory + err = medium.EnsureDir("emptydir") + assert.NoError(t, err) + err = medium.Delete("emptydir") + assert.NoError(t, err) + assert.False(t, medium.IsDir("emptydir")) +} + +func TestDelete_Bad_NotEmpty(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_delete_notempty_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + // Create a directory with a file + err = medium.Write("mydir/file.txt", "content") + assert.NoError(t, err) + + // Try to delete non-empty directory + err = medium.Delete("mydir") + assert.Error(t, err) +} + +func TestDeleteAll_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_deleteall_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + // Create nested structure + err = medium.Write("mydir/file1.txt", "content1") + assert.NoError(t, err) + err = medium.Write("mydir/subdir/file2.txt", "content2") + assert.NoError(t, err) + + // Delete all + err = medium.DeleteAll("mydir") + assert.NoError(t, err) + assert.False(t, medium.Exists("mydir")) + assert.False(t, medium.Exists("mydir/file1.txt")) + assert.False(t, medium.Exists("mydir/subdir/file2.txt")) +} + +func TestRename_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_rename_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + // Rename a file + err = medium.Write("old.txt", "content") + assert.NoError(t, err) + err = medium.Rename("old.txt", "new.txt") + assert.NoError(t, err) + assert.False(t, medium.IsFile("old.txt")) + assert.True(t, medium.IsFile("new.txt")) + + content, err := medium.Read("new.txt") + assert.NoError(t, err) + assert.Equal(t, "content", content) +} + +func TestRename_Traversal_Sanitized(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_rename_traversal_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + err = medium.Write("file.txt", "content") + assert.NoError(t, err) + + // Traversal attempts are sanitized (.. becomes .), so this renames to "./escaped.txt" + // which is just "escaped.txt" in the root + err = medium.Rename("file.txt", "../escaped.txt") + assert.NoError(t, err) + assert.False(t, medium.Exists("file.txt")) + assert.True(t, medium.Exists("escaped.txt")) +} + +func TestList_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_list_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + // Create some files and directories + err = medium.Write("file1.txt", "content1") + assert.NoError(t, err) + err = medium.Write("file2.txt", "content2") + assert.NoError(t, err) + err = medium.EnsureDir("subdir") + assert.NoError(t, err) + + // List root + entries, err := medium.List(".") + assert.NoError(t, err) + assert.Len(t, entries, 3) + + names := make(map[string]bool) + for _, e := range entries { + names[e.Name()] = true + } + assert.True(t, names["file1.txt"]) + assert.True(t, names["file2.txt"]) + assert.True(t, names["subdir"]) +} + +func TestStat_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_stat_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + // Stat a file + err = medium.Write("file.txt", "hello world") + assert.NoError(t, err) + info, err := medium.Stat("file.txt") + assert.NoError(t, err) + assert.Equal(t, "file.txt", info.Name()) + assert.Equal(t, int64(11), info.Size()) + assert.False(t, info.IsDir()) + + // Stat a directory + err = medium.EnsureDir("mydir") + assert.NoError(t, err) + info, err = medium.Stat("mydir") + assert.NoError(t, err) + assert.Equal(t, "mydir", info.Name()) + assert.True(t, info.IsDir()) +} + +func TestExists_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_exists_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + assert.False(t, medium.Exists("nonexistent")) + + err = medium.Write("file.txt", "content") + assert.NoError(t, err) + assert.True(t, medium.Exists("file.txt")) + + err = medium.EnsureDir("mydir") + assert.NoError(t, err) + assert.True(t, medium.Exists("mydir")) +} + +func TestIsDir_Good(t *testing.T) { + testRoot, err := os.MkdirTemp("", "local_isdir_test") + assert.NoError(t, err) + defer os.RemoveAll(testRoot) + + medium, err := New(testRoot) + assert.NoError(t, err) + + err = medium.Write("file.txt", "content") + assert.NoError(t, err) + assert.False(t, medium.IsDir("file.txt")) + + err = medium.EnsureDir("mydir") + assert.NoError(t, err) + assert.True(t, medium.IsDir("mydir")) + + assert.False(t, medium.IsDir("nonexistent")) +} diff --git a/pkg/log/errors.go b/pkg/log/errors.go new file mode 100644 index 00000000..b5e9c467 --- /dev/null +++ b/pkg/log/errors.go @@ -0,0 +1,219 @@ +// Package log provides structured logging and error handling for Core applications. +// +// This file implements structured error types and combined log-and-return helpers +// that simplify common error handling patterns. + +package log + +import ( + "errors" + "fmt" +) + +// Err represents a structured error with operational context. +// It implements the error interface and supports unwrapping. +type Err struct { + Op string // Operation being performed (e.g., "user.Save") + Msg string // Human-readable message + Err error // Underlying error (optional) + Code string // Error code (optional, e.g., "VALIDATION_FAILED") +} + +// Error implements the error interface. +func (e *Err) Error() string { + if e.Err != nil { + if e.Code != "" { + return fmt.Sprintf("%s: %s [%s]: %v", e.Op, e.Msg, e.Code, e.Err) + } + return fmt.Sprintf("%s: %s: %v", e.Op, e.Msg, e.Err) + } + if e.Code != "" { + return fmt.Sprintf("%s: %s [%s]", e.Op, e.Msg, e.Code) + } + return fmt.Sprintf("%s: %s", e.Op, e.Msg) +} + +// Unwrap returns the underlying error for use with errors.Is and errors.As. +func (e *Err) Unwrap() error { + return e.Err +} + +// --- Error Creation Functions --- + +// E creates a new Err with operation context. +// The underlying error can be nil for creating errors without a cause. +// +// Example: +// +// return log.E("user.Save", "failed to save user", err) +// return log.E("api.Call", "rate limited", nil) // No underlying cause +func E(op, msg string, err error) error { + return &Err{Op: op, Msg: msg, Err: err} +} + +// Wrap wraps an error with operation context. +// Returns nil if err is nil, to support conditional wrapping. +// +// Example: +// +// return log.Wrap(err, "db.Query", "database query failed") +func Wrap(err error, op, msg string) error { + if err == nil { + return nil + } + return E(op, msg, err) +} + +// WrapCode wraps an error with operation context and error code. +// Returns nil only if both err is nil AND code is empty. +// Useful for API errors that need machine-readable codes. +// +// Example: +// +// return log.WrapCode(err, "VALIDATION_ERROR", "user.Validate", "invalid email") +func WrapCode(err error, code, op, msg string) error { + if err == nil && code == "" { + return nil + } + return &Err{Op: op, Msg: msg, Err: err, Code: code} +} + +// NewCode creates an error with just code and message (no underlying error). +// Useful for creating sentinel errors with codes. +// +// Example: +// +// var ErrNotFound = log.NewCode("NOT_FOUND", "resource not found") +func NewCode(code, msg string) error { + return &Err{Msg: msg, Code: code} +} + +// --- Standard Library Wrappers --- + +// Is reports whether any error in err's tree matches target. +// Wrapper around errors.Is for convenience. +func Is(err, target error) bool { + return errors.Is(err, target) +} + +// As finds the first error in err's tree that matches target. +// Wrapper around errors.As for convenience. +func As(err error, target any) bool { + return errors.As(err, target) +} + +// NewError creates a simple error with the given text. +// Wrapper around errors.New for convenience. +func NewError(text string) error { + return errors.New(text) +} + +// Join combines multiple errors into one. +// Wrapper around errors.Join for convenience. +func Join(errs ...error) error { + return errors.Join(errs...) +} + +// --- Error Introspection Helpers --- + +// Op extracts the operation name from an error. +// Returns empty string if the error is not an *Err. +func Op(err error) string { + var e *Err + if As(err, &e) { + return e.Op + } + return "" +} + +// ErrCode extracts the error code from an error. +// Returns empty string if the error is not an *Err or has no code. +func ErrCode(err error) string { + var e *Err + if As(err, &e) { + return e.Code + } + return "" +} + +// Message extracts the message from an error. +// Returns the error's Error() string if not an *Err. +func Message(err error) string { + if err == nil { + return "" + } + var e *Err + if As(err, &e) { + return e.Msg + } + return err.Error() +} + +// Root returns the root cause of an error chain. +// Unwraps until no more wrapped errors are found. +func Root(err error) error { + if err == nil { + return nil + } + for { + unwrapped := errors.Unwrap(err) + if unwrapped == nil { + return err + } + err = unwrapped + } +} + +// --- Combined Log-and-Return Helpers --- + +// LogError logs an error at Error level and returns a wrapped error. +// Reduces boilerplate in error handling paths. +// +// Example: +// +// // Before +// if err != nil { +// log.Error("failed to save", "err", err) +// return errors.Wrap(err, "user.Save", "failed to save") +// } +// +// // After +// if err != nil { +// return log.LogError(err, "user.Save", "failed to save") +// } +func LogError(err error, op, msg string) error { + if err == nil { + return nil + } + wrapped := Wrap(err, op, msg) + defaultLogger.Error(msg, "op", op, "err", err) + return wrapped +} + +// LogWarn logs at Warn level and returns a wrapped error. +// Use for recoverable errors that should be logged but not treated as critical. +// +// Example: +// +// return log.LogWarn(err, "cache.Get", "cache miss, falling back to db") +func LogWarn(err error, op, msg string) error { + if err == nil { + return nil + } + wrapped := Wrap(err, op, msg) + defaultLogger.Warn(msg, "op", op, "err", err) + return wrapped +} + +// Must panics if err is not nil, logging first. +// Use for errors that should never happen and indicate programmer error. +// +// Example: +// +// log.Must(Initialize(), "app", "startup failed") +func Must(err error, op, msg string) { + if err != nil { + defaultLogger.Error(msg, "op", op, "err", err) + panic(Wrap(err, op, msg)) + } +} diff --git a/pkg/log/errors_test.go b/pkg/log/errors_test.go new file mode 100644 index 00000000..84390eae --- /dev/null +++ b/pkg/log/errors_test.go @@ -0,0 +1,274 @@ +package log + +import ( + "bytes" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// --- Err Type Tests --- + +func TestErr_Error_Good(t *testing.T) { + // With underlying error + err := &Err{Op: "db.Query", Msg: "failed to query", Err: errors.New("connection refused")} + assert.Equal(t, "db.Query: failed to query: connection refused", err.Error()) + + // With code + err = &Err{Op: "api.Call", Msg: "request failed", Code: "TIMEOUT"} + assert.Equal(t, "api.Call: request failed [TIMEOUT]", err.Error()) + + // With both underlying error and code + err = &Err{Op: "user.Save", Msg: "save failed", Err: errors.New("duplicate key"), Code: "DUPLICATE"} + assert.Equal(t, "user.Save: save failed [DUPLICATE]: duplicate key", err.Error()) + + // Just op and msg + err = &Err{Op: "cache.Get", Msg: "miss"} + assert.Equal(t, "cache.Get: miss", err.Error()) +} + +func TestErr_Unwrap_Good(t *testing.T) { + underlying := errors.New("underlying error") + err := &Err{Op: "test", Msg: "wrapped", Err: underlying} + + assert.Equal(t, underlying, errors.Unwrap(err)) + assert.True(t, errors.Is(err, underlying)) +} + +// --- Error Creation Function Tests --- + +func TestE_Good(t *testing.T) { + underlying := errors.New("base error") + err := E("op.Name", "something failed", underlying) + + assert.NotNil(t, err) + var logErr *Err + assert.True(t, errors.As(err, &logErr)) + assert.Equal(t, "op.Name", logErr.Op) + assert.Equal(t, "something failed", logErr.Msg) + assert.Equal(t, underlying, logErr.Err) +} + +func TestE_Good_NilError(t *testing.T) { + // E creates an error even with nil underlying - useful for errors without causes + err := E("op.Name", "message", nil) + assert.NotNil(t, err) + assert.Equal(t, "op.Name: message", err.Error()) +} + +func TestWrap_Good(t *testing.T) { + underlying := errors.New("base") + err := Wrap(underlying, "handler.Process", "processing failed") + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "handler.Process") + assert.Contains(t, err.Error(), "processing failed") + assert.True(t, errors.Is(err, underlying)) +} + +func TestWrapCode_Good(t *testing.T) { + underlying := errors.New("validation failed") + err := WrapCode(underlying, "INVALID_INPUT", "api.Validate", "bad request") + + assert.NotNil(t, err) + var logErr *Err + assert.True(t, errors.As(err, &logErr)) + assert.Equal(t, "INVALID_INPUT", logErr.Code) + assert.Equal(t, "api.Validate", logErr.Op) + assert.Contains(t, err.Error(), "[INVALID_INPUT]") +} + +func TestWrapCode_Good_NilError(t *testing.T) { + // WrapCode with nil error but with code still creates an error + err := WrapCode(nil, "CODE", "op", "msg") + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "[CODE]") + + // Only returns nil when both error and code are empty + err = WrapCode(nil, "", "op", "msg") + assert.Nil(t, err) +} + +func TestNewCode_Good(t *testing.T) { + err := NewCode("NOT_FOUND", "resource not found") + + var logErr *Err + assert.True(t, errors.As(err, &logErr)) + assert.Equal(t, "NOT_FOUND", logErr.Code) + assert.Equal(t, "resource not found", logErr.Msg) + assert.Nil(t, logErr.Err) +} + +// --- Standard Library Wrapper Tests --- + +func TestIs_Good(t *testing.T) { + sentinel := errors.New("sentinel") + wrapped := Wrap(sentinel, "test", "wrapped") + + assert.True(t, Is(wrapped, sentinel)) + assert.False(t, Is(wrapped, errors.New("other"))) +} + +func TestAs_Good(t *testing.T) { + err := E("test.Op", "message", errors.New("base")) + + var logErr *Err + assert.True(t, As(err, &logErr)) + assert.Equal(t, "test.Op", logErr.Op) +} + +func TestNewError_Good(t *testing.T) { + err := NewError("simple error") + assert.NotNil(t, err) + assert.Equal(t, "simple error", err.Error()) +} + +func TestJoin_Good(t *testing.T) { + err1 := errors.New("error 1") + err2 := errors.New("error 2") + joined := Join(err1, err2) + + assert.True(t, errors.Is(joined, err1)) + assert.True(t, errors.Is(joined, err2)) +} + +// --- Helper Function Tests --- + +func TestOp_Good(t *testing.T) { + err := E("mypackage.MyFunc", "failed", errors.New("cause")) + assert.Equal(t, "mypackage.MyFunc", Op(err)) +} + +func TestOp_Good_NotLogError(t *testing.T) { + err := errors.New("plain error") + assert.Equal(t, "", Op(err)) +} + +func TestErrCode_Good(t *testing.T) { + err := WrapCode(errors.New("base"), "ERR_CODE", "op", "msg") + assert.Equal(t, "ERR_CODE", ErrCode(err)) +} + +func TestErrCode_Good_NoCode(t *testing.T) { + err := E("op", "msg", errors.New("base")) + assert.Equal(t, "", ErrCode(err)) +} + +func TestMessage_Good(t *testing.T) { + err := E("op", "the message", errors.New("base")) + assert.Equal(t, "the message", Message(err)) +} + +func TestMessage_Good_PlainError(t *testing.T) { + err := errors.New("plain message") + assert.Equal(t, "plain message", Message(err)) +} + +func TestMessage_Good_Nil(t *testing.T) { + assert.Equal(t, "", Message(nil)) +} + +func TestRoot_Good(t *testing.T) { + root := errors.New("root cause") + level1 := Wrap(root, "level1", "wrapped once") + level2 := Wrap(level1, "level2", "wrapped twice") + + assert.Equal(t, root, Root(level2)) +} + +func TestRoot_Good_SingleError(t *testing.T) { + err := errors.New("single") + assert.Equal(t, err, Root(err)) +} + +func TestRoot_Good_Nil(t *testing.T) { + assert.Nil(t, Root(nil)) +} + +// --- Log-and-Return Helper Tests --- + +func TestLogError_Good(t *testing.T) { + // Capture log output + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + underlying := errors.New("connection failed") + err := LogError(underlying, "db.Connect", "database unavailable") + + // Check returned error + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "db.Connect") + assert.Contains(t, err.Error(), "database unavailable") + assert.True(t, errors.Is(err, underlying)) + + // Check log output + output := buf.String() + assert.Contains(t, output, "[ERR]") + assert.Contains(t, output, "database unavailable") + assert.Contains(t, output, "op=db.Connect") +} + +func TestLogError_Good_NilError(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + err := LogError(nil, "op", "msg") + assert.Nil(t, err) + assert.Empty(t, buf.String()) // No log output for nil error +} + +func TestLogWarn_Good(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + underlying := errors.New("cache miss") + err := LogWarn(underlying, "cache.Get", "falling back to db") + + assert.NotNil(t, err) + assert.True(t, errors.Is(err, underlying)) + + output := buf.String() + assert.Contains(t, output, "[WRN]") + assert.Contains(t, output, "falling back to db") +} + +func TestLogWarn_Good_NilError(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + err := LogWarn(nil, "op", "msg") + assert.Nil(t, err) + assert.Empty(t, buf.String()) +} + +func TestMust_Good_NoError(t *testing.T) { + // Should not panic when error is nil + assert.NotPanics(t, func() { + Must(nil, "test", "should not panic") + }) +} + +func TestMust_Ugly_Panics(t *testing.T) { + var buf bytes.Buffer + logger := New(Options{Level: LevelDebug, Output: &buf}) + SetDefault(logger) + defer SetDefault(New(Options{Level: LevelInfo})) + + assert.Panics(t, func() { + Must(errors.New("fatal error"), "startup", "initialization failed") + }) + + // Verify error was logged before panic + output := buf.String() + assert.True(t, strings.Contains(output, "[ERR]") || len(output) > 0) +}