Skip to content

feat: improve error handling #5878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 4, 2025
Merged

feat: improve error handling #5878

merged 6 commits into from
Jul 4, 2025

Conversation

caarlos0
Copy link
Member

@caarlos0 caarlos0 commented Jul 4, 2025

  • created a gerrors package
  • wraps errors with details, final message, exit code

@caarlos0 caarlos0 self-assigned this Jul 4, 2025
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 4, 2025
@caarlos0 caarlos0 requested a review from Copilot July 4, 2025 02:09
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new gerrors package for structured error wrapping (with details, messages, and exit codes) and replaces prior ad-hoc error-handling logic across internal pipes and CLI commands.

  • Add internal/gerrors with Wrap, WrapExit, and detail/exit/message accessors
  • Remove old errDetailed from internal/pipe and switch usage to gerrors
  • Update CLI (cmd/) to use gerrors for exit codes, messages, and details, removing legacy helpers

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/pipe/pipe_test.go Removed legacy tests for NewDetailedError
internal/pipe/pipe.go Dropped old DetailsOf and NewDetailedError implementations
internal/pipe/build/build.go Swapped pipe.NewDetailedError for gerrors.Wrap
internal/gerrors/errors.go Added new gerrors implementation
internal/gerrors/errors_test.go Added tests for gerrors.Wrap and accessors
cmd/root.go Replaced custom exit handler with gerrors.ExitOf/MessageOf
cmd/error.go Removed legacy exitError type
cmd/release.go Updated releaseProject to use decorateWithCtxErr and after
cmd/check.go Rewrote error aggregation to use gerrors
cmd/build.go Updated buildProject to use decorateWithCtxErr, after, and gerrors
Comments suppressed due to low confidence (4)

cmd/root.go:4

  • The import "cmp" does not exist in the standard library. If you need an Or helper for strings, consider importing the correct package or implement a small helper locally (e.g., func or(a, b string) string { if a != "" { return a }; return b }).
	"cmp"

cmd/check.go:10

  • This file uses fmt.Errorf and errors.New but does not import "fmt" or "errors". Add these imports to avoid undefined identifier errors.
	"github.com/goreleaser/goreleaser/v2/internal/gerrors"

cmd/release.go:104

  • decorateWithCtxErr and after are defined in cmd/root.go but not visible here. You need to either define them in this file or move them to a shared utility that cmd/release.go can import.
		return decorateWithCtxErr(parent, err, "release", after(start))

cmd/build.go:122

  • decorateWithCtxErr and after are not defined in this file. Either define these helpers here or extract them to a shared package so both build.go and release.go can access them.
		return decorateWithCtxErr(parent, err, "build", after(start))

caarlos0 added 2 commits July 3, 2025 23:21
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Copy link

cloudflare-workers-and-pages bot commented Jul 4, 2025

Deploying goreleaser with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9f899a0
Status: ✅  Deploy successful!
Preview URL: https://c0d6efb9.goreleaser.pages.dev
Branch Preview URL: https://errhandling.goreleaser.pages.dev

View logs

caarlos0 and others added 3 commits July 4, 2025 00:43
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Copy link

codecov bot commented Jul 4, 2025

Codecov Report

Attention: Patch coverage is 90.62500% with 9 lines in your changes missing coverage. Please review.

Project coverage is 82.71%. Comparing base (2e81664) to head (9f899a0).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/root.go 60.00% 3 Missing and 3 partials ⚠️
cmd/build.go 91.66% 1 Missing ⚠️
cmd/release.go 91.66% 1 Missing ⚠️
internal/gerrors/errors.go 97.43% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5878      +/-   ##
==========================================
- Coverage   82.76%   82.71%   -0.05%     
==========================================
  Files         165      165              
  Lines       16526    16521       -5     
==========================================
- Hits        13677    13666      -11     
- Misses       2258     2262       +4     
- Partials      591      593       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@caarlos0 caarlos0 merged commit 4990a80 into main Jul 4, 2025
17 checks passed
@caarlos0 caarlos0 deleted the errhandling branch July 4, 2025 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant