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

io: document that ReadAtLeast and ReadFull can drop errors #20477

Closed
akalin opened this Issue May 24, 2017 · 5 comments

Comments

Projects
None yet
6 participants
@akalin
Copy link

akalin commented May 24, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.8.2

What operating system and processor architecture are you using (go env)?

darwin amd64

What did you do?

When passing an io.Reader to io.ReadAtLeast or io.ReadFull, if the conditions of the function are fulfilled (i.e., enough bytes are returned) but it also returns an error, I would expect that error to be propagated. Instead, it is dropped. Example:

https://play.golang.org/p/3XvnMqGYMr

The behavior of these functions can't be changed because of backwards compatibility. I would argue that the function comment should call this case out specifically, since it's quite surprising (and even led to a bug in encoding/base32), and it seems that callers do want the error-propagating behavior most of the time.

Split off from #20044 .

@niubaoshu

This comment has been minimized.

Copy link

niubaoshu commented May 24, 2017

io ReadAtLeast doc
// It returns the number of bytes copied and an error if fewer bytes were read.

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone May 24, 2017

@akalin

This comment has been minimized.

Copy link
Author

akalin commented May 24, 2017

That part doesn't say anything about the error returned if enough bytes are read, but later in the comment:

On return, n >= min if and only if err == nil.

I think this could be clarified to:

On return, n >= min if and only if err == nil. In particular, if r.Read() returns an error but fills up buf, that error will get dropped.

@dsnet dsnet changed the title io: ReadAtLeast and ReadFull sometimes drop errors io: document that ReadAtLeast and ReadFull can drop errors May 24, 2017

@dsnet dsnet added the Documentation label May 24, 2017

@dsnet

This comment has been minimized.

Copy link
Member

dsnet commented May 24, 2017

The persistence of an error in a io.Reader is not specified in the documentation and it is reasonable that there is a class of errors that are transient and do not cause an issue if Read were called again. Furthermore, there's nothing that you can do in ReadAtLeast since it is a stateless function and cannot affect the stickiness of an error on the io.Reader passed in.

This is only relevant to documentation; documentation fixes are okay for Go1.9, but leaving in Go1.10 milestone.

@akalin

This comment has been minimized.

Copy link
Author

akalin commented May 25, 2017

You're right about persistence of errors in io.Reader, but that's not what I'm referring to here. The loop in ReadAtLeast stops as soon as r returns an error, but as long as enough bytes are read, that error is dropped. That's the behavior that I'm making a case for being documented.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Mar 25, 2018

Change https://golang.org/cl/102459 mentions this issue: io: document that ReadAtLeast and ReadFull can drop errors

@gopherbot gopherbot closed this in 665af04 Mar 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.