Skip to content
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

Enable Errors support for any multi-error #75

Merged
merged 2 commits into from
Mar 16, 2023
Merged

Enable Errors support for any multi-error #75

merged 2 commits into from
Mar 16, 2023

Conversation

sywhang
Copy link
Contributor

@sywhang sywhang commented Mar 14, 2023

Starting Go 1.20, any multi-error should conform to the standard unwrap method: Unwrap() []error.

This changes multierr.Errors() method to support any error that complies to that interface.

Fix #70 / GO-1883

error_post_go120.go Outdated Show resolved Hide resolved
error_post_go120.go Outdated Show resolved Hide resolved
Starting Go 1.20, any multi-error should conform to the standard
unwrap method: Unwrap() []error.

This changes multierr.Errors() method to support any error that
complies to that interface.

Fix #70.
Copy link
Collaborator

@abhinav abhinav left a comment

Choose a reason for hiding this comment

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

LGTM!
Please add a changelog entry.

return []error{err}
}

return append(([]error)(nil), eg.Unwrap()...)
Copy link

Choose a reason for hiding this comment

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

out of curiosity, why a new slice rather than the old one?

it would seem that any unrwappable error should be okay with us passing the slice up?

Copy link
Collaborator

@abhinav abhinav Mar 15, 2023

Choose a reason for hiding this comment

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

It's a new slice because multierr.Errors specifically returns a copy of the slice.
Docs say:

Callers of this function are free to modify the returned slice.

Unwrap() []error returns the underlying slice. Per the proposal:

Callers must not modify the list returned by Unwrap.

And indeed, in the implementation:

func (e *joinError) Unwrap() []error {
	return e.errs
}

So we have to create a new slice here.

@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #75 (c934b2a) into master (8767aa9) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #75   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          110       117    +7     
=========================================
+ Hits           110       117    +7     
Impacted Files Coverage Δ
error.go 100.00% <100.00%> (ø)
error_post_go120.go 100.00% <100.00%> (ø)
error_pre_go120.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sywhang sywhang merged commit faff69d into master Mar 16, 2023
@sywhang sywhang deleted the 70 branch March 16, 2023 05:40
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.

Go 1.20: Errors() should support any multi-error
5 participants