io.Copy has to discard at least one error here. And it's debatable
whether to discard the read error or the write error.

If we change it to return the Read error, then I think it shouldn't issue
the Write call at all. Because the io.Copy has already failed.
However, on the other hand, we all know that's not the correct way
of dealing with the result from io.Reader.

We could also interpret the docs in a different way. Because it's
copying between dst and src, the Read and Write operation should
always appear in pair if the n returned from Read is non-zero (the
Read did succeed in some sense), and the first error is indeed
coming from Write.

BTW, could network connections return (n, err) with n > 0 and err != nil?
I think the underlying read syscall doesn't expose this capability.

io.Copy has to discard at least one error here. And it's debatable
whether to discard the read error or the write error.

Technically, it doesn't necessarily have to discard one of the errors; returning an error that holds both underlying errors could be an option too, but it is probably not worth the added complexity. So I agree that it is reasonable to return just one of the errors in this situation.

If we change it to return the Read error, then I think it shouldn't issue
the Write call at all. Because the io.Copy has already failed.
However, on the other hand, we all know that's not the correct way
of dealing with the result from io.Reader.

This paragraph seems self-contradictory to me... I agree with the second half -- not issuing the Write call would be against the convention regarding how to handle io.Reader errors.

We could also interpret the docs in a different way. Because it's
copying between dst and src, the Read and Write operation should
always appear in pair if the n returned from Read is non-zero (the
Read did succeed in some sense), and the first error is indeed
coming from Write.

I see where you're coming from; it makes sense if "first error" is interpreted as in byte positions. E.g. with read error at byte index 1024, write error at byte index 512, the write error can be considered "first" even if the read error occurred earlier. The question is, first, whether that was the intent, and second, what people reading the docs think.

For the first question, it may as well be the intent, and this way the error is actually correlated to the position (number of bytes successfully copied) that the error is returned together with.

For the second question, I can only speak for myself, but it didn't occur to me that "first error encountered" could refer to something other than temporal/"happens-before" first... Given the current docs (and the established Go convention to process any "residual" data received together with a read error), my interpretation was that if I get a write error, I could assume that there was no (read) error prior to that. On the other hand, if I get a read error, I couldn't assume anything about what happened afterwards. Although I have to add that English is not my native language (but then, the same is true for a large portion of people reading the docs).

Assuming the behavior is intended, I'm thinking that there should be a better way to put it that is clearer to a wider range of users, though I haven't managed come up with a good one -- I'll keep thinking.

BTW, could network connections return (n, err) with n > 0 and err != nil?
I think the underlying read syscall doesn't expose this capability.

You're right that syscalls don't directly expose this capability; I was assuming the case when using a buffering reader implementation that could aggregate results from multiple syscalls.

It all depends on the meaning of "the first error".

If you think it means the read, then it shouldn't continue to issue
the Write even if n > 0, because the Copy call is guaranteed to
fail.

On the other hand, if you regard that a Read that returns (n, err)
with n > 0 and err != nil as successful, then the current behavior
is correct.

Making io.Copy return a new Error struct combining the two errors
is an API change that might break existing code.

There are more corner cases in the design of io.Copy (for example,
if the Write fails due to some flakiness in underlying writer, the client
might want to retry, so it is desirable to not discard the unwritten
bytes.) If you want more control, I think you should design your own
copy routine.

It all depends on the meaning of "the first error".

Yes, and my opinion is that it's quite a bit of stretch to call the error from Write the first error in this scenario, and that it is not what most people reading the docs would think.

The way I interpret the docs is that the first error is returned (which in this situation would be the one from Read), and there may be further errors (e.g. from a subsequent Write) but those will be discarded.

If you think it means the read, then it shouldn't continue to issue
the Write even if n > 0, because the Copy call is guaranteed to
fail.

I think it means the read, but as I've already said in my previous comment, I don't agree with this reasoning; not issuing the Write is contrary to the established Go convention, that is, contrary to what people reading the docs should be expecting.

On the other hand, if you regard that a Read that returns (n, err)
with n > 0 and err != nil as successful, then the current behavior
is correct.

I'd prefer not to introduce new terms ("successful"); we have enough confusion with the existing ones already. :) My primary issue is not which behavior is "correct"; it is that it's not doing what (I think) the docs say.

Making io.Copy return a new Error struct combining the two errors
is an API change that might break existing code.

I'm aware of that, and I didn't mean to suggest changing it that way. I just pointed it out to emphasize that discarding some errors is not "inevitable", but is a result of a conscious design choice, favoring simplicity over "correctness"/"consistency", which I agree with.

Applying the same rationale, even if the current implementation is deemed the more "correct" or "consistent" behavior, changing the implementation to match the docs (return Read error, discard any further) can be considered favorable to documenting the current behavior (which may require introducing new terminology such as "successful read").

In addition, in case the two errors share the same underlying cause, keeping the one that occurred first (in time) may be more helpful than the second.

There are more corner cases in the design of io.Copy (for example,
if the Write fails due to some flakiness in underlying writer, the client
might want to retry, so it is desirable to not discard the unwritten
bytes.) If you want more control, I think you should design your own
copy routine.

I don't want more control, I just want it to do what it says (or, say what it does).

TL;DR version:

• The current implementation is reasonable, but I think it is not what people reading the docs would expect.
• The behavior described in the current docs (in my interpretation) seems just as reasonable (if not more), but it is not what the function does.
• I think either docs or implementation should be changed to match the other, mildly favoring changing the implementation for the following reasons:
  1. The documented behavior is simpler (easier to understand and document)
  2. Existing users are more likely to expect the documented behavior (more likely to have read the docs than to have read the code.)
  3. In case the two errors share a common underlying cause, keeping the error that happened first (in time) may be more helpful than the second.

I'm in favor of document this corner case.
Changing the behavior will just add more confusion to the issue.

Right, I didn't consider that a behavior change would need to also involve ReaderFrom/WriterTo.
Sent https://golang.org/cl/3686/ .