From c281f721bc167248f53b0f80d1597046358c0e88 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 23:28:00 +0000 Subject: [PATCH] Preserve transport error identities --- docs/integrations/openai-compatible-chat.md | 11 +- docs/internal/llm.md | 21 ++- engine_test.go | 3 + internal/llm/openai_compatible_client.go | 14 +- internal/llm/openai_compatible_client_test.go | 142 ++++++++++++++++-- 5 files changed, 171 insertions(+), 20 deletions(-) diff --git a/docs/integrations/openai-compatible-chat.md b/docs/integrations/openai-compatible-chat.md index e96734f..5714b8d 100644 --- a/docs/integrations/openai-compatible-chat.md +++ b/docs/integrations/openai-compatible-chat.md @@ -90,6 +90,11 @@ Invalid JSON, absent choices, and empty first-choice content are malformed responses. For a non-2xx status, the error includes the status code but never the provider response body. +An outbound `http.Client.Do` failure retains both Promptkit's request-failure +identity and the exact transport error for `errors.Is` and `errors.As` checks. +The rendered error does not include the selected endpoint, request headers, +request content, credentials, or provider body. + ## Timeout And Cancellation Timeouts are layered: @@ -103,5 +108,7 @@ Timeouts are layered: timeout when the supplied value is not positive. The earliest applicable caller, generation, or transport deadline controls the -request. Constructing the internal client does not mutate a supplied -`http.Client`. +request. Caller cancellation retains `context.Canceled`; caller, generation, +and whole-request timeout failures retain `context.DeadlineExceeded`, together +with the request-failure identity. Constructing the internal client does not +mutate a supplied `http.Client`. diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 75cab78..2f917f6 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -65,9 +65,14 @@ configuration, invalid generation requests, request execution failures, non-success provider statuses, and malformed successful responses. Provider response bodies are not included in non-success errors. -Caller cancellation and deadline failures during the outbound request are -reported as request execution failures. The runner classifies these identities -without depending on HTTP status mapping. +An `http.Client.Do` failure is represented by a redacting multi-cause error: +the package request-failure sentinel and the exact returned transport error are +both available through `errors.Is` and `errors.As`, while the rendered text +does not expose the endpoint, headers, request content, credential, transport +detail, or provider body. Caller cancellation retains `context.Canceled`; +caller deadlines, generation deadlines, and whole-request client timeouts +retain `context.DeadlineExceeded`. The runner adds its generation category +without discarding those identities or depending on HTTP status mapping. ## Test Ownership @@ -75,7 +80,9 @@ The [OpenAI-compatible client tests](../../internal/llm/openai_compatible_client_test.go) own configuration, client cloning, deterministic deadline precedence, authentication, request and response mapping, malformed data, error identity, -cancellation, and response-body suppression. The root transport contract test -also verifies that resolved backend settings reach this client without -serializing backend identity. All use local test servers or test transports; -the default suite makes no live or paid provider requests. +cancellation, endpoint selection, and response-body suppression. The root +transport contract tests also verify that resolved backend settings reach this +client without serializing backend identity and that ordinary-run cancellation +retains its public generation and context identities. All use local test +servers or controlled test transports; the default suite makes no live or paid +provider requests. diff --git a/engine_test.go b/engine_test.go index 4a6286c..b2e878e 100644 --- a/engine_test.go +++ b/engine_test.go @@ -737,6 +737,9 @@ func TestEngineRunPropagatesCallerCancellation(t *testing.T) { if !errors.Is(err, promptkit.ErrLLMGenerate) { t.Fatalf("expected ErrLLMGenerate after caller cancellation, got %v", err) } + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context cancellation identity, got %v", err) + } case <-watchdog.C: t.Fatal("timed out waiting for Engine.Run to return after cancellation") } diff --git a/internal/llm/openai_compatible_client.go b/internal/llm/openai_compatible_client.go index 788c300..c666565 100644 --- a/internal/llm/openai_compatible_client.go +++ b/internal/llm/openai_compatible_client.go @@ -25,6 +25,18 @@ var ( ErrMalformedResponse = errors.New("malformed llm response") ) +type requestFailedError struct { + cause error +} + +func (e *requestFailedError) Error() string { + return ErrRequestFailed.Error() +} + +func (e *requestFailedError) Unwrap() []error { + return []error{ErrRequestFailed, e.cause} +} + type OpenAICompatibleConfig struct { BaseURL string Model string @@ -130,7 +142,7 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera httpResp, err := httpClient.Do(httpReq) if err != nil { - return nil, fmt.Errorf("%w: %v", ErrRequestFailed, err) + return nil, &requestFailedError{cause: err} } defer httpResp.Body.Close() diff --git a/internal/llm/openai_compatible_client_test.go b/internal/llm/openai_compatible_client_test.go index b28e780..0a64eb5 100644 --- a/internal/llm/openai_compatible_client_test.go +++ b/internal/llm/openai_compatible_client_test.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "errors" + "io" "math" "net/http" "net/http/httptest" + "net/url" "strconv" "strings" "testing" @@ -20,10 +22,14 @@ var errTransportStopped = errors.New("transport stopped after request inspection type deadlineCapturingTransport struct { deadline time.Time hasDeadline bool + err error } func (t *deadlineCapturingTransport) RoundTrip(req *http.Request) (*http.Response, error) { t.deadline, t.hasDeadline = req.Context().Deadline() + if t.err != nil { + return nil, t.err + } return nil, errTransportStopped } @@ -33,6 +39,19 @@ func (contextErrorTransport) RoundTrip(req *http.Request) (*http.Response, error return nil, req.Context().Err() } +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +type waitingContextTransport struct{} + +func (waitingContextTransport) RoundTrip(req *http.Request) (*http.Response, error) { + <-req.Context().Done() + return nil, req.Context().Err() +} + func assertDeadlineNear(t *testing.T, deadline, before, after time.Time, duration time.Duration) { t.Helper() @@ -762,6 +781,9 @@ func TestOpenAICompatibleClientOmittedTimeoutUsesClientTimeout(t *testing.T) { if !errors.Is(err, ErrRequestFailed) { t.Fatalf("expected ErrRequestFailed, got %v", err) } + if !errors.Is(err, errTransportStopped) { + t.Fatalf("expected transport cause, got %v", err) + } if !transport.hasDeadline { t.Fatal("expected client timeout to set a transport deadline") } @@ -1027,7 +1049,7 @@ func TestOpenAICompatibleClientMalformedResponseMissingChoices(t *testing.T) { } func TestOpenAICompatibleClientGenerationTimeoutSetsEarlierDeadline(t *testing.T) { - transport := &deadlineCapturingTransport{} + transport := &deadlineCapturingTransport{err: context.DeadlineExceeded} generationTimeout := 2 * time.Second client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ @@ -1056,6 +1078,9 @@ func TestOpenAICompatibleClientGenerationTimeoutSetsEarlierDeadline(t *testing.T if !errors.Is(err, ErrRequestFailed) { t.Fatalf("expected ErrRequestFailed, got %v", err) } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected generation deadline identity, got %v", err) + } if !transport.hasDeadline { t.Fatal("expected generation timeout to set a transport deadline") } @@ -1063,7 +1088,7 @@ func TestOpenAICompatibleClientGenerationTimeoutSetsEarlierDeadline(t *testing.T } func TestOpenAICompatibleClientCallerDeadlineTakesPrecedence(t *testing.T) { - transport := &deadlineCapturingTransport{} + transport := &deadlineCapturingTransport{err: context.DeadlineExceeded} client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ BaseURL: "http://example.com/v1", Model: "m", @@ -1092,6 +1117,9 @@ func TestOpenAICompatibleClientCallerDeadlineTakesPrecedence(t *testing.T) { if !errors.Is(err, ErrRequestFailed) { t.Fatalf("expected ErrRequestFailed, got %v", err) } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected caller deadline identity, got %v", err) + } if !transport.hasDeadline { t.Fatal("expected caller context to set a transport deadline") } @@ -1124,8 +1152,87 @@ func TestOpenAICompatibleClientCancellationReturnsRequestFailure(t *testing.T) { if !errors.Is(err, ErrRequestFailed) { t.Fatalf("expected ErrRequestFailed, got %v", err) } - if !strings.Contains(err.Error(), context.Canceled.Error()) { - t.Fatalf("expected cancellation detail, got %v", err) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected cancellation identity, got %v", err) + } +} + +func TestOpenAICompatibleClientExpiredCallerDeadlineReturnsRequestFailure(t *testing.T) { + client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ + BaseURL: "http://example.com/v1", + Model: "m", + HTTPClient: &http.Client{ + Transport: contextErrorTransport{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithDeadline(context.Background(), time.Unix(1, 0)) + defer cancel() + _, err = client.Generate(ctx, domain.GenerateRequest{ + Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: "hi"}}}, + }) + if !errors.Is(err, ErrRequestFailed) || !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected request failure and caller deadline identities, got %v", err) + } +} + +func TestOpenAICompatibleClientWholeRequestTimeoutReturnsRequestFailure(t *testing.T) { + client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ + BaseURL: "http://example.com/v1", + Model: "m", + HTTPClient: &http.Client{ + Timeout: time.Millisecond, + Transport: waitingContextTransport{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + _, err = client.Generate(context.Background(), domain.GenerateRequest{ + Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: "hi"}}}, + }) + if !errors.Is(err, ErrRequestFailed) || !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected request failure and client timeout identities, got %v", err) + } +} + +func TestOpenAICompatibleClientTransportFailurePreservesCauseWithoutSensitiveText(t *testing.T) { + transportCause := errors.New("transport diagnostic") + const ( + endpoint = "http://sensitive-endpoint.example/private" + apiKey = "sensitive-api-key" + content = "sensitive prompt content" + ) + client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ + BaseURL: endpoint, + Model: "m", + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return nil, transportCause + })}, + }) + if err != nil { + t.Fatal(err) + } + + _, err = client.Generate(context.Background(), domain.GenerateRequest{ + Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: content}}}, + Target: domain.ExecutionTarget{APIKey: apiKey}, + }) + if !errors.Is(err, ErrRequestFailed) || !errors.Is(err, transportCause) { + t.Fatalf("expected request failure and transport cause identities, got %v", err) + } + var requestErr *url.Error + if !errors.As(err, &requestErr) { + t.Fatalf("expected underlying http.Client.Do URL error, got %T: %v", err, err) + } + for _, sensitive := range []string{endpoint, "sensitive-endpoint.example", apiKey, content, transportCause.Error()} { + if strings.Contains(err.Error(), sensitive) { + t.Fatalf("transport error exposed %q: %v", sensitive, err) + } } } @@ -1170,23 +1277,38 @@ func TestOpenAICompatibleClientRejectsInvalidExecutionSettings(t *testing.T) { } func TestOpenAICompatibleClientAllowsEmptyConfiguredBaseURL(t *testing.T) { + var selectedURL string client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ BaseURL: "", Model: "m", + HTTPClient: &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + selectedURL = req.URL.String() + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader( + `{"choices":[{"message":{"content":"request endpoint"}}]}`, + )), + Request: req, + }, nil + })}, }) if err != nil { t.Fatalf("expected empty configured base URL to be allowed, got %v", err) } - _, err = client.Generate(context.Background(), domain.GenerateRequest{ + response, err := client.Generate(context.Background(), domain.GenerateRequest{ Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: "hi"}}}, - Target: domain.ExecutionTarget{Endpoint: "http://localhost:9999/v1"}, + Target: domain.ExecutionTarget{Endpoint: "http://request-endpoint.example/v1"}, }) - if err == nil { - t.Fatal("expected request failure due to unreachable endpoint") + if err != nil { + t.Fatalf("generate with request endpoint: %v", err) } - if !errors.Is(err, ErrRequestFailed) { - t.Fatalf("expected ErrRequestFailed with request endpoint override, got %v", err) + if response.Content != "request endpoint" { + t.Fatalf("response content = %q, want request endpoint", response.Content) + } + if selectedURL != "http://request-endpoint.example/v1/chat/completions" { + t.Fatalf("selected URL = %q", selectedURL) } }