From 424e0a1f898180a9c59326ca703e7690fcbdf0be Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 15 Jul 2026 12:16:00 +1000 Subject: [PATCH 1/4] fix(server): stop replaying non-idempotent compute requests on auth rejection The proxy layer re-sent the entire request after a 401/403 invalidate-and-refresh, which silently re-executed guest commands for compute.exec_sandbox. Thread a replay policy down to the proxy request: compute mutations refresh the token but return a clear retry error instead of re-sending; read-only requests and all other operations keep the single retry. --- pkg/server/api.go | 23 +++++- pkg/server/operations_benchmarkoor.go | 1 + pkg/server/operations_clickhouse.go | 1 + pkg/server/operations_compute.go | 8 ++ pkg/server/operations_dispatch.go | 1 + pkg/server/proxy_replay_test.go | 106 ++++++++++++++++++++++++++ pkg/server/proxy_routing_test.go | 2 + 7 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 pkg/server/proxy_replay_test.go diff --git a/pkg/server/api.go b/pkg/server/api.go index 7e9b138b..abc73798 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -760,6 +760,13 @@ func (s *service) handleAPIBuildStatus(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, proxyResp) } +// Replay policy for proxied requests: a replayable request may be re-sent once +// after an auth rejection; a non-replayable one must never run twice upstream. +const ( + proxyReplayable = true + proxyNoReplay = false +) + func (s *service) proxyRequest( ctx context.Context, method string, @@ -772,7 +779,7 @@ func (s *service) proxyRequest( return nil, http.StatusServiceUnavailable, nil, err } - return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers) + return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers, proxyReplayable) } func (s *service) proxyDatasourceRequest( @@ -783,13 +790,14 @@ func (s *service) proxyDatasourceRequest( requestPath string, body io.Reader, headers http.Header, + replayable bool, ) ([]byte, int, http.Header, error) { proxySvc, status, err := s.proxyServiceForDatasource(datasourceType, datasourceName) if err != nil { return nil, status, nil, err } - return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers) + return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers, replayable) } func (s *service) proxyRequestWithService( @@ -799,6 +807,7 @@ func (s *service) proxyRequestWithService( requestPath string, body io.Reader, headers http.Header, + replayable bool, ) ([]byte, int, http.Header, error) { if s.proxyService == nil { return nil, http.StatusServiceUnavailable, nil, fmt.Errorf("proxy service is unavailable") @@ -874,11 +883,17 @@ func (s *service) proxyRequestWithService( } // One invalidate-and-retry on auth rejection covers a token revoked - // server-side before the local refresh buffer kicks in. + // server-side before the local refresh buffer kicks in. Non-replayable + // requests still refresh the token but must not run twice upstream. if status == http.StatusUnauthorized || status == http.StatusForbidden { proxySvc.Invalidate() - return send() + if replayable { + return send() + } + + return nil, status, header, fmt.Errorf( + "authentication expired mid-request; token refreshed — retry the command") } return data, status, header, nil diff --git a/pkg/server/operations_benchmarkoor.go b/pkg/server/operations_benchmarkoor.go index 75fb18a5..aaad4b3a 100644 --- a/pkg/server/operations_benchmarkoor.go +++ b/pkg/server/operations_benchmarkoor.go @@ -185,6 +185,7 @@ func (s *service) handleBenchmarkoorGetRun(w http.ResponseWriter, r *http.Reques benchmarkoorAPIPrefix+"/index/query/runs?"+params.Encode(), nil, http.Header{handlers.DatasourceHeader: []string{datasource}}, + proxyReplayable, ) if err != nil { writeAPIError(w, status, err.Error()) diff --git a/pkg/server/operations_clickhouse.go b/pkg/server/operations_clickhouse.go index 3633c34f..e2d2828f 100644 --- a/pkg/server/operations_clickhouse.go +++ b/pkg/server/operations_clickhouse.go @@ -108,6 +108,7 @@ func (s *service) handleClickHouseQuery(w http.ResponseWriter, r *http.Request) handlers.DatasourceHeader: []string{datasource}, "Content-Type": []string{"text/plain"}, }, + proxyReplayable, ) if err != nil { writeAPIError(w, status, err.Error()) diff --git a/pkg/server/operations_compute.go b/pkg/server/operations_compute.go index 9ca88ba2..5cf4e1c7 100644 --- a/pkg/server/operations_compute.go +++ b/pkg/server/operations_compute.go @@ -57,6 +57,13 @@ func (d *computeProxyDoer) Do(req *http.Request) (*http.Response, error) { headers.Set(handlers.DatasourceHeader, d.datasource) + // Compute mutations (exec above all) must never run twice, so only + // read-only methods keep the proxy's invalidate-and-retry on auth rejection. + replayable := proxyNoReplay + if req.Method == http.MethodGet || req.Method == http.MethodHead { + replayable = proxyReplayable + } + respBody, status, respHeaders, err := d.svc.proxyDatasourceRequest( req.Context(), "compute", @@ -65,6 +72,7 @@ func (d *computeProxyDoer) Do(req *http.Request) (*http.Response, error) { requestPath, body, headers, + replayable, ) if err != nil { return nil, err diff --git a/pkg/server/operations_dispatch.go b/pkg/server/operations_dispatch.go index 5a2ef261..8c81613f 100644 --- a/pkg/server/operations_dispatch.go +++ b/pkg/server/operations_dispatch.go @@ -53,6 +53,7 @@ func (s *service) proxyPassthroughGet( requestPath, nil, http.Header{handlers.DatasourceHeader: []string{datasource}}, + proxyReplayable, ) if err != nil { writeAPIError(w, status, err.Error()) diff --git a/pkg/server/proxy_replay_test.go b/pkg/server/proxy_replay_test.go new file mode 100644 index 00000000..aef4bef8 --- /dev/null +++ b/pkg/server/proxy_replay_test.go @@ -0,0 +1,106 @@ +package server + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ethpandaops/panda/pkg/proxy" + "github.com/ethpandaops/panda/pkg/types" +) + +// sequenceTransport answers each request with the next queued status, then +// repeats the last one. It counts round trips so tests can assert on replays. +type sequenceTransport struct { + statuses []int + bodies []string + calls int +} + +func (t *sequenceTransport) RoundTrip(_ *http.Request) (*http.Response, error) { + idx := t.calls + if idx >= len(t.statuses) { + idx = len(t.statuses) - 1 + } + + t.calls++ + + resp := &http.Response{ + StatusCode: t.statuses[idx], + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewBufferString(t.bodies[idx])), + } + + return resp, nil +} + +// invalidatingProxyClient counts Invalidate calls on top of the routing fake. +type invalidatingProxyClient struct { + routingProxyClient + invalidations int +} + +func (c *invalidatingProxyClient) Invalidate() { c.invalidations++ } + +func newReplayComputeService(t *testing.T, transport http.RoundTripper) (*service, *invalidatingProxyClient) { + t.Helper() + + client := &invalidatingProxyClient{ + routingProxyClient: routingProxyClient{ + url: "https://hosted.proxy", + token: "hosted-token", + compute: []types.DatasourceInfo{{Name: "production"}}, + }, + } + + svc := testRoutingService(t, transport, []proxy.ClientRoute{{Name: "hosted", Client: client}}) + + return svc, client +} + +// TestComputeExecNotReplayedOnAuthRejection guards against silently running a +// guest command twice: an auth rejection must refresh the token and surface a +// retry hint instead of re-sending the exec. +func TestComputeExecNotReplayedOnAuthRejection(t *testing.T) { + t.Parallel() + + transport := &sequenceTransport{ + statuses: []int{http.StatusUnauthorized, http.StatusOK}, + bodies: []string{`{"error":"token expired"}`, `{"exit_code":0}`}, + } + svc, client := newReplayComputeService(t, transport) + + rec := callComputeOp(t, svc, "compute.exec_sandbox", map[string]any{ + "id": "sb-1", + "command": []any{"rm", "-rf", "/tmp/scratch"}, + }) + + assert.Equal(t, 1, transport.calls, "a non-replayable request must not be re-sent") + assert.Equal(t, 1, client.invalidations, "the cached token must still be invalidated") + assert.Equal(t, http.StatusBadGateway, rec.Code) + assert.Contains(t, rec.Body.String(), "authentication expired mid-request") + assert.Contains(t, rec.Body.String(), "retry the command") +} + +// TestComputeReadReplaysOnAuthRejection verifies read-only compute requests +// keep the one invalidate-and-retry on auth rejection. +func TestComputeReadReplaysOnAuthRejection(t *testing.T) { + t.Parallel() + + transport := &sequenceTransport{ + statuses: []int{http.StatusUnauthorized, http.StatusOK}, + bodies: []string{`{"error":"token expired"}`, `{"id":"sb-1"}`}, + } + svc, client := newReplayComputeService(t, transport) + + rec := callComputeOp(t, svc, "compute.get_sandbox", map[string]any{"id": "sb-1"}) + + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + assert.Equal(t, 2, transport.calls, "a replayable request retries once after invalidation") + assert.Equal(t, 1, client.invalidations) + assert.JSONEq(t, `{"id":"sb-1"}`, rec.Body.String()) +} diff --git a/pkg/server/proxy_routing_test.go b/pkg/server/proxy_routing_test.go index 2af03ef5..4c2dfd7d 100644 --- a/pkg/server/proxy_routing_test.go +++ b/pkg/server/proxy_routing_test.go @@ -101,6 +101,7 @@ func TestDatasourceProxyRequestRoutesByTypeAndName(t *testing.T) { "/loki/loki/api/v1/query?query=up", nil, http.Header{handlers.DatasourceHeader: []string{"logs"}}, + proxyReplayable, ) if err != nil { t.Fatalf("proxyDatasourceRequest error = %v", err) @@ -329,6 +330,7 @@ func TestProxyRequestForwardsAttribution(t *testing.T) { "/clickhouse/", strings.NewReader("SELECT 1"), http.Header{handlers.DatasourceHeader: []string{"xatu"}}, + proxyReplayable, ) if err != nil { t.Fatalf("proxyDatasourceRequest error = %v", err) From 32a2b40002098c3bff11ee42db951f8ee9947095 Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 15 Jul 2026 12:17:50 +1000 Subject: [PATCH 2/4] fix(cli): bound compute exec with a wall-clock deadline The CLI HTTP client has no timeout, so a hung exec path stalled the command until the proxy write timeout expired. Cap the exec call at the guest timeout (or the 30s server default) plus a minute of transport headroom, and point the user at the sandbox when the deadline fires. --- pkg/cli/compute.go | 39 +++++++++++++++++++++++++++++++++++++-- pkg/cli/compute_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 pkg/cli/compute_test.go diff --git a/pkg/cli/compute.go b/pkg/cli/compute.go index 3482b7f2..2a2bf7e0 100644 --- a/pkg/cli/compute.go +++ b/pkg/cli/compute.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "encoding/hex" "encoding/json" + "errors" "fmt" "strconv" "strings" @@ -559,10 +560,40 @@ var computeSandboxesExecCmd = &cobra.Command{ opArgs["command"] = command setIfNotEmpty(opArgs, "timeout", computeExecTimeout) - return runComputeRaw(cmd, "compute.exec_sandbox", opArgs) + budget := computeExecDeadline(computeExecTimeout) + + ctx, cancel := context.WithTimeout(commandContext(cmd), budget) + defer cancel() + + err := runComputeRawContext(ctx, "compute.exec_sandbox", opArgs) + if err != nil && errors.Is(ctx.Err(), context.DeadlineExceeded) { + return fmt.Errorf( + "no response after %s — the guest command may still be running; check the sandbox with 'panda compute sandboxes get %s'", + budget, args[0], + ) + } + + return err }, } +// computeExecDeadline is the CLI wall-clock budget for one exec call: the +// guest timeout (or the 30s server default) plus transport headroom. Invalid +// --timeout values keep the default budget; the server still rejects them. +func computeExecDeadline(timeout string) time.Duration { + const ( + defaultGuestTimeout = 30 * time.Second + transportHeadroom = time.Minute + ) + + guest := defaultGuestTimeout + if parsed, err := time.ParseDuration(timeout); err == nil && parsed > 0 { + guest = parsed + } + + return guest + transportHeadroom +} + var computeSandboxesMetricsCmd = &cobra.Command{ Use: "metrics ", Short: "Fetch guest resource metrics for a sandbox", @@ -969,7 +1000,11 @@ func computeIdemOrGenerated() string { // single object) and raw JSON when --output json is set. List results honour // --filter, which the server applies before pagination. func runComputeRaw(cmd *cobra.Command, operationID string, args map[string]any) error { - response, err := runServerOperationRaw(cmd, operationID, args) + return runComputeRawContext(commandContext(cmd), operationID, args) +} + +func runComputeRawContext(ctx context.Context, operationID string, args map[string]any) error { + response, err := serverOperationRaw(ctx, operationID, args) if err != nil { return err } diff --git a/pkg/cli/compute_test.go b/pkg/cli/compute_test.go new file mode 100644 index 00000000..bf97609c --- /dev/null +++ b/pkg/cli/compute_test.go @@ -0,0 +1,32 @@ +package cli + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestComputeExecDeadline(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + timeout string + want time.Duration + }{ + {name: "unset uses the server default budget", timeout: "", want: 90 * time.Second}, + {name: "explicit timeout adds headroom", timeout: "2m", want: 3 * time.Minute}, + {name: "max guest timeout adds headroom", timeout: "5m", want: 6 * time.Minute}, + {name: "unparseable falls back to the default budget", timeout: "bogus", want: 90 * time.Second}, + {name: "non-positive falls back to the default budget", timeout: "-10s", want: 90 * time.Second}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, computeExecDeadline(tt.timeout)) + }) + } +} From 03423aad3bed48afc9dc5eb7f881f2e76610e0a4 Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 15 Jul 2026 12:19:25 +1000 Subject: [PATCH 3/4] fix(proxy): keep server and transport timeouts above the exec ceiling The default write timeout equalled the 5m guest command maximum, so a near-max exec raced the proxy deadline; bump it to 6m. Give the reverse proxy transport a 5m30s response-header timeout so a hung backend fails fast instead of holding the connection until the write timeout. --- pkg/proxy/handlers/transport.go | 16 +++++++++++----- pkg/proxy/server_config.go | 4 +++- proxy-config.example.yaml | 5 +++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/proxy/handlers/transport.go b/pkg/proxy/handlers/transport.go index 4adf111f..a0920e3c 100644 --- a/pkg/proxy/handlers/transport.go +++ b/pkg/proxy/handlers/transport.go @@ -14,6 +14,11 @@ const ( defaultMaxIdleConns = 100 defaultMaxIdleConnsPerHost = 10 defaultIdleConnTimeout = 90 * time.Second + + // Above the 5m compute exec ceiling (sync exec sends headers only after + // the command completes) and below the 6m server write timeout, so a hung + // backend fails fast instead of holding the connection open. + defaultResponseHeaderTimeout = 5*time.Minute + 30*time.Second ) // newProxyTransport returns an *http.Transport with sensible defaults for @@ -24,10 +29,11 @@ func newProxyTransport(skipVerify bool) *http.Transport { TLSClientConfig: &tls.Config{ InsecureSkipVerify: skipVerify, //nolint:gosec // User-configured per datasource }, - DialContext: (&net.Dialer{Timeout: defaultDialTimeout}).DialContext, - TLSHandshakeTimeout: defaultTLSHandshakeTimeout, - MaxIdleConns: defaultMaxIdleConns, - MaxIdleConnsPerHost: defaultMaxIdleConnsPerHost, - IdleConnTimeout: defaultIdleConnTimeout, + DialContext: (&net.Dialer{Timeout: defaultDialTimeout}).DialContext, + TLSHandshakeTimeout: defaultTLSHandshakeTimeout, + MaxIdleConns: defaultMaxIdleConns, + MaxIdleConnsPerHost: defaultMaxIdleConnsPerHost, + IdleConnTimeout: defaultIdleConnTimeout, + ResponseHeaderTimeout: defaultResponseHeaderTimeout, } } diff --git a/pkg/proxy/server_config.go b/pkg/proxy/server_config.go index 6e082eed..f9bd6d7a 100644 --- a/pkg/proxy/server_config.go +++ b/pkg/proxy/server_config.go @@ -482,8 +482,10 @@ func (c *ServerConfig) ApplyDefaults() { c.Server.ReadTimeout = 30 * time.Second } + // Above the 5m compute exec ceiling, so a near-max guest command cannot + // race the server deadline. if c.Server.WriteTimeout == 0 { - c.Server.WriteTimeout = 5 * time.Minute + c.Server.WriteTimeout = 6 * time.Minute } if c.Server.IdleTimeout == 0 { diff --git a/proxy-config.example.yaml b/proxy-config.example.yaml index bc16c4e8..d71e7ecf 100644 --- a/proxy-config.example.yaml +++ b/proxy-config.example.yaml @@ -14,9 +14,10 @@ server: # Address to listen on listen_addr: ":18081" - # HTTP timeouts + # HTTP timeouts. Keep write_timeout above the 5m compute exec ceiling so a + # near-max guest command cannot race the server deadline. read_timeout: 30s - write_timeout: 5m + write_timeout: 6m idle_timeout: 60s auth: From c4033b895b4e8df25304c0252ad479813530de75 Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 15 Jul 2026 12:22:33 +1000 Subject: [PATCH 4/4] fix(compute): surface structured upstream errors end to end Non-2xx compute backend bodies were re-wrapped as {"error": ""}, hiding the structured code and request_id fields from the CLI. Forward JSON error objects verbatim, render code/request_id in CLI error messages, and add a 500 hint pointing at the sandbox. --- pkg/cli/serverclient.go | 27 ++++++++++-- pkg/cli/serverclient_test.go | 61 +++++++++++++++++++++++++++ pkg/server/operations_compute.go | 19 +++++++++ pkg/server/operations_compute_test.go | 31 ++++++++++++++ 4 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 pkg/cli/serverclient_test.go diff --git a/pkg/cli/serverclient.go b/pkg/cli/serverclient.go index 73e58b7f..2930e4b5 100644 --- a/pkg/cli/serverclient.go +++ b/pkg/cli/serverclient.go @@ -230,15 +230,16 @@ func decodeAPIError(status int, data []byte) error { // apiErrorMessage extracts a human-readable message from an error response body. // It tries the common JSON error fields in order — panda's `error`, then RFC // 7807 problem+json `detail` and `title` — before falling back to the raw body. -// The raw-body fallback covers non-JSON responses (e.g. an unknown route's -// `text/plain` "404 page not found"), so decoding never panics or loses the -// message. +// Structured `code` and `request_id` fields are appended when present so the +// failure is actionable and traceable. The raw-body fallback covers non-JSON +// responses (e.g. an unknown route's `text/plain` "404 page not found"), so +// decoding never panics or loses the message. func apiErrorMessage(data []byte) string { var payload map[string]any if err := json.Unmarshal(data, &payload); err == nil { for _, key := range []string{"error", "detail", "title"} { if msg, ok := payload[key].(string); ok && msg != "" { - return msg + return msg + apiErrorContext(payload) } } } @@ -246,6 +247,22 @@ func apiErrorMessage(data []byte) string { return strings.TrimSpace(string(data)) } +func apiErrorContext(payload map[string]any) string { + parts := make([]string, 0, 2) + + for _, key := range []string{"code", "request_id"} { + if value, ok := payload[key].(string); ok && value != "" { + parts = append(parts, key+"="+value) + } + } + + if len(parts) == 0 { + return "" + } + + return " (" + strings.Join(parts, ", ") + ")" +} + func serverErrorHint(status int, message string) string { // Error classification is the ClickHouse module's knowledge; the CLI only // owns the command-idiom wording per class. @@ -298,6 +315,8 @@ func serverErrorHint(status int, message string) string { switch status { case http.StatusNotFound: return "the requested module, operation, datasource, or resource is not available on this server; check 'panda datasources' and 'panda resources'" + case http.StatusInternalServerError: + return "the backend hit an internal error; for compute operations check the sandbox with 'panda compute sandboxes get ' — the guest may be unreachable" case http.StatusBadGateway: return "an upstream datasource or node returned a gateway error; the target may be temporarily unreachable — retry, or confirm it is still advertised with 'panda datasources'" case http.StatusServiceUnavailable: diff --git a/pkg/cli/serverclient_test.go b/pkg/cli/serverclient_test.go new file mode 100644 index 00000000..8095eff6 --- /dev/null +++ b/pkg/cli/serverclient_test.go @@ -0,0 +1,61 @@ +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAPIErrorMessage(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body string + want string + }{ + { + name: "plain error field", + body: `{"error":"sandbox not found"}`, + want: "sandbox not found", + }, + { + name: "error with code and request id", + body: `{"error":"guest unreachable","code":"guest_unreachable","request_id":"req-123"}`, + want: "guest unreachable (code=guest_unreachable, request_id=req-123)", + }, + { + name: "error with code only", + body: `{"error":"guest unreachable","code":"guest_unreachable"}`, + want: "guest unreachable (code=guest_unreachable)", + }, + { + name: "error with request id only", + body: `{"error":"internal error","request_id":"req-456"}`, + want: "internal error (request_id=req-456)", + }, + { + name: "problem json detail", + body: `{"detail":"upstream failed","title":"Bad Gateway"}`, + want: "upstream failed", + }, + { + name: "non-string structured fields are ignored", + body: `{"error":"boom","code":42}`, + want: "boom", + }, + { + name: "non-json falls back to raw body", + body: "404 page not found\n", + want: "404 page not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, apiErrorMessage([]byte(tt.body))) + }) + } +} diff --git a/pkg/server/operations_compute.go b/pkg/server/operations_compute.go index 5cf4e1c7..aaec1d95 100644 --- a/pkg/server/operations_compute.go +++ b/pkg/server/operations_compute.go @@ -693,6 +693,14 @@ func (s *service) writeComputeResult(w http.ResponseWriter, resp *http.Response, } if resp.StatusCode < 200 || resp.StatusCode >= 300 { + // Forward JSON error objects verbatim so structured fields (code, + // request_id) survive to the CLI instead of being nested in a wrapper. + if trimmed := bytes.TrimSpace(body); jsonObjectBody(trimmed) { + writePassthroughResponse(w, resp.StatusCode, "application/json", trimmed) + + return + } + writeAPIError(w, resp.StatusCode, strings.TrimSpace(string(body))) return @@ -708,6 +716,17 @@ func (s *service) writeComputeResult(w http.ResponseWriter, resp *http.Response, writePassthroughResponse(w, resp.StatusCode, contentType, body) } +// jsonObjectBody reports whether body is a JSON object that can be forwarded +// to the caller verbatim. +func jsonObjectBody(body []byte) bool { + var payload map[string]any + if err := json.Unmarshal(body, &payload); err != nil { + return false + } + + return payload != nil +} + func (s *service) computeDatasources() ([]types.DatasourceInfo, error) { if s.proxyService == nil { return nil, fmt.Errorf("compute is unavailable") diff --git a/pkg/server/operations_compute_test.go b/pkg/server/operations_compute_test.go index d4f30e19..f2d49000 100644 --- a/pkg/server/operations_compute_test.go +++ b/pkg/server/operations_compute_test.go @@ -212,6 +212,37 @@ func TestComputeListForwardsFilters(t *testing.T) { assert.Equal(t, []string{"status=running", "vcpu>=4"}, transport.last.URL.Query()["filter"]) } +// TestComputeForwardsStructuredUpstreamError verifies a JSON error object from +// the backend is forwarded verbatim, keeping structured fields like code and +// request_id visible to the CLI instead of nesting them in a wrapper. +func TestComputeForwardsStructuredUpstreamError(t *testing.T) { + t.Parallel() + + upstream := `{"error":"guest unreachable","code":"guest_unreachable","request_id":"req-123"}` + transport := &recordingTransport{status: http.StatusInternalServerError, body: upstream, contentType: "application/json"} + svc := newComputeService(t, transport, types.DatasourceInfo{Name: "production"}) + + rec := callComputeOp(t, svc, "compute.get_sandbox", map[string]any{"id": "sb-1"}) + + assert.Equal(t, http.StatusInternalServerError, rec.Code) + assert.Equal(t, "application/json", rec.Header().Get("Content-Type")) + assert.JSONEq(t, upstream, rec.Body.String()) +} + +// TestComputeWrapsNonJSONUpstreamError verifies non-JSON error bodies still get +// the standard {"error": ...} wrapper. +func TestComputeWrapsNonJSONUpstreamError(t *testing.T) { + t.Parallel() + + transport := &recordingTransport{status: http.StatusBadGateway, body: "upstream exploded", contentType: "text/plain"} + svc := newComputeService(t, transport, types.DatasourceInfo{Name: "production"}) + + rec := callComputeOp(t, svc, "compute.get_sandbox", map[string]any{"id": "sb-1"}) + + assert.Equal(t, http.StatusBadGateway, rec.Code) + assert.JSONEq(t, `{"error":"upstream exploded"}`, rec.Body.String()) +} + // TestComputeRequiresDatasourceWhenAmbiguous verifies an explicit datasource is // required when more than one compute datasource is advertised. func TestComputeRequiresDatasourceWhenAmbiguous(t *testing.T) {