Is returning another concrete error value a breaking API change?

[minux]

Hi gophers,

https://golang.org/cl/9236 and others unified the way the net package

returns errors,  and we found that it breaks one of the test in the syscall

package, see https://golang.org/issue/10703 (the fix is at https://golang.org/cl/9714).

I'm concerned that changing the concrete type returned by an error might

be an API change as the user has to change the source code after this

change and even worse, the change is silent in the sense that it won't

break compilation of user code.

Should we reconsider those changes?

Thanks.

[Dave Cheney]

Unless the contract or documentation of the method guarantees a particular set of error values, I don't think this is a breaking change.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Andrew Gerrand]

The change is not a violation of the compatibility promise, so technically we can do it.

It is probably worth writing to golang-nuts and asking about people's handling of net errors. Maybe even worth surveying the corpus of open source Go code for things that might break. (Josh? :)

Andrew

--

[minux]

On Wed, May 6, 2015 at 1:02 AM, Andrew Gerrand <a...@golang.org> wrote:

The change is not a violation of the compatibility promise, so technically we can do it.

I agree that the Go 1 API guarantee doesn't forbid this kind of changes, but my concern

is that it still breaks source level compatibility and a test in the standard library.

I find it hard to believe that the test in the syscall package is badly written.

There is nothing in doc/go1.5.txt for this potentially breaking change (not in the strictest

sense of breaking API, but it does have the potential to break user programs, silently.)

[Andrew Gerrand]

On 6 May 2015 at 15:34, minux <mi...@golang.org> wrote:

On Wed, May 6, 2015 at 1:02 AM, Andrew Gerrand <a...@golang.org> wrote:

The change is not a violation of the compatibility promise, so technically we can do it.

I agree that the Go 1 API guarantee doesn't forbid this kind of changes, but my concern

is that it still breaks source level compatibility and a test in the standard library.

I find it hard to believe that the test in the syscall package is badly written.

I wouldn't say that it's badly written, but it is arguably too tightly coupled to the implementation.

 

There is nothing in doc/go1.5.txt for this potentially breaking change (not in the strictest

sense of breaking API, but it does have the potential to break user programs, silently.)

Well, that is a serious omission. Mikio?

[Mikio Hara]

On Wed, May 6, 2015 at 2:36 PM, Andrew Gerrand <a...@golang.org> wrote:

> Well, that is a serious omission. Mikio?

yup, sent: https://golang.org/cl/9775
also iant suggests to update the package documentation
(golang.org/issue/10624) and i will try it later, during go1.5 cycle.

[Jérôme Champion]

Isn't this article quite spot on? http://dave.cheney.net/2014/12/24/inspecting-errors

Perhaps it would be easier to guarantee the behaviour insteand of the concrete type.

[Russ Cox]

I think the net cleanup is still okay.

Russ

[Dave Cheney]

I tend to agree, if we don't already, we should have tests that asserts the errors returned by the net package comply with an interface, not as a specified type.

Offhand I  don't know if we do or do not have those tests.

--

[Dave Cheney]

Here is a data point, https://github.com/juju/juju/pull/2304/files

Given my stance on errors being opaque, I can't tell if this reinforces my point, or makes me look like a fool.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.

[brainman]

This bit me just yesterday https://go-review.googlesource.com/#/c/9942/4/src/net/net_windows_test.go (see body of new toErrno function is different from original code). I had to make change for a different reason, and, luckily, I discovered code that silently didn't work as expected.

I was going to argue against these net error changes. But then I looked at go1.4 net errors values - old errors are way broken (on windows). I cannot imagine many people use it. But it would be good if we could stop changing these in the future.

Alex

[minux]

error really is just a special case of the general problem changing the type

returned for an interface.

my understanding is that if the old type is exported, then it's part of the API.

otherwise it's impossible to really write Go 1 code that is future-proof.

(have any of you written code that depends on the concrete type of Sys

method of FileInfo returned from os.Stat? what if we change it to return

another type so that it could contain more information? see #9611)

the goal for the Go 1 API guarantee is to make sure that the code written

today can works with tomorrow's Go 1 release, if the code is correct. We

went through a great length to preserve that in the past (even if the old type

is completely broken, we still preserve it rather than fix it). now there are

multiple cases that the net changes have caused silent behavior change in

existing user code, I think we should re-evaluate this case.

I agree that for interfaces, the ideal way is to assert behavior, but that's not

how the error values from the net package works.

I think if we really want to do this change in Go 1.5, we should switch to

returning an unexported type and instead define getter methods; and we

need to modify go vet to warn against any code that will be silently broken

by the change (it's even better if we can write go fix rules to fix those code,

but I imagine that pretty hard.)