errors: ensure the 1.13 method to access the cause is called Cause() not Unwrap()

[knz]

The rationale in https://go.googlesource.com/proposal/+/master/design/go2draft-error-values-overview.md mentions this:

Unwrap. It is unclear if Unwrap is the right name for the method returning the next error in the error chain. Dave Cheney’s github.com/pkg/errors has popularized Cause for the method name, but it also uses Cause for the function that returns the last error in the chain. At least a few people we talked to did not at first understand the subtle semantic difference between method and function. An early draft of our design used Next, but all our explanations referred to wrapping and unwrapping, so we changed the method name to match.

In summary the rationale was "some people were confused so we're going to change the name".

However this choice creates a more serious technical problem.

Suppose a program upgrades to use Go 1.13 but has some dependency library that relies on github.com/pkg/errors (or another Cause()-compatible error library). Now consider what happens if our program makes some API call through its dependency, and that encounters an error from the standard library -- for example io.EOF or context.DeadlineExceeded. And then the intermediate library dependency wraps the error using the facilities in github.com/pkg/errors which only provides Cause().

How is the program now meant to identify the ultimate cause of its error?

The proposal with Is() and Unwrap() will be unable to "peek through" the wrappers created by the existing library code. This is defective design.

Solution: By changing the design in Go 1.13 to instead make the causer access method remain Cause(), the new 1.13 error package would become/remain drop-in compatible with existing library error wrapper code.

[ianlancetaylor]

CC @jba @neild

[jba]

It is not a goal of the proposal to be compatible with existing error packages. Indeed, that is impossible, since they contradict each other. The goal is to establish a common set of names and behaviors, and the hope is that existing packages like pkg/errors will adopt them.

How is the program now meant to identify the ultimate cause of its error?

Couldn't you continue to use Cause? Leaving aside the fact that you should never wrap io.EOF, you could write

if pkgerrors.Cause(err) == io.EOF || xerrors.Is(err, io.EOF) { ... }

[knz]

It is not a goal of the proposal to be compatible with existing error packages.

1. why?
2. can you also explain why in the proposal itself

Indeed, that is impossible, since they contradict each other.

why/how?

Couldn't you continue to use Cause? Leaving aside the fact that you should never wrap io.EOF, you could write
if pkgerrors.Cause(err) == io.EOF || xerrors.Is(err, io.EOF) { ... }

No that will fail if an error is wrapped both by xerrors and pkgerrors.

[neild]

why/how?

There exist error implementations where the Cause method returns the root of the chain, as well as ones where Cause returns the next step in the chain. There is no way to reconcile these two definitions.

[knz]

There exist error implementations where the Cause method returns the root of the chain, as well as ones where Cause returns the next step in the chain. There is no way to reconcile these two definitions.

Can you provide examples. This is not my understanding. If you look at the doc of the related packages you will see the following used consistently:

• the package-level function accesses the root cause
• the causer interface method Cause() accesses the next step

What is the evidence you've found that some libraries do it differently? Are you going to report this evidence in the proposal?

[neild]

What is the evidence you've found that some libraries do it differently? Are you going to report this evidence in the proposal?

One package where the Cause method returns the root of the chain is gopkg.in/errgo.v2/errors, which is cited in the draft design for error inspection.

[knz]

I went and looked at the package you linked, and I am afraid the source code contradicts your statements:

• the Cause() method does exactly what I said and only accesses one level up -- which is what the top level issue in this thread is requesting for Go 1.13. See the code here:
  https://github.com/go-errgo/errgo/blob/v2.1.0/errors/struct.go#L48

• the Cause() package-level function accesses just 1 level deep (instead of the root cause) and I would agree it is better to provide Is() like you did, but this point is irrelevant for the issue at hand.

So at this point I still see no evidence of library code in the ecosystem where the Cause() method is contradicting the proposal.

I would kindly request that you either provide some evidence or instead reconsider the Go 2/1.13 proposal for the method as requested at the top of this thread.

[neild]

I went and looked at the package you linked, and I am afraid the source code contradicts your statements:

You are mistaken:
https://play.golang.org/p/1rR97pPojOd

[jba]

It looks to me like the errgo Cause function goes one level:

func Cause(err error) error {
	if err, ok := err.(Causer); ok {
		if cause := err.Cause(); cause != nil {
			return cause
		}
	}
	return err
}

while the pkg/errors one follows the chain:

func Cause(err error) error {
	type causer interface {
		Cause() error
	}

	for err != nil {
		cause, ok := err.(causer)
		if !ok {
			break
		}
		err = cause.Cause()
	}
	return err
}

[jba]

No that will fail if an error is wrapped both by xerrors and pkgerrors.

You could instead write your own function, looking for both Cause and Unwrap methods.

[knz]

You are mistaken:
play.golang.org/p/1rR97pPojOd

Thank you for the example, although I believe it is disingenuous: this exemplifies specifically the feature of that package that hides the cause. The goal of that Note API is to provide control for the code that constructs the error on the ability for other downstream code to query that cause. That use case / requirement would exist also for users of xerrors and arguably an xerror-compatible implementation could provide the same feature that prevents a cause from being visible via Unwrap().

So far it does not seem you have provided evidence that in the common case (even a common case for gopkg.in/errgo.v2/errors) the established ecosystem is incompatible with the Go 1.13 proposal, and it does not seem that the conversation so far has provided a reason to keep the current design that outweighs the inconvenience that I have outlined initially.

No that will fail if an error is wrapped both by xerrors and pkgerrors.
You could instead write your own function, looking for both Cause and Unwrap methods.

Yes I am aware I could do this. However, if I and other people find ourselves required to do this often (as I am arguing since the beginning of the thread), this would be strong evidence that the proposed definition of Go's error API is defective. Now (before 1.13 is released) would be an adequate time to study this risk and perhaps make adjustments before the Go user community finds themselves inconvenienced by the defect.

[darkfeline]

As a matter of practicality, golang.org/x/xerrors uses Unwrap(), and there are people using that package now. Which means that whether we end up choosing Unwrap() or Cause() or something else, everyone else is going to have to add backward compatibility,

If we choose Unwrap(), then github.com/pkg/errors and everyone else will have to add support for Unwrap().

If we choose Cause(), then golang.org/x/xerrors and everyone else will have to add support for Cause().

And ditto for any other API that is out there.

I understand that the error values proposal isn't final yet, but I think it would be impractical to change it drastically without a strong reason since people are using it already (including me). Similarly, there was no guarantee that github.com/pkg/errors provided any kind of standard API across the Go ecosystem and their users should have understood that they were committing to an API only implemented by github.com/pkg/errors and any explicitly cooperating packages.

The point of this part of the error values proposal, as I see it, was to commit to a standard API across the Go ecosystem. That it is not the github.com/pkg/errors API is perhaps unfortunate, but at this point in time being noncommittal about the API will cause more harm than benefit.

Addressing @knz's concern directly,

Suppose a program upgrades to use Go 1.13 but has some dependency library that relies on github.com/pkg/errors (or another Cause()-compatible error library). Now consider what happens if our program makes some API call through its dependency, and that encounters an error from the standard library -- for example io.EOF or context.DeadlineExceeded. And then the intermediate library dependency wraps the error using the facilities in github.com/pkg/errors which only provides Cause().

How is the program now meant to identify the ultimate cause of its error?

• If your program previously used Cause style functions to check errors, it will keep working.
• If you switched your program to use Unwrap style functions on a library that doesn't support it, well, don't do that.
• If the library adds support for Unwrap style errors (which is trivial), then you are free to to switch to using Unwrap style functions or keep using Cause style functions.

The error values proposal doesn't change the fact that every package needs to document an explicit contract about how a user will go about unwrapping or checking errors, or the backward compatibility implications thereof. All it does is adds a standard contract that hopefully all packages can agree upon going forward.

[knz]

As a matter of practicality, golang.org/x/xerrors uses Unwrap(), and there are people using that package now. Which means that whether we end up choosing Unwrap() or Cause() or something else, everyone else is going to have to add backward compatibility

I am really glad that you recognize that some people are going to be inconvenienced.

It would be interesting here, for the sake of clarity and methodology, to estimate the size of the user populations. AFAIK xerrors has been around for less time than the alternatives using Cause().

There is a utility argument to be made about the amount of inconvenience. A solution that causes less inconvenience in aggregate seems (to me) to be the better solution. I am sure that the fact that you happen to be invested in the initial development of xerrors will not bias you against looking at the utility aspect as a whole.

[tv42]

I would expect third party packages like github.com/pkg/errors to change their Cause function to support both their own Cause method and Unwrap. That seems like the way to migrate from github.com/pkg/errors to xerrors to Go1.14.

[knz]

You realize that you could change the go 1.14/xerror code to use Cause instead and take the cost of the change for yourself, instead of pushing it to the community right? So far the discussion has been pretty clear: 1) there's no real good reason to keep Unwrap over Cause other than choosing who to inconvenience the most 2) in terms of number of people inconvenienced you are choosing the maximum amount of inconvenience because... I guess it makes someone's life easier, consequences be dammed? -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[josharian]

@knz please be respectful.

[AlekSi]

Was this issue a reason for revert at 3e2c522?

Edit after @jba's answer below.
Found it: #29934 (comment)

[jba]

No.

[jba]

Our final decision is to stay with Unwrap.