fix(errors): extract API error body from GenericOpenAPIError

cmd/logs_simple.go

@@ -7,7 +7,6 @@ package cmd
 
 import (
 	"fmt"
-	"io"
 	"regexp"
 	"strings"
 	"time"
@@ -789,13 +788,16 @@ func runLogsSearch(cmd *cobra.Command, args []string) error {
 	// Fetch first page
 	resp, r, err := api.ListLogs(client.Context(), opts)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		// These inline error handlers use extractAPIErrorBody directly instead of
+		// formatAPIError because they include domain-specific request details and
+		// troubleshooting context that the centralized helper does not support.
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				fromTimeObj := time.UnixMilli(fromTime).UTC()
 				toTimeObj := time.UnixMilli(toTime).UTC()
 				return fmt.Errorf("failed to search logs: %w\nStatus: %d\nAPI Response: %s\n\nRequest Details:\n- Query: %s\n- From: %s UTC (parsed from: %s)\n- To: %s UTC (parsed from: %s)\n- Limit: %d\n\nTroubleshooting:\n- Verify your time range is valid\n- Check that your query syntax is correct\n- Ensure you have proper permissions",
-					err, r.StatusCode, string(bodyBytes),
+					err, r.StatusCode, apiBody,
 					logsQuery,
 					fromTimeObj.Format(time.RFC3339), logsFrom,
 					toTimeObj.Format(time.RFC3339), logsTo,
@@ -926,10 +928,10 @@ func runLogsList(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.ListLogs(client.Context(), opts)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
-				return fmt.Errorf("failed to list logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, string(bodyBytes))
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
+				return fmt.Errorf("failed to list logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, apiBody)
 			}
 			return fmt.Errorf("failed to list logs: %w (status: %d)", err, r.StatusCode)
 		}
@@ -998,10 +1000,10 @@ func runLogsQuery(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.ListLogs(client.Context(), opts)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
-				return fmt.Errorf("failed to query logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, string(bodyBytes))
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
+				return fmt.Errorf("failed to query logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, apiBody)
 			}
 			return fmt.Errorf("failed to query logs: %w (status: %d)", err, r.StatusCode)
 		}
@@ -1088,13 +1090,13 @@ func runLogsAggregate(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.AggregateLogs(client.Context(), body)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				fromTimeObj := time.UnixMilli(fromTime).UTC()
 				toTimeObj := time.UnixMilli(toTime).UTC()
 				return fmt.Errorf("failed to aggregate logs: %w\nStatus: %d\nAPI Response: %s\n\nRequest Details:\n- Query: %s\n- Compute: %s (parsed as: aggregation=%q, metric=%q)\n- Group By: %s\n- From: %s UTC (parsed from: %s)\n- To: %s UTC (parsed from: %s)\n- Limit: %d\n\nTroubleshooting:\n- Verify the aggregation function is supported\n- Ensure the metric field exists in your logs (e.g., @duration, @bytes)\n- Check your query syntax\n- Verify your time range is valid",
-					err, r.StatusCode, string(bodyBytes),
+					err, r.StatusCode, apiBody,
 					logsQuery,
 					logsCompute, aggregation, metric,
 					logsGroupBy,

cmd/metrics.go

@@ -7,7 +7,6 @@ package cmd
 
 import (
 	"fmt"
-	"io"
 	"strconv"
 	"strings"
 	"time"
@@ -559,11 +558,11 @@ func runMetricsQuery(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.QueryTimeseriesData(client.Context(), body)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				return fmt.Errorf("failed to query metrics: %w\nStatus: %d\nAPI Response: %s\n\nRequest Details:\n- Query: %s\n- From: %s (Unix: %d)\n- To: %s (Unix: %d)\n\nTroubleshooting:\n- Verify your query syntax is correct (e.g., avg:metric.name{filter})\n- Check that the time range is valid\n- Ensure the metric exists and has data in the specified time range\n- Confirm you have proper permissions to access the metric",
-					err, r.StatusCode, string(bodyBytes),
+					err, r.StatusCode, apiBody,
 					queryString,
 					from.Format(time.RFC3339), from.Unix(),
 					to.Format(time.RFC3339), to.Unix())
@@ -604,11 +603,11 @@ func runMetricsSearch(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.QueryMetrics(client.Context(), from.Unix(), to.Unix(), queryString)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				return fmt.Errorf("failed to search metrics: %w\nStatus: %d\nAPI Response: %s",
-					err, r.StatusCode, string(bodyBytes))
+					err, r.StatusCode, apiBody)
 			}
 			return fmt.Errorf("failed to search metrics: %w (status: %d)", err, r.StatusCode)
 		}
@@ -643,11 +642,11 @@ func runMetricsList(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.ListActiveMetrics(client.Context(), from, *opts)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				return fmt.Errorf("failed to list metrics: %w\nStatus: %d\nAPI Response: %s\n\nRequest Details:\n- Filter: %s\n- From: %s (Unix: %d)\n\nTroubleshooting:\n- Check that your filter pattern is valid\n- Verify you have permissions to list metrics",
-					err, r.StatusCode, string(bodyBytes),
+					err, r.StatusCode, apiBody,
 					filterPattern,
 					time.Unix(from, 0).Format(time.RFC3339), from)
 			}
@@ -677,11 +676,11 @@ func runMetricsMetadataGet(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.GetMetricMetadata(client.Context(), metricName)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				return fmt.Errorf("failed to get metric metadata: %w\nStatus: %d\nAPI Response: %s\n\nMetric: %s\n\nTroubleshooting:\n- Verify the metric name is correct\n- Ensure the metric exists in your account\n- Check that you have permissions to view metadata",
-					err, r.StatusCode, string(bodyBytes), metricName)
+					err, r.StatusCode, apiBody, metricName)
 			}
 			return fmt.Errorf("failed to get metric metadata: %w (status: %d)", err, r.StatusCode)
 		}
@@ -733,11 +732,11 @@ func runMetricsMetadataUpdate(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.UpdateMetricMetadata(client.Context(), metricName, body)
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				return fmt.Errorf("failed to update metric metadata: %w\nStatus: %d\nAPI Response: %s\n\nMetric: %s\n\nTroubleshooting:\n- Verify the metric name is correct\n- Check that the metadata values are valid (unit, type, etc.)\n- Ensure you have permissions to update metadata",
-					err, r.StatusCode, string(bodyBytes), metricName)
+					err, r.StatusCode, apiBody, metricName)
 			}
 			return fmt.Errorf("failed to update metric metadata: %w (status: %d)", err, r.StatusCode)
 		}
@@ -833,11 +832,11 @@ func runMetricsSubmit(cmd *cobra.Command, args []string) error {
 
 	resp, r, err := api.SubmitMetrics(client.Context(), body, *datadogV2.NewSubmitMetricsOptionalParameters())
 	if err != nil {
-		if r != nil && r.Body != nil {
-			bodyBytes, readErr := io.ReadAll(r.Body)
-			if readErr == nil && len(bodyBytes) > 0 {
+		if r != nil {
+			apiBody := extractAPIErrorBody(err)
+			if apiBody != "" {
 				return fmt.Errorf("failed to submit metrics: %w\nStatus: %d\nAPI Response: %s\n\nRequest Details:\n- Metric: %s\n- Value: %f\n- Type: %s\n- Timestamp: %d\n- Tags: %v\n\nTroubleshooting:\n- Verify the metric name follows naming conventions (lowercase, dots/underscores)\n- Check that the metric type is valid (gauge, count, rate)\n- Ensure your API key has permission to submit metrics\n- Verify tags are in key:value format",
-					err, r.StatusCode, string(bodyBytes),
+					err, r.StatusCode, apiBody,
 					submitName, submitValue, submitType, timestamp, tags)
 			}
 			return fmt.Errorf("failed to submit metrics: %w (status: %d)", err, r.StatusCode)

cmd/root.go

@@ -7,11 +7,13 @@ package cmd
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"strings"
 
+	"github.com/DataDog/datadog-api-client-go/v2/api/datadog"
 	"github.com/DataDog/pup/internal/version"
 	"github.com/DataDog/pup/pkg/client"
 	"github.com/DataDog/pup/pkg/config"
@@ -259,7 +261,26 @@ func readConfirmation() (string, error) {
 	return "", scanner.Err()
 }
 
-// formatAPIError creates user-friendly error messages for API errors
+// extractAPIErrorBody extracts the API response body from a
+// datadog.GenericOpenAPIError. The datadog-api-client-go library consumes
+// http.Response.Body during deserialization and stores the bytes in the error.
+// Callers that try to re-read http.Response.Body will always get empty data.
+func extractAPIErrorBody(err error) string {
+	if err == nil {
+		return ""
+	}
+	var apiErr datadog.GenericOpenAPIError
+	if errors.As(err, &apiErr) {
+		if body := apiErr.Body(); len(body) > 0 {
+			return string(body)
+		}
+	}
+	return ""
+}
+
+// formatAPIError creates user-friendly error messages for API errors.
+// It extracts the API response body from GenericOpenAPIError when available
+// and appends contextual guidance based on the HTTP status code.
 func formatAPIError(operation string, err error, response any) error {
 	type httpResponse interface {
 		StatusCode() int
@@ -269,6 +290,11 @@ func formatAPIError(operation string, err error, response any) error {
 		statusCode := r.StatusCode()
 		baseMsg := fmt.Sprintf("failed to %s: %v (status: %d)", operation, err, statusCode)
 
+		// Include API response body if available
+		if body := extractAPIErrorBody(err); body != "" {
+			baseMsg = fmt.Sprintf("failed to %s: %v (status: %d)\nAPI Response: %s", operation, err, statusCode, body)
+		}
+
 		switch {
 		case statusCode >= 500:
 			// 5xx Server errors

cmd/root_test.go

@@ -7,9 +7,11 @@ package cmd
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
+	"github.com/DataDog/datadog-api-client-go/v2/api/datadog"
 	"github.com/DataDog/pup/pkg/config"
 )
 
@@ -209,7 +211,7 @@ func TestFormatAPIError_AllStatusCodes(t *testing.T) {
 	}
 
 	for _, tt := range statusTests {
-		t.Run(string(rune(tt.code)), func(t *testing.T) {
+		t.Run(fmt.Sprintf("%d", tt.code), func(t *testing.T) {
 			err := formatAPIError("test operation", errors.New("test error"), &mockHTTPResponse{statusCode: tt.code})
 
 			if err == nil {
@@ -318,3 +320,83 @@ func TestTestCmd_InvalidSite(t *testing.T) {
 		t.Errorf("testCmd.RunE() error should mention DD_SITE, got: %v", err)
 	}
 }
+
+func TestExtractAPIErrorBody(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{
+			name: "GenericOpenAPIError with body",
+			err: datadog.GenericOpenAPIError{
+				ErrorBody:    []byte(`{"errors":["Invalid query: avg:nonexistent.metric{*}"]}`),
+				ErrorMessage: "400 Bad Request",
+			},
+			want: `{"errors":["Invalid query: avg:nonexistent.metric{*}"]}`,
+		},
+		{
+			name: "GenericOpenAPIError with empty body",
+			err: datadog.GenericOpenAPIError{
+				ErrorBody:    []byte{},
+				ErrorMessage: "400 Bad Request",
+			},
+			want: "",
+		},
+		{
+			name: "GenericOpenAPIError with nil body",
+			err: datadog.GenericOpenAPIError{
+				ErrorBody:    nil,
+				ErrorMessage: "400 Bad Request",
+			},
+			want: "",
+		},
+		{
+			name: "wrapped GenericOpenAPIError",
+			err: fmt.Errorf("api call failed: %w", datadog.GenericOpenAPIError{
+				ErrorBody:    []byte(`{"errors":["bad query"]}`),
+				ErrorMessage: "400 Bad Request",
+			}),
+			want: `{"errors":["bad query"]}`,
+		},
+		{
+			name: "non-GenericOpenAPIError",
+			err:  errors.New("some other error"),
+			want: "",
+		},
+		{
+			name: "nil error",
+			err:  nil,
+			want: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := extractAPIErrorBody(tt.err)
+			if got != tt.want {
+				t.Errorf("extractAPIErrorBody() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestFormatAPIError_IncludesResponseBody(t *testing.T) {
+	// This test verifies that formatAPIError surfaces the API response body
+	// from GenericOpenAPIError, which was previously lost because the code
+	// tried to re-read the already-consumed http.Response.Body.
+	apiErr := datadog.GenericOpenAPIError{
+		ErrorBody:    []byte(`{"errors":["Query parse error: unknown metric"]}`),
+		ErrorMessage: "400 Bad Request",
+	}
+
+	err := formatAPIError("query metrics", apiErr, &mockHTTPResponse{statusCode: 400})
+	errMsg := err.Error()
+
+	if !strings.Contains(errMsg, "unknown metric") {
+		t.Errorf("formatAPIError() should include API response body, got: %q", errMsg)
+	}
+	if !strings.Contains(errMsg, "status: 400") {
+		t.Errorf("formatAPIError() should include status code, got: %q", errMsg)
+	}
+}