Skip to content

ReadFrom: keep the bytes returned together with a read error - #33

Merged
smallnest merged 1 commit into
smallnest:masterfrom
youdie006:readfrom-keep-bytes-on-error
Sep 28, 2026
Merged

smallnest merged 1 commit into
smallnest:masterfrom
youdie006:readfrom-keep-bytes-on-error

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

io.Reader may return n > 0 together with a non-EOF error, and the caller should use those bytes before looking at the error. ReadFrom sets the error and breaks out without committing them or adding them to n (ring_buffer.go:553). So io.Copy(rb, src) from a source that ends with io.ErrUnexpectedEOF (a truncated HTTP body or flate stream) reports fewer bytes than it read, and the last chunk is lost. WriteTo already counts partial bytes on error since #20.

This commits and counts nr before setting the error. The errormock reader now returns its bytes together with the error, the way its Write already does, which makes three rows of TestRingBuffer_ReadFrom_Error fail on master; TestRingBuffer_ReadFrom_ErrorKeepsData also checks Length().

go vet, gofmt -s -l and go test -race ./... pass. One note on the last: TestRingBuffer_Copy hit its 60s timeout on my machine under heavy load, on master as well (3 of 3 isolated -race runs took 68-70s); its stream only returns nil or io.EOF, so it never reaches this branch. With it skipped the -race suite passes.

Written with AI assistance (Claude); I ran the checks above locally and reviewed the change.

io.Reader may return n > 0 bytes along with a non-EOF error, and those bytes
should be processed before the error. ReadFrom set the error and broke out
without committing them or counting them in n, so io.Copy into a ring buffer
from a source ending in io.ErrUnexpectedEOF under-reported what it took and
dropped the last chunk. WriteTo already counts partial bytes on error (smallnest#20).
@smallnest
smallnest merged commit a3c7188 into smallnest:master Sep 28, 2026
3 checks passed

This branch was successfully deployed

1 active deployment
ci — ac409923 Deployed Sep 27, 2026 by youdie006 via test #24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants