Skip to content

Commit eb918bb

Browse files
maruinaclaude
andcommitted
fix(errors): extract API error body from GenericOpenAPIError
The datadog-api-client-go library consumes http.Response.Body during deserialization and stores the raw bytes in GenericOpenAPIError.ErrorBody. The error handling in metrics and logs commands was trying to re-read http.Response.Body via io.ReadAll, which always returned empty data since the body was already consumed. This silently discarded the actual API error details, making 400/4xx errors impossible to diagnose. - Add extractAPIErrorBody() helper that uses errors.As to extract the response body from GenericOpenAPIError - Replace all 10 broken io.ReadAll(r.Body) calls in cmd/metrics.go and cmd/logs_simple.go with the new helper - Enhance formatAPIError() to include the API response body when available - Add tests for extractAPIErrorBody and formatAPIError body inclusion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d7e927e commit eb918bb

4 files changed

Lines changed: 137 additions & 42 deletions

File tree

cmd/logs_simple.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package cmd
77

88
import (
99
"fmt"
10-
"io"
1110
"regexp"
1211
"strings"
1312
"time"
@@ -789,13 +788,13 @@ func runLogsSearch(cmd *cobra.Command, args []string) error {
789788
// Fetch first page
790789
resp, r, err := api.ListLogs(client.Context(), opts)
791790
if err != nil {
792-
if r != nil && r.Body != nil {
793-
bodyBytes, readErr := io.ReadAll(r.Body)
794-
if readErr == nil && len(bodyBytes) > 0 {
791+
if r != nil {
792+
apiBody := extractAPIErrorBody(err)
793+
if apiBody != "" {
795794
fromTimeObj := time.UnixMilli(fromTime).UTC()
796795
toTimeObj := time.UnixMilli(toTime).UTC()
797796
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",
798-
err, r.StatusCode, string(bodyBytes),
797+
err, r.StatusCode, apiBody,
799798
logsQuery,
800799
fromTimeObj.Format(time.RFC3339), logsFrom,
801800
toTimeObj.Format(time.RFC3339), logsTo,
@@ -926,10 +925,10 @@ func runLogsList(cmd *cobra.Command, args []string) error {
926925

927926
resp, r, err := api.ListLogs(client.Context(), opts)
928927
if err != nil {
929-
if r != nil && r.Body != nil {
930-
bodyBytes, readErr := io.ReadAll(r.Body)
931-
if readErr == nil && len(bodyBytes) > 0 {
932-
return fmt.Errorf("failed to list logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, string(bodyBytes))
928+
if r != nil {
929+
apiBody := extractAPIErrorBody(err)
930+
if apiBody != "" {
931+
return fmt.Errorf("failed to list logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, apiBody)
933932
}
934933
return fmt.Errorf("failed to list logs: %w (status: %d)", err, r.StatusCode)
935934
}
@@ -998,10 +997,10 @@ func runLogsQuery(cmd *cobra.Command, args []string) error {
998997

999998
resp, r, err := api.ListLogs(client.Context(), opts)
1000999
if err != nil {
1001-
if r != nil && r.Body != nil {
1002-
bodyBytes, readErr := io.ReadAll(r.Body)
1003-
if readErr == nil && len(bodyBytes) > 0 {
1004-
return fmt.Errorf("failed to query logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, string(bodyBytes))
1000+
if r != nil {
1001+
apiBody := extractAPIErrorBody(err)
1002+
if apiBody != "" {
1003+
return fmt.Errorf("failed to query logs: %w\nStatus: %d\nAPI Response: %s", err, r.StatusCode, apiBody)
10051004
}
10061005
return fmt.Errorf("failed to query logs: %w (status: %d)", err, r.StatusCode)
10071006
}
@@ -1088,13 +1087,13 @@ func runLogsAggregate(cmd *cobra.Command, args []string) error {
10881087

10891088
resp, r, err := api.AggregateLogs(client.Context(), body)
10901089
if err != nil {
1091-
if r != nil && r.Body != nil {
1092-
bodyBytes, readErr := io.ReadAll(r.Body)
1093-
if readErr == nil && len(bodyBytes) > 0 {
1090+
if r != nil {
1091+
apiBody := extractAPIErrorBody(err)
1092+
if apiBody != "" {
10941093
fromTimeObj := time.UnixMilli(fromTime).UTC()
10951094
toTimeObj := time.UnixMilli(toTime).UTC()
10961095
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",
1097-
err, r.StatusCode, string(bodyBytes),
1096+
err, r.StatusCode, apiBody,
10981097
logsQuery,
10991098
logsCompute, aggregation, metric,
11001099
logsGroupBy,

cmd/metrics.go

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package cmd
77

88
import (
99
"fmt"
10-
"io"
1110
"strconv"
1211
"strings"
1312
"time"
@@ -559,11 +558,11 @@ func runMetricsQuery(cmd *cobra.Command, args []string) error {
559558

560559
resp, r, err := api.QueryTimeseriesData(client.Context(), body)
561560
if err != nil {
562-
if r != nil && r.Body != nil {
563-
bodyBytes, readErr := io.ReadAll(r.Body)
564-
if readErr == nil && len(bodyBytes) > 0 {
561+
if r != nil {
562+
apiBody := extractAPIErrorBody(err)
563+
if apiBody != "" {
565564
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",
566-
err, r.StatusCode, string(bodyBytes),
565+
err, r.StatusCode, apiBody,
567566
queryString,
568567
from.Format(time.RFC3339), from.Unix(),
569568
to.Format(time.RFC3339), to.Unix())
@@ -604,11 +603,11 @@ func runMetricsSearch(cmd *cobra.Command, args []string) error {
604603

605604
resp, r, err := api.QueryMetrics(client.Context(), from.Unix(), to.Unix(), queryString)
606605
if err != nil {
607-
if r != nil && r.Body != nil {
608-
bodyBytes, readErr := io.ReadAll(r.Body)
609-
if readErr == nil && len(bodyBytes) > 0 {
606+
if r != nil {
607+
apiBody := extractAPIErrorBody(err)
608+
if apiBody != "" {
610609
return fmt.Errorf("failed to search metrics: %w\nStatus: %d\nAPI Response: %s",
611-
err, r.StatusCode, string(bodyBytes))
610+
err, r.StatusCode, apiBody)
612611
}
613612
return fmt.Errorf("failed to search metrics: %w (status: %d)", err, r.StatusCode)
614613
}
@@ -643,11 +642,11 @@ func runMetricsList(cmd *cobra.Command, args []string) error {
643642

644643
resp, r, err := api.ListActiveMetrics(client.Context(), from, *opts)
645644
if err != nil {
646-
if r != nil && r.Body != nil {
647-
bodyBytes, readErr := io.ReadAll(r.Body)
648-
if readErr == nil && len(bodyBytes) > 0 {
645+
if r != nil {
646+
apiBody := extractAPIErrorBody(err)
647+
if apiBody != "" {
649648
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",
650-
err, r.StatusCode, string(bodyBytes),
649+
err, r.StatusCode, apiBody,
651650
filterPattern,
652651
time.Unix(from, 0).Format(time.RFC3339), from)
653652
}
@@ -677,11 +676,11 @@ func runMetricsMetadataGet(cmd *cobra.Command, args []string) error {
677676

678677
resp, r, err := api.GetMetricMetadata(client.Context(), metricName)
679678
if err != nil {
680-
if r != nil && r.Body != nil {
681-
bodyBytes, readErr := io.ReadAll(r.Body)
682-
if readErr == nil && len(bodyBytes) > 0 {
679+
if r != nil {
680+
apiBody := extractAPIErrorBody(err)
681+
if apiBody != "" {
683682
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",
684-
err, r.StatusCode, string(bodyBytes), metricName)
683+
err, r.StatusCode, apiBody, metricName)
685684
}
686685
return fmt.Errorf("failed to get metric metadata: %w (status: %d)", err, r.StatusCode)
687686
}
@@ -733,11 +732,11 @@ func runMetricsMetadataUpdate(cmd *cobra.Command, args []string) error {
733732

734733
resp, r, err := api.UpdateMetricMetadata(client.Context(), metricName, body)
735734
if err != nil {
736-
if r != nil && r.Body != nil {
737-
bodyBytes, readErr := io.ReadAll(r.Body)
738-
if readErr == nil && len(bodyBytes) > 0 {
735+
if r != nil {
736+
apiBody := extractAPIErrorBody(err)
737+
if apiBody != "" {
739738
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",
740-
err, r.StatusCode, string(bodyBytes), metricName)
739+
err, r.StatusCode, apiBody, metricName)
741740
}
742741
return fmt.Errorf("failed to update metric metadata: %w (status: %d)", err, r.StatusCode)
743742
}
@@ -833,11 +832,11 @@ func runMetricsSubmit(cmd *cobra.Command, args []string) error {
833832

834833
resp, r, err := api.SubmitMetrics(client.Context(), body, *datadogV2.NewSubmitMetricsOptionalParameters())
835834
if err != nil {
836-
if r != nil && r.Body != nil {
837-
bodyBytes, readErr := io.ReadAll(r.Body)
838-
if readErr == nil && len(bodyBytes) > 0 {
835+
if r != nil {
836+
apiBody := extractAPIErrorBody(err)
837+
if apiBody != "" {
839838
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",
840-
err, r.StatusCode, string(bodyBytes),
839+
err, r.StatusCode, apiBody,
841840
submitName, submitValue, submitType, timestamp, tags)
842841
}
843842
return fmt.Errorf("failed to submit metrics: %w (status: %d)", err, r.StatusCode)

cmd/root.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ package cmd
77

88
import (
99
"bufio"
10+
"errors"
1011
"fmt"
1112
"io"
1213
"os"
1314
"strings"
1415

16+
"github.com/DataDog/datadog-api-client-go/v2/api/datadog"
1517
"github.com/DataDog/pup/internal/version"
1618
"github.com/DataDog/pup/pkg/client"
1719
"github.com/DataDog/pup/pkg/config"
@@ -259,6 +261,23 @@ func readConfirmation() (string, error) {
259261
return "", scanner.Err()
260262
}
261263

264+
// extractAPIErrorBody extracts the API response body from a
265+
// datadog.GenericOpenAPIError. The datadog-api-client-go library consumes
266+
// http.Response.Body during deserialization and stores the bytes in the error.
267+
// Callers that try to re-read http.Response.Body will always get empty data.
268+
func extractAPIErrorBody(err error) string {
269+
if err == nil {
270+
return ""
271+
}
272+
var apiErr datadog.GenericOpenAPIError
273+
if errors.As(err, &apiErr) {
274+
if body := apiErr.Body(); len(body) > 0 {
275+
return string(body)
276+
}
277+
}
278+
return ""
279+
}
280+
262281
// formatAPIError creates user-friendly error messages for API errors
263282
func formatAPIError(operation string, err error, response any) error {
264283
type httpResponse interface {
@@ -269,6 +288,11 @@ func formatAPIError(operation string, err error, response any) error {
269288
statusCode := r.StatusCode()
270289
baseMsg := fmt.Sprintf("failed to %s: %v (status: %d)", operation, err, statusCode)
271290

291+
// Include API response body if available
292+
if body := extractAPIErrorBody(err); body != "" {
293+
baseMsg = fmt.Sprintf("failed to %s: %v (status: %d)\nAPI Response: %s", operation, err, statusCode, body)
294+
}
295+
272296
switch {
273297
case statusCode >= 500:
274298
// 5xx Server errors

cmd/root_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111
"testing"
1212

13+
"github.com/DataDog/datadog-api-client-go/v2/api/datadog"
1314
"github.com/DataDog/pup/pkg/config"
1415
)
1516

@@ -318,3 +319,75 @@ func TestTestCmd_InvalidSite(t *testing.T) {
318319
t.Errorf("testCmd.RunE() error should mention DD_SITE, got: %v", err)
319320
}
320321
}
322+
323+
func TestExtractAPIErrorBody(t *testing.T) {
324+
tests := []struct {
325+
name string
326+
err error
327+
want string
328+
}{
329+
{
330+
name: "GenericOpenAPIError with body",
331+
err: datadog.GenericOpenAPIError{
332+
ErrorBody: []byte(`{"errors":["Invalid query: avg:nonexistent.metric{*}"]}`),
333+
ErrorMessage: "400 Bad Request",
334+
},
335+
want: `{"errors":["Invalid query: avg:nonexistent.metric{*}"]}`,
336+
},
337+
{
338+
name: "GenericOpenAPIError with empty body",
339+
err: datadog.GenericOpenAPIError{
340+
ErrorBody: []byte{},
341+
ErrorMessage: "400 Bad Request",
342+
},
343+
want: "",
344+
},
345+
{
346+
name: "GenericOpenAPIError with nil body",
347+
err: datadog.GenericOpenAPIError{
348+
ErrorBody: nil,
349+
ErrorMessage: "400 Bad Request",
350+
},
351+
want: "",
352+
},
353+
{
354+
name: "non-GenericOpenAPIError",
355+
err: errors.New("some other error"),
356+
want: "",
357+
},
358+
{
359+
name: "nil error",
360+
err: nil,
361+
want: "",
362+
},
363+
}
364+
365+
for _, tt := range tests {
366+
t.Run(tt.name, func(t *testing.T) {
367+
got := extractAPIErrorBody(tt.err)
368+
if got != tt.want {
369+
t.Errorf("extractAPIErrorBody() = %q, want %q", got, tt.want)
370+
}
371+
})
372+
}
373+
}
374+
375+
func TestFormatAPIError_IncludesResponseBody(t *testing.T) {
376+
// This test verifies that formatAPIError surfaces the API response body
377+
// from GenericOpenAPIError, which was previously lost because the code
378+
// tried to re-read the already-consumed http.Response.Body.
379+
apiErr := datadog.GenericOpenAPIError{
380+
ErrorBody: []byte(`{"errors":["Query parse error: unknown metric"]}`),
381+
ErrorMessage: "400 Bad Request",
382+
}
383+
384+
err := formatAPIError("query metrics", apiErr, &mockHTTPResponse{statusCode: 400})
385+
errMsg := err.Error()
386+
387+
if !strings.Contains(errMsg, "unknown metric") {
388+
t.Errorf("formatAPIError() should include API response body, got: %q", errMsg)
389+
}
390+
if !strings.Contains(errMsg, "status: 400") {
391+
t.Errorf("formatAPIError() should include status code, got: %q", errMsg)
392+
}
393+
}

0 commit comments

Comments
 (0)