fix: restore req body #397
Conversation
|
Your Render PR Server URL is https://chproxy-pr-397.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cmv26nv109ks739iu2t0. |
proxy.go
Outdated
| // update body | ||
| rp(rw, req) | ||
|
|
||
| // Restore req.Body after it's consumed by 'rp' for potential reuse. | ||
| req.Body = io.NopCloser(bytes.NewBuffer(body)) | ||
| req.Body.Close() |
There was a problem hiding this comment.
why do you have a call to Close() here and there? https://github.com/ContentSquare/chproxy/pull/397/files#diff-8b7f25bac2ae04c2ce2abb628c6aa28103fd113103a62f3e9d232fa995684915R111
I don't know what should be done, but it doesn't look logic to have inconsistencies for the same use case.
There was a problem hiding this comment.
There was a problem hiding this comment.
You are right. This one was at the beginning. I'll check if can be removed
There was a problem hiding this comment.
req.Body.Close() appears in few places. Maybe, better to refactor it in a separate PR?
I'll try |
sigua-cs
force-pushed
the
issue/336_restore-req-body
branch
from
03bae19 to
1698f40
Compare
proxy_test.go
Outdated
| } | ||
|
|
||
| //func TestRequestBody(t *testing.T) { |
There was a problem hiding this comment.
Can you remove the commented code?
sigua-cs
force-pushed
the
issue/336_restore-req-body
branch
6 times, most recently
from
d5d10c2 to
245d3ea
Compare
sigua-cs
force-pushed
the
issue/336_restore-req-body
branch
from
245d3ea to
11de1a4
Compare
Description
Fix restore original req.Body
Pull request type
implements #336
Please check the type of change your PR introduces:
Checklist
Does this introduce a breaking change?
Further comments