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

proposal: Go 2: universal WrapError type #36994

Closed
JavierZunzunegui opened this issue Feb 3, 2020 · 10 comments
Closed

proposal: Go 2: universal WrapError type #36994

JavierZunzunegui opened this issue Feb 3, 2020 · 10 comments
Labels
FrozenDueToAge Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Milestone

Comments

@JavierZunzunegui
Copy link

JavierZunzunegui commented Feb 3, 2020

go2 proprosal: universal WrapError type

See filled questionare #36994 (comment)

This is a go2 proposal in the context of error wrapping. It is a concrete follow up to the more open ended #35929 (errors: how should formatting be handled?).

It requires changes to the errors package but no language change. All code is in this repo.

Context

Error wrapping support was added in go 1.13. It did not introduce some advanced features like error formatting and stack traces, not because these were not desirable (Error Values — Problem Overview, Error Printing — Draft Design) but because no good implementation was found that delivered them. This puts forward an implementation that delivers on the above, and which can easily introduce new feautures going forward.

Abstract

Currently, to be a wrapping error means implementing interface {Unwrap() error}.

Under this proposal, it means beaing an instance of the new WrapError struct, which itself contains plain, non-wrapping errors. The key advantage is new features can be added accross all wrapped error uses by adding functionality to WrapError, as opposed to introducing additional interfaces that have to be inplemented by every individual error type.

Details

Immediate Changes

The following are to be added to errors package (implementations in werrors.go):

  • type WrapError struct: effectively a linked list of errors, this is set to become the universal type for all wrapped errors. It supports Is, As and Unwrap as expected.
  • func Wrap(error, error) error: used to produce the WrapError error chain. All error wrapping is set to be produced through this method.

This is the extent of the 'immediate' changes put forward by this proposal.

Follow ups

The more advanced features can be built on top of these changes as demonstrated in the following orthogonal extensions. This are in place to demostrate the ease with which such features can be delivered, but are not complete implementations of such features.

Note that the above can be introduced without requiring changes on any other error type other than WrapError. More generally, in a WrapError compliant world adding functionality to all wrapped errors requires updating WrapError only.

Limitations and Breaking Changes

Note the migration to WrapError is needed before the follow ups above (and theremore, the primary gains) can be delivered, but WrapError and Wrap can be introduced (with no additional functionality) without any breaking change. This allows for a gradual migration before the ultimate breaking changes.

The primary backwards-incompatible changes are:

  • Removal of interface {Unwrap() error}:
    it was added with go1.13 to the errors package as the fundamental method that defines wrapped errors. Errors implementing this need to be changed to no longer have this method nor contain other errors in their fields, and the wrapping be provided by the new Wrap method. This represents the largest breaking change in the proposal. A significant error in this category is os.PathError.
  • Limited fmt.Errorf support:
    While %w-suffixed strings can be supported, having it elsewhere can't. This makes fmt.Errorf("foo, err=%w, bar", err) unsupportable, but fmt.Errorf("foo, bar, err: %w", err) is. Even in the suffix scenario, the requiremenet for formatting imposes for a uniform suffix syntax, presumably the ": %w" that was part of an earlier errors draft. A new fmt.WErrorf(error, string, ...interface{}) without %w may be altogether more suitable. Note that, more than an implementation detail, such standarisation is required if an error is to support formatting.
  • Phasing out type assertions:
    While the preferred error type checking is via errors.Is and errors.As, type assertions are still used widely. These will not work as expected under WrapError,
    since the type of the error is always WrapError even if the last wrapped error is of the type being asserted.
@gopherbot gopherbot added this to the Proposal milestone Feb 3, 2020
@JavierZunzunegui
Copy link
Author

Though not a language change, following the new questionaire for language changes in the hope of making this easier to review.


Would you consider yourself a novice, intermediate, or experienced Go programmer?

Expert, ~6y using it.

What other languages do you have experience with?

C++ and Java, though neither at the level of Go.

Would this change make Go easier or harder to learn, and why?

No meaningful change.

Has this idea, or one like it, been proposed before?

If so, how does this proposal differ?

Go team proposals:
Error Values - Problem Overview and Error Printing — Draft Design: same objective (error formatting), but different implementation.

My own proposals:

Who does this proposal help, and why?

Anyone embracing wrapped errors trying to do anything but the bare minimum with them. For example doing not standard error printing (previously, I myself had to do JSON error encoding of wrapped errors) or wanting stack frames on their errors.

What is the proposed change?

Please describe as precisely as possible the change to the language.
What would change in the [https://golang.org/ref/spec](language spec)?
Please also describe the change informally, as in a class teaching Go.

No language change.
At first 1 additional type and 1 additional function in the errors package. Follow up changes will require further functions in the errors package.

Is this change backward compatible?
Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit.

(in terms of standard library)
The immediate change, yes.
The follow up changes would have breaking changes, primarily the removal of interface {Unwrap() error} and allowed formatting in fmt.Errorf. Described in detail in the main post.

Show example code before and after the change.

Very little change for the average user of wrapped erorrs. Primarily from:

if err := nil {
  return myError{myData: ..., e: err}
}

to

if err := nil {
  return errors.Wrap(err, myError{myData: ...})
}

Richer examples in this repo and the various variants in the extensions directory within it.

What is the cost of this proposal? (Every language change has a cost).

No language change.

How many tools (such as vet, gopls, gofmt, goimports, etc.) would be affected?

Primarily vet, to highlight error types with an Unwrap() error method, errors containing other errors or unsupported formatting in fmt.Errorf.

What is the compile time cost?

None expected.

What is the run time cost?

The optimize extension actually shows it can be made to be more efficient than the current implementation, particularly for longer chains of wrapped errors. Adding stack frames would have a cost, though.

Can you describe a possible implementation?

Do you have a prototype? (This is not required.)

See this repo and the various extensions. The implementation is actually very small.

How would the language spec change?

It doesn't.

Orthogonality: how does this change interact or overlap with existing features?

N/A

Is the goal of this change a performance improvement?

If so, what quantifiable improvement should we expect?
How would we measure it?

Performance is not a goal.

Does this affect error handling?

If so, how does this differ from previous error handling proposals?

Yes, it targets error handling directly. It changes the definition of a wrapped error.

Is this about generics?

If so, how does this differ from the the current design
draft

and the previous generics proposals?

No

@jba
Copy link
Contributor

jba commented Feb 3, 2020

It changes the definition of a wrapped error.

We want a wrapped error to be defined by an interface so existing errors can conform. By requiring errors to be instances of WrapError, you would be breaking not only many things in the standard library, but also many error types across the Go ecosystem. I don't see how such a change could be smoothly introduced.

@JavierZunzunegui
Copy link
Author

But by having wrapped errors defined by an interface the other advanced features can't be delivered. The cost of not doing anything is not negligible by any means.

I don't see how such a change could be smoothly introduced.

Best we can do is mitigate the damage. For that, the most urgent thing is to introduce the Wrap method and encourage it (via documentation, blog posts, vet, etc). At the very least, it won't make the problem larger by adding new instances of the interface one, and hopefully the errors than can be migrated without breaking their exported API will be. Hopefully most breaking errors can be dealt with through rewrite rules. Worst case scenario, it becomes a long process of many individual incompatibility breaks.

@JavierZunzunegui
Copy link
Author

I have released a library, zerrors that implements the ideas exposed above, namely:

  • frame info: simply include in your main package import _ "github.com/JavierZunzunegui/zerrors/zmain" for wrapped error with Error() as "{err1}:{err2}: ... : {errN}" to transparently become "{err1} ({file1}:{line1}): {err2} ({file2}:{line2}): ... : {errN} ({fileN}:{lineN})".
  • error formatting: does the default error format (characterised by ": ") not work for you, or do you want the frames formatted differently? It is supported, i.e. the above could also be made to produce "{err1} - {err2} - ... - {errN}" , or indeed any other format.

The library is performant, in fact the frameless form is more so that fmt.Errorf or errors.New. Frames do come at a cost. Details in benchmark results.

The library uses the idea of a universal wrap error type outlines in this proposal.

@rsc rsc added the v2 A language change or incompatible library change label Feb 26, 2020
@ianlancetaylor
Copy link
Contributor

Any project can start using their own new error type.

It's clear that different Go programs have different needs for error types. The idea of adding new features to a standard error type runs directly counter to that. We want the error type to provide the minimum required for all Go programs. We don't want to slowly add features to it.

As @jba notes, the transition from error to some new type would be difficult, slow, and would never complete.

It seems better to provide this in a separate package, as you've already done.

@JavierZunzunegui
Copy link
Author

Any project can start using their own new error type.

Of course - while the universal wrapper type holds the wrapping functionality, it may carry any custom error. That is already in zerrors: func Wrap(inErr, outErr error) error.

It's clear that different Go programs have different needs for error types. The idea of adding new features to a standard error type runs directly counter to that. We want the error type to provide the minimum required for all Go programs. We don't want to slowly add features to it.

As @jba notes, the transition from error to some new type would be difficult, slow, and would never complete.

To clarify it is not error that I propose to change in any way, but its wrapper form, interface {Unwrap() error}. The reason being the interface is not enough to do anything but the most basic wrapping, and building it up as an interface to have multiple methods (such as Format or similar) with an expected (non-trivial) standard implementation is probably more unrealistic still.

It seems better to provide this in a separate package, as you've already done.

I understand this has to start like this, as a separate library, and prove itself first. But while it remains outside the standard library and interface {Unwrap() error} exists, it is not universal, and the existance of such errors means it can't fully and safely implement all the things it could otherwise (including features that were once part of the official errors proposal). But I think it can show it could if it were to replace interface {Unwrap() error}, and I will try to ilustrate that (I am improving zerrors in that direction).

@JavierZunzunegui
Copy link
Author

I have released zerrors@v0.2.0.

WRT error formatting & stack traces: a single error produced by zerrors can be serialised in arbitrary ways, with or without stack frames. This is best shown in the benchmarks: the zerrors there (wErr and swErr) can produce both their own encoding as well as the %+v format of the popular golang.org/x/xerrors and github.com/pkg/errors(*). In other words, zerrors support error formatting without imposing any requirements on custom errors.

v0.2.0 also includes support for modifying the default encoding of zerrors, i.e. if you are migrating from say golang.org/x/xerrors, you can have all zerror's Error() and %+v format string forms remain unchanged.


(*): github.com/pkg/errors does full stack capture vs zerrors single frame capture, which means it can't 100% replicate the format since the last few frames are not captured by zerrors. Because golang.org/x/xerrors does single frame capturing too, zerrors does reproduce it exactly.

@ianlancetaylor
Copy link
Contributor

This has been released as a third party package (thanks!). Let's see how that goes before considering adopting this into the standard library in some future release. For now, this is a likely decline. Leaving open for four weeks for final comments.

@JavierZunzunegui
Copy link
Author

@ianlancetaylor I will be trying to spread awareness. Any visibility boost would be very welcome, I not active nor experienced in sharing to the community.

@ianlancetaylor
Copy link
Contributor

No change in consensus.

@golang golang locked and limited conversation to collaborators Apr 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-FinalCommentPeriod v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants