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>

Author: sean-

CHANGELOG.md

@@ -227,7 +227,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Fix `opensearchtransport.Transport.Perform` silently dropping `io.ReadAll` errors during response buffering via `:=` shadowing; the read error now propagates wrapped in the new `opensearchtransport.ErrResponseBodyRead` sentinel, and `opensearch.Client.Do` classifies the `(resp != nil, err != nil)` case via `errors.Is` so only genuine body-read failures are labeled `ErrReadBody` (an unrelated transport error returned alongside a response, such as context cancellation during retry backoff, is no longer misreported as a read failure). As a consequence, `opensearch.Client.Do` now returns a non-nil `*Response` alongside a non-nil error in this case where it previously returned `(nil, err)`; callers detecting a hard transport failure should check `resp == nil` rather than `err != nil` ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
 - Fix error-response body not being closed in `opensearch.ParseError`. `ParseError` now closes the original body before re-wrapping the read bytes in a `NopCloser`. The generated `opensearchapi` `do()` no-decode error path no longer needs its own drain: `opensearch.Do` routes through the buffered `opensearchtransport.Transport.Perform`, so the returned `resp.Body` is already an in-memory `NopCloser` over the full payload and stays readable for the caller ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
 - Fix response-body lifecycle on the raw `RoundTrip` paths that lack `Perform`'s buffering safety net, where closing a partially-read body defeated HTTP keep-alive: the stats poller (`cluster_health.go`), discovery's `/_cat/shards`, `/_cluster/state/metadata`, and `/_nodes` paths, and the `fetchClusterHealth`/`baselineHealthCheck`/`hardwareInfoHealthCheck` pollers now drain to EOF (`io.Copy(io.Discard, ...)`) before close (covering both non-200 returns and `json.Decode` success paths that stop before EOF). The AWS v1 and v2 signers now close the request body on the read-error path in `hexEncodedSha256OfRequest` ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
-- Fix `Client.Do` not buffering error-response bodies, which let a value-receiver `Response.String()` (e.g. `log.Printf("%s", resp)`) drain the single-use error `Body` and leave a subsequent `ParseError` reading an empty payload (surfacing `ErrJSONUnmarshalBody` instead of the real API error). `Do` now buffers error responses into `rawBody` like success responses, so `String()` renders from `rawBody` without touching `Body` and `ParseError` reads an intact body ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
+- Fix `Client.Do` to buffer every response body into `rawBody` -- decoded success, error, and no-decode (nil `dataPointer`) success alike (previously some paths, including the nil-`dataPointer` success path, were left unbuffered). Without this, a value-receiver `Response.String()` (e.g. `log.Printf("%s", resp)`) drained the single-use `Body` and left a subsequent `ParseError` reading an empty payload (surfacing `ErrJSONUnmarshalBody` instead of the real API error). Now `String()` renders from `rawBody` without touching `Body` and `ParseError` reads an intact body ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
 - Add typed response-format defaults for generated `opensearchapi/` cat, list, ppl, and sql operations: when the caller leaves `Format` unset, the SDK now emits the value the typed Resp struct expects (`json` for cat/list/explain, `jdbc` for ppl/sql query) instead of letting the server fall back to a default the JSON decoder cannot handle.
 - Replace `WaitForAllNodesReady` inline `require.Eventually` loop with a layered readiness FSM (`internal/test/readiness`) that observes per-node progression through `LayerTCP -> LayerHTTP -> LayerClusterJoin -> LayerStatsReady`, records transitions including regressions, and emits a structured per-node diagnostic with the full last cat-nodes response on timeout. Per-layer budgets are tuned for CI pessimism (cold JVM startup is the long pole); total budget for `TargetClusterReady` is 6.5 minutes. ([#650](https://github.com/opensearch-project/opensearch-go/issues/650))
 - Fix bulk indexer HTML-escaping `_id` and `routing` values containing `<`, `>`, or `&` characters, causing OpenSearch to store escaped values (e.g., `\u003croot_account\u003e` stored instead of `<root_account>`), leading to duplicate documents, unreachable data on read-by-ID paths, and potential shard routing mismatches. Present since the `json.Marshal` migration in 2021 (commit `3da59092`). Replace `json.Marshal` with `json.NewEncoder` + `SetEscapeHTML(false)` in `opensearchutil.worker.writeMeta` and `opensearchutil.JSONReader`; replace per-worker `aux []byte` with `sync.Pool`-backed `*bytes.Buffer`; add table-driven test coverage for `writeMeta` edge cases and refactor remaining `TestBulkIndexer` subtests to table-driven `require`-based style ([#824](https://github.com/opensearch-project/opensearch-go/pull/824))

opensearch.go

@@ -529,35 +529,28 @@ func (c *Client) Do(ctx context.Context, method string, req Request, dataPointer
 		return response, fmt.Errorf("status: %d, err: %w", resp.StatusCode, err)
 	}
 
-	if dataPointer != nil && resp.Body != nil && !response.IsError() {
-		data, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return response, fmt.Errorf("%w, status: %d, err: %w", ErrReadBody, resp.StatusCode, err)
-		}
-
-		response.rawBody = data
-		response.Body = io.NopCloser(bytes.NewReader(data))
-
-		if err := json.Unmarshal(data, dataPointer); err != nil {
-			return response, fmt.Errorf("%w, status: %d, body: %s, err: %w", ErrJSONUnmarshalBody, resp.StatusCode, data, err)
-		}
-	}
-
-	if resp.Body != nil && response.IsError() {
-		// Buffer error-response bodies into rawBody so String renders via the
-		// rawBody fast-path (never draining Body) and a subsequent ParseError
-		// still reads an intact Body. Without this, a value-receiver String
-		// call (e.g. log.Printf("%s", resp)) would consume the single-use
-		// error Body and leave ParseError with an empty payload. In the
-		// default buffered mode Perform already returned an in-memory
-		// NopCloser, so this just copies bytes already resident in memory.
+	if resp.Body != nil {
+		// Buffer the response payload into rawBody for every Do response --
+		// success, error, and no-decode (nil dataPointer) alike -- so the
+		// value-receiver String renders via the rawBody fast-path and never
+		// drains Body, and a subsequent ParseError still reads an intact Body.
+		// Without this, a String call (e.g. log.Printf("%s", resp)) would
+		// consume the single-use Body. In the default buffered mode Perform
+		// already returned an in-memory NopCloser, so this just copies bytes
+		// already resident in memory.
 		data, rerr := io.ReadAll(resp.Body)
 		if rerr != nil {
 			return response, fmt.Errorf("%w, status: %d, err: %w", ErrReadBody, resp.StatusCode, rerr)
 		}
 
 		response.rawBody = data
 		response.Body = io.NopCloser(bytes.NewReader(data))
+
+		if dataPointer != nil && !response.IsError() {
+			if err := json.Unmarshal(data, dataPointer); err != nil {
+				return response, fmt.Errorf("%w, status: %d, body: %s, err: %w", ErrJSONUnmarshalBody, resp.StatusCode, data, err)
+			}
+		}
 	}
 
 	return response, nil

response.go

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