Add trace logging for all squelched errors #1615

osterman · 2025-10-11T19:23:30Z

what

Add log.Trace() calls for all squelched errors throughout the codebase
Update logging documentation to include squelched error handling guidance
Update error handling strategy PRD to document squelched error patterns

why

Ensures no errors are silently lost, even when intentionally ignored
Provides complete error visibility during debugging with --logs-level=Trace
Establishes clear patterns for handling non-critical errors

details

Code Changes (19 files)

Added trace logging for squelched errors in:

Configuration binding: Environment variables and flags (viper.BindEnv(), viper.BindPFlag())
File cleanup: Temporary file and directory removal (os.Remove(), os.RemoveAll())
Resource closing: File handles, clients, connections (Close())
Lock operations: File locks in defer statements (Unlock())
UI operations: Terminal output, command help (fmt.Fprint(), cmd.Help())
Performance tracking: Histogram value recording
Cache operations: Non-critical cache file operations on Windows

Patterns Applied

// ❌ WRONG: Silent error squelching
_ = os.Remove(tempFile)

// ✅ CORRECT: Log squelched errors at Trace level
if err := os.Remove(tempFile); err != nil && !os.IsNotExist(err) {
    log.Trace("Failed to remove temporary file during cleanup", "error", err, "file", tempFile)
}

Special Cases

Defer statements: Capture errors in closures for logging
File existence checks: Use os.IsNotExist() to avoid logging expected conditions
Log file cleanup: Use fmt.Fprintf(os.Stderr, ...) to avoid logger recursion

Documentation Updates

docs/logging.md: Added comprehensive "Squelched Errors" section with patterns and examples
docs/prd/error-handling-strategy.md: Added "Squelched Error Handling" section with guidelines and code examples

references

Related to overall error handling and logging strategy
All changes compile and pass pre-commit hooks

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Prevented silent failures by adding error handling and trace-level logging across command flags, env bindings, config parsing, file cleanup, network response closing, metrics recording, and TUI rendering.
- Improved cleanup reliability by logging non-fatal errors during temporary file/dir removal and resource closure.
Refactor
- Standardized logging for squelched (non-fatal) errors, replacing ignored returns with guarded paths without changing core behavior.
Documentation
- Expanded logging guidance with a new “Squelched Errors” section, examples, patterns, and best practices.
- Updated error-handling strategy to include when and how to log squelched errors.

Audit and update all instances where errors are intentionally ignored (squelched) to include log.Trace() calls. This ensures no errors are silently lost and provides complete error visibility during debugging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

coderabbitai · 2025-10-11T19:26:24Z

📝 Walkthrough

Walkthrough

Adds Trace-level logging for previously ignored errors across CLI flag/env bindings, cleanup/close operations, config/cache reads, and various package utilities. Updates docs to define “squelched errors” and prescribe logging patterns. No public API changes; control flow continues after logging. Minor logging import adjustments throughout.

Changes

Cohort / File(s)	Summary of changes
CLI auth flag/env binding logging `cmd/auth.go`, `cmd/auth_env.go`, `cmd/auth_validate.go`, `cmd/auth_whoami.go`	Guard viper BindEnv/BindPFlag and completions with error checks; log Trace on failures instead of ignoring.
CLI helpers and root cleanup `cmd/cmd_utils.go`, `cmd/root.go`, `cmd/validate_editorconfig.go`	Log Trace on Help(), log Sync/Close failures during log cleanup, and log Parse() errors; continue execution.
Documentation: squelched errors `docs/logging.md`, `docs/prd/error-handling-strategy.md`	Add/expand guidance on squelched error handling, patterns, examples, and revision history.
Terraform exec/backends cleanup logging `internal/exec/terraform.go`, `internal/terraform_backend/terraform_backend_s3.go`	Log Trace on os.Remove plan/var cleanup failures and on S3 response body Close errors; proceed normally.
TUI render output logging `internal/tui/workflow/list_item.go`	Check Fprint error; log Trace on write failure.
GitHub OIDC provider env binds `pkg/auth/providers/github/oidc.go`	Guard BindEnv for multiple keys; log Trace on bind failures; existing token/URL reads and validation unchanged.
Config cache and unix locks `pkg/config/cache.go`, `pkg/config/cache_lock_unix.go`	Log Trace on ReadInConfig/Unmarshal failures (Windows path) and on unlock errors in defer blocks.
Downloader git temp cleanup `pkg/downloader/get_git.go`	Log Trace on temp SSH key Close/Remove failures and on git cleanup RemoveAll failures.
Perf metrics recording `pkg/perf/perf.go`	Log Trace when RecordValue returns error; continue metrics flow.
Stores cleanup and GSM client close `pkg/store/artifactory_store.go`, `pkg/store/google_secret_manager_store.go`	Log Trace on temp dir/file cleanup failures and on GSM client Close error after creation failure.
Utils: print and URL open `pkg/utils/log_utils.go`, `pkg/utils/url_utils.go`	Log Trace on Fprint failure; guard BindEnv for go.test and log on bind failure; update logger import.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Cobra as Cobra Command
  participant Viper as Viper
  participant Logger as Logger(Trace)

  User->>Cobra: init()
  Cobra->>Viper: BindPFlag("key", flag)
  alt bind error
    Viper-->>Cobra: error
    Cobra->>Logger: Trace("BindPFlag failed", err, key)
    Note over Cobra,Logger: Flow continues
  else ok
    Viper-->>Cobra: nil
  end
  Cobra->>Viper: BindEnv("key")
  alt bind error
    Viper-->>Cobra: error
    Cobra->>Logger: Trace("BindEnv failed", err, key)
  else ok
    Viper-->>Cobra: nil
  end

sequenceDiagram
  autonumber
  participant Op as Operation
  participant OS as OS/File/Net
  participant Logger as Logger(Trace)

  Op->>OS: Remove/Close/Sync(...)
  alt error
    OS-->>Op: error
    Op->>Logger: Trace("cleanup failed", err, path|handle)
    Note over Op,Logger: Continue without surfacing error
  else ok
    OS-->>Op: nil
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

feat: add trace log level support for enhanced debugging #1525 — Introduced/adjusted logger APIs and Trace level that this PR leverages for squelched error logs.
Document Atmos Development Conventions #793 — Also edits docs/logging.md regarding log-level conventions, overlapping with this PR’s documentation changes.
Google secret manager store implementation #1034 — Touches the same Google Secret Manager store; this PR augments its error handling on client close.

Suggested labels

patch

Suggested reviewers

aknysh
mcalhoun
Gowiem

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title precisely and succinctly conveys the main change of the pull request, which is adding trace-level logging for all squelched errors across the codebase. It is clear, concise, and directly related to the changeset without extraneous details.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch osterman/log-squelched-errors

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4275bb8 and 2ae22ad.

📒 Files selected for processing (4)

cmd/cmd_utils.go (1 hunks)
cmd/root.go (1 hunks)
internal/exec/terraform.go (1 hunks)
pkg/perf/perf.go (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

pkg/perf/perf.go
internal/exec/terraform.go
cmd/root.go

🧰 Additional context used

📓 Path-based instructions (4)

cmd/**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate

cmd/**/*.go: Define one Cobra command per file under cmd/ and follow example pattern with embedded markdown examples.
New commands should rely on automatic telemetry capture; add explicit CaptureCmd/CaptureCmdString only for non-standard paths.

Files:

cmd/cmd_utils.go

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All comments must end with periods across all Go code (godot linter enforced).
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sorted alphabetically within groups; preserve existing aliases.
Add defer perf.Track(<atmosConfig|nil>, ".")() at the start of all public and critical private functions, followed by a blank line.
All errors must be wrapped using static errors from errors/errors.go; prefer errors.Join for multiple errors, fmt.Errorf with %w for context, and check with errors.Is; never use dynamic errors directly.
Use utils.PrintfMarkdown() to render embedded markdown examples for CLI help output.
Co-locate unit tests with implementation files; integration tests reside under tests/.
Distinguish UI output from logging: UI prompts/status/errors to stderr; data/results to stdout; logging for system/debug info only with structured fields.
Most text UI must go to stderr; only data/results to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
Bind all environment variables with viper.BindEnv(); every env var must have an ATMOS_ alternative binding.
Favor cross-platform code: prefer SDKs over external binaries, use filepath/os/runtime, and handle OS-specific differences or use build tags.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd or telemetry.CaptureCmdString without collecting user data.
Search for existing methods and utilities (internal/exec, pkg/) before implementing new functionality; prefer reuse/refactoring over duplication.

Files:

cmd/cmd_utils.go

**/!(*_test).go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

Document all exported functions, types, and methods with Go doc comments

Files:

cmd/cmd_utils.go

cmd/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Place each CLI command in its own file (one command per file) under cmd/.

Files:

cmd/cmd_utils.go

🧬 Code graph analysis (1)

cmd/cmd_utils.go (1)

pkg/logger/log.go (1)

Trace (14-16)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: Analyze (go)
GitHub Check: Lint (golangci)
GitHub Check: Lint (golangci)
GitHub Check: Analyze (go)
GitHub Check: Build (macos-latest, macos)
GitHub Check: Build (windows-latest, windows)
GitHub Check: Summary

🔇 Additional comments (1)

cmd/cmd_utils.go (1)

295-297: LGTM! Trace logging added for squelched Help error.

The implementation correctly logs the error from cmd.Help() at Trace level before proceeding to exit. This aligns with the PR's pattern for surfacing intentionally squelched errors without changing control flow.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-10-11T19:37:29Z

Codecov Report

❌ Patch coverage is 2.50000% with 117 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.50%. Comparing base (9b5598a) to head (2ae22ad).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/auth_env.go	0.00%	9 Missing and 4 partials ⚠️
cmd/auth.go	0.00%	8 Missing and 4 partials ⚠️
pkg/auth/providers/github/oidc.go	0.00%	8 Missing and 4 partials ⚠️
pkg/store/artifactory_store.go	0.00%	9 Missing and 3 partials ⚠️
pkg/downloader/get_git.go	0.00%	10 Missing ⚠️
cmd/root.go	0.00%	5 Missing and 2 partials ⚠️
cmd/auth_validate.go	0.00%	4 Missing and 2 partials ⚠️
cmd/auth_whoami.go	0.00%	4 Missing and 2 partials ⚠️
internal/exec/terraform.go	0.00%	4 Missing and 2 partials ⚠️
pkg/config/cache.go	0.00%	6 Missing ⚠️
... and 8 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1615      +/-   ##
==========================================
- Coverage   64.65%   64.50%   -0.15%     
==========================================
  Files         333      333              
  Lines       37218    37281      +63     
==========================================
- Hits        24062    24049      -13     
- Misses      11245    11292      +47     
- Partials     1911     1940      +29

Flag	Coverage Δ
unittests	`64.50% <2.50%> (-0.15%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/validate_editorconfig.go	`22.51% <100.00%> (+0.51%)`	⬆️
cmd/cmd_utils.go	`24.71% <0.00%> (-0.05%)`	⬇️
internal/terraform_backend/terraform_backend_s3.go	`89.36% <0.00%> (-3.03%)`	⬇️
internal/tui/workflow/list_item.go	`0.00% <0.00%> (ø)`
pkg/perf/perf.go	`90.58% <0.00%> (-1.72%)`	⬇️
pkg/store/google_secret_manager_store.go	`69.81% <0.00%> (-0.32%)`	⬇️
pkg/utils/log_utils.go	`62.50% <0.00%> (-37.50%)`	⬇️
pkg/utils/url_utils.go	`0.00% <0.00%> (ø)`
cmd/auth_validate.go	`12.90% <0.00%> (-9.32%)`	⬇️
cmd/auth_whoami.go	`2.83% <0.00%> (-2.08%)`	⬇️
... and 9 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Trace logging is diagnostic infrastructure code that's difficult to test meaningfully. Tests would need to inject errors and verify log output, which doesn't add meaningful value to coverage metrics. This change adds an ignore_patterns section to exclude lines containing log.Trace( from coverage calculations, addressing the low patch coverage on PR #1615 where 117 out of 120 new lines are trace logging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 27e6b48 and 4275bb8.

📒 Files selected for processing (1)

codecov.yml (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-10-07T14:35:22.186Z

Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T14:35:22.186Z
Learning: Applies to **/mock_*.go : Exclude files matching mock_*.go from coverage metrics.

Applied to files:

codecov.yml

📚 Learning: 2025-10-07T14:35:22.186Z

Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T14:35:22.186Z
Learning: Applies to **/mock/*.go : Exclude files under any mock/ directory from coverage metrics.

Applied to files:

codecov.yml

📚 Learning: 2025-10-07T14:35:22.186Z

Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T14:35:22.186Z
Learning: Applies to @(cmd|internal|pkg)/**/!(*_test).go : Ensure 80% minimum coverage on new/changed lines; exclude mock files from coverage (mock_*.go, **/mock_*.go, **/mock/*.go); include comprehensive unit and required integration tests.

Applied to files:

codecov.yml

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: Build (ubuntu-latest, linux)
GitHub Check: Build (windows-latest, windows)
GitHub Check: Analyze (go)
GitHub Check: Lint (golangci)
GitHub Check: Run pre-commit hooks
GitHub Check: Summary

codecov.yml

mergify · 2025-10-11T23:24:37Z

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

Resolved conflict in pkg/perf/perf.go by keeping the refactored recordMetrics function from main and adding trace logging for histogram recording errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

github-actions · 2025-10-13T01:12:31Z

These changes were released in v1.194.1-rc.0.

osterman requested review from a team as code owners October 11, 2025 19:23

github-actions bot added the size/m Medium size PR label Oct 11, 2025

coderabbitai bot previously approved these changes Oct 11, 2025

View reviewed changes

osterman added the no-release Do not create a new release (wait for additional code changes) label Oct 11, 2025

osterman dismissed coderabbitai[bot]’s stale review via 4275bb8 October 11, 2025 19:46

coderabbitai bot requested changes Oct 11, 2025

View reviewed changes

codecov.yml Outdated Show resolved Hide resolved

osterman force-pushed the osterman/log-squelched-errors branch from 4275bb8 to 27e6b48 Compare October 11, 2025 20:49

mergify bot added the conflict This PR has conflicts label Oct 11, 2025

mergify bot removed the conflict This PR has conflicts label Oct 12, 2025

coderabbitai bot approved these changes Oct 12, 2025

View reviewed changes

aknysh approved these changes Oct 12, 2025

View reviewed changes

aknysh merged commit 09f27f2 into main Oct 12, 2025
58 checks passed

aknysh deleted the osterman/log-squelched-errors branch October 12, 2025 02:57

coderabbitai bot mentioned this pull request Nov 8, 2025

fix: Enable YAML function authentication in terraform commands with --identity flag #1769

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add trace logging for all squelched errors #1615

Add trace logging for all squelched errors #1615

Uh oh!

osterman commented Oct 11, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 11, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

codecov bot commented Oct 11, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

mergify bot commented Oct 11, 2025

Uh oh!

Uh oh!

github-actions bot commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Add trace logging for all squelched errors #1615

Add trace logging for all squelched errors #1615

Uh oh!

Conversation

osterman commented Oct 11, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what

why

details

Code Changes (19 files)

Patterns Applied

Special Cases

Documentation Updates

references

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mergify bot commented Oct 11, 2025

Uh oh!

Uh oh!

github-actions bot commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

osterman commented Oct 11, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 11, 2025 •

edited

Loading

codecov bot commented Oct 11, 2025 •

edited

Loading