Skip to content

Commit f8072b5

Browse files
committed
fix(response): buffer Do response body for every response
Unify the body-buffering paths in Client.Do so the response payload is read into rawBody for every response -- success, error, and no-decode (nil dataPointer) alike -- instead of only for decoded-success and error responses. This makes the value-receiver String() render via the rawBody fast-path in all cases (never draining Body), and keeps a subsequent ParseError able to read an intact Body. Update the RawBody and String doc comments on Response to match: rawBody is populated for every Do response and is nil only for a hand-built Response or one obtained through the unbuffered Stream path. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 08206ed commit f8072b5

2 files changed

Lines changed: 26 additions & 36 deletions

File tree

opensearch.go

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -529,35 +529,28 @@ func (c *Client) Do(ctx context.Context, method string, req Request, dataPointer
529529
return response, fmt.Errorf("status: %d, err: %w", resp.StatusCode, err)
530530
}
531531

532-
if dataPointer != nil && resp.Body != nil && !response.IsError() {
533-
data, err := io.ReadAll(resp.Body)
534-
if err != nil {
535-
return response, fmt.Errorf("%w, status: %d, err: %w", ErrReadBody, resp.StatusCode, err)
536-
}
537-
538-
response.rawBody = data
539-
response.Body = io.NopCloser(bytes.NewReader(data))
540-
541-
if err := json.Unmarshal(data, dataPointer); err != nil {
542-
return response, fmt.Errorf("%w, status: %d, body: %s, err: %w", ErrJSONUnmarshalBody, resp.StatusCode, data, err)
543-
}
544-
}
545-
546-
if resp.Body != nil && response.IsError() {
547-
// Buffer error-response bodies into rawBody so String renders via the
548-
// rawBody fast-path (never draining Body) and a subsequent ParseError
549-
// still reads an intact Body. Without this, a value-receiver String
550-
// call (e.g. log.Printf("%s", resp)) would consume the single-use
551-
// error Body and leave ParseError with an empty payload. In the
552-
// default buffered mode Perform already returned an in-memory
553-
// NopCloser, so this just copies bytes already resident in memory.
532+
if resp.Body != nil {
533+
// Buffer the response payload into rawBody for every Do response --
534+
// success, error, and no-decode (nil dataPointer) alike -- so the
535+
// value-receiver String renders via the rawBody fast-path and never
536+
// drains Body, and a subsequent ParseError still reads an intact Body.
537+
// Without this, a String call (e.g. log.Printf("%s", resp)) would
538+
// consume the single-use Body. In the default buffered mode Perform
539+
// already returned an in-memory NopCloser, so this just copies bytes
540+
// already resident in memory.
554541
data, rerr := io.ReadAll(resp.Body)
555542
if rerr != nil {
556543
return response, fmt.Errorf("%w, status: %d, err: %w", ErrReadBody, resp.StatusCode, rerr)
557544
}
558545

559546
response.rawBody = data
560547
response.Body = io.NopCloser(bytes.NewReader(data))
548+
549+
if dataPointer != nil && !response.IsError() {
550+
if err := json.Unmarshal(data, dataPointer); err != nil {
551+
return response, fmt.Errorf("%w, status: %d, body: %s, err: %w", ErrJSONUnmarshalBody, resp.StatusCode, data, err)
552+
}
553+
}
561554
}
562555

563556
return response, nil

response.go

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,10 @@ type renderCache struct {
6161
// handling: copy with bytes.Clone if either is needed, or use
6262
// HijackBody to transfer ownership to the caller.
6363
//
64-
// Populated for both success and error responses buffered by Client.Do
65-
// (the default buffered mode). Returns nil when:
66-
//
67-
// - The response was streamed without buffering (DisableResponseBuffering).
68-
// - The Response was constructed directly (e.g. via NewResponse) rather
69-
// than returned by Client.Do.
64+
// Populated for every response returned by Client.Do -- success, error, and
65+
// no-decode (nil dataPointer) alike. Returns nil only when the Response was
66+
// constructed directly (e.g. via NewResponse) or obtained through the
67+
// unbuffered Client.Stream path rather than Client.Do.
7068
func (r *Response) RawBody() []byte {
7169
return r.rawBody
7270
}
@@ -96,14 +94,13 @@ func NewResponse(statusCode int, body io.ReadCloser, header http.Header) *Respon
9694
// String returns the response status and body as a string.
9795
//
9896
// String uses a value receiver, so both Response and *Response satisfy
99-
// fmt.Stringer. For any Response returned by Client.Do (success or error, in
100-
// the default buffered mode) it renders from the buffered rawBody and never
101-
// touches Body, so logging a response does not drain it. For a Response
102-
// holding only an unbuffered Body -- a streamed response
103-
// (DisableResponseBuffering) or a hand-built Response -- String reads Body
104-
// once to render it; repeat calls stay consistent via an internal cache, but
105-
// a value receiver cannot restore the caller's Body field, so that single-use
106-
// stream is consumed.
97+
// fmt.Stringer. For any Response returned by Client.Do it renders from the
98+
// buffered rawBody and never touches Body, so logging a response does not
99+
// drain it. For a Response holding only an unbuffered Body -- one obtained via
100+
// Client.Stream or constructed by hand -- String reads Body once to render it;
101+
// repeat calls stay consistent via an internal cache, but a value receiver
102+
// cannot restore the caller's Body field, so that single-use stream is
103+
// consumed.
107104
func (r Response) String() string {
108105
body, rerr, ok := r.renderedBody()
109106
if !ok {

0 commit comments

Comments
 (0)