Skip to content

feat: removes formatter from Error function#10

Merged
rjfonseca merged 1 commit into
mainfrom
feat/removes-formatter-by-default
Jul 3, 2025
Merged

feat: removes formatter from Error function#10
rjfonseca merged 1 commit into
mainfrom
feat/removes-formatter-by-default

Conversation

@rjfonseca
Copy link
Copy Markdown
Member

The default formatter was caused unwanted behavior with external wrapping methods, like fmt.Errorf, because of how the Error functions was implemented and the use of GetRootError.

Some context was lost in those cases.

The solution was to remove de formatter from the Error function. Now the Error function will only print the inner Error(). This will preserve the context of other errors.

The formatters are still available to be used directly or vai calling the new Format() function that will apply the formatter set in the error chain or the Default one. This was the previous behavior of the Error functions.

Copilot AI review requested due to automatic review settings July 3, 2025 13:26
@rjfonseca rjfonseca requested a review from xico42 as a code owner July 3, 2025 13:26
Copy link
Copy Markdown
Contributor

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 refactors error formatting so that calling Error() returns only the inner error message, and introduces a new Format() function to apply the full formatter chain. Tests and examples have been updated to use Format() instead of relying on the default Error() formatting.

  • Modify Error.Error() to return only the inner error.
  • Add Format() helper and adjust existing formatters.
  • Update tests and examples to call Format() and adjust expected outputs.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
with_test.go Switch tests from err.Error() to Format(err) and updated expected strings.
value_test.go Add new TestValueAllSlice and test empty/error value slicing.
root_error_test.go Add test covering GetRootError with fmt.Errorf wrapping.
formatter_test.go Update formatter tests to use GetFormatter(err)(err) and Format().
formatter.go Introduce Format() function and update formatter implementations.
example/main.go Update example calls to use Format() instead of printing err.
error_test.go Add tests for nil inner error and mixed error chains.
error.go Change Error() to bypass default formatter and return inner error.
README.md Extend example showing both Error() and Format() usage.

Comment thread with_test.go Outdated
Comment thread value_test.go Outdated
Comment thread example/main.go Outdated
@rjfonseca rjfonseca force-pushed the feat/removes-formatter-by-default branch from be4ba44 to 588f28e Compare July 3, 2025 13:58
xico42
xico42 previously approved these changes Jul 3, 2025
The default formatter was causing unwanted behavior with external
wrapping methods, like fmt.Errorf, because of how the Error functions
was implemented and the use of GetRootError.

Some context was lost in those cases.

The solution was to remove de formatter from the Error function. Now the
Error function will only print the inner Error(). This will preserve the
context of other errors.

The formatters are still available to be used directly or by calling
the new Format() function that will apply the formatter set in the
error chain or the Default one. This was the previous behavior of the
Error functions.
@rjfonseca rjfonseca force-pushed the feat/removes-formatter-by-default branch from 588f28e to 5511d76 Compare July 3, 2025 16:30
@rjfonseca rjfonseca merged commit af94aec into main Jul 3, 2025
3 checks passed
@rjfonseca rjfonseca deleted the feat/removes-formatter-by-default branch July 3, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants