Skip to content

feat: implement io.ReaderFrom in wrapped response#73

Merged
samber merged 1 commit into
mainfrom
feat/implements-readerfrom
Feb 2, 2026
Merged

feat: implement io.ReaderFrom in wrapped response#73
samber merged 1 commit into
mainfrom
feat/implements-readerfrom

Conversation

@samber
Copy link
Copy Markdown
Owner

@samber samber commented Feb 2, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 2, 2026 16:48
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 2, 2026

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (d76cea2) to head (3a34ec5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
dump.go 0.00% 7 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main     #73   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          4       4           
  Lines        365     372    +7     
=====================================
- Misses       365     372    +7     
Flag Coverage Δ
unittests 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samber samber merged commit 346894c into main Feb 2, 2026
17 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds io.ReaderFrom support to the wrapped gin.ResponseWriter (bodyWriter) so io.Copy-style response writes can take the optimized ReadFrom path while still tracking response byte counts.

Changes:

  • Assert bodyWriter implements io.ReaderFrom.
  • Add ReadFrom(io.Reader) (int64, error) implementation to bodyWriter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dump.go
return n, err
}
}
return io.Copy(w, r)
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadFrom calls io.Copy(w, r) in the fallback path, but since bodyWriter now implements io.ReaderFrom, io.Copy will call ReadFrom again and recurse indefinitely (e.g., when w.body != nil or when the underlying ResponseWriter does not implement io.ReaderFrom). Implement the copy loop manually, or call io.Copy on a wrapper type that only exposes io.Writer to avoid the ReaderFrom fast-path.

Suggested change
return io.Copy(w, r)
var written int64
buf := make([]byte, 32*1024)
for {
nr, er := r.Read(buf)
if nr > 0 {
nw, ew := w.Write(buf[:nr])
if nw > 0 {
written += int64(nw)
}
if ew != nil {
return written, ew
}
if nw != nr {
return written, io.ErrShortWrite
}
}
if er != nil {
if er == io.EOF {
break
}
return written, er
}
}
return written, nil

Copilot uses AI. Check for mistakes.
Comment thread dump.go
Comment on lines +44 to +46
n, err := rf.ReadFrom(r)
w.bytes += int(n)
return n, err
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

w.bytes += int(n) converts the int64 byte count returned by ReadFrom to int, which can overflow on large responses (and silently wrap). Consider tracking byte counts as int64 throughout (bodyWriter.bytes, bodyReader.bytes) and logging with slog.Int64, or clamp the value before converting.

Copilot uses AI. Check for mistakes.
@samber samber deleted the feat/implements-readerfrom branch February 6, 2026 02:08
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