Keep the request URL out of transport errors and document fixture provenance

A base URL path may hold the API key, and net/http's *url.Error names the full request URL. Transport failures now keep only the wrapped cause, with the key redacted, and unwrap only to llm.ErrTimeout or context.Canceled. The request-build error no longer echoes the URL either. A new test fails the exchange (connection refused and connection dropped) against a base URL whose path holds the key. testdata/README.md records each fixture's source in the vendor's public API documentation (Ollama: its openai middleware source), how it was sanitized, and which fields are not in the public example. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 16:33:41 -04:00
parent 1f4356cd17
commit 80d91eba70
3 changed files with 155 additions and 10 deletions
@@ -330,7 +330,8 @@ func (p *Provider) do(ctx context.Context, method, path string, payload []byte)
 	}
 	httpReq, err := http.NewRequestWithContext(ctx, method, p.base+path, reqBody)
 	if err != nil {
-		return nil, 0, fmt.Errorf("build request: %w", err)
+		// The parse error names the URL, whose path may hold the API key.
+		return nil, 0, errors.New("build request: request URL is invalid")
 	}
 	for name, values := range p.headers {
 		httpReq.Header[name] = values
@@ -345,20 +346,20 @@ func (p *Provider) do(ctx context.Context, method, path string, payload []byte)

 	httpResp, err := p.client.Do(httpReq)
 	if err != nil {
-		return nil, 0, transportError(err)
+		return nil, 0, p.transportError(err)
 	}
 	defer httpResp.Body.Close()

 	if httpResp.StatusCode < 200 || httpResp.StatusCode > 299 {
 		body, err := io.ReadAll(io.LimitReader(httpResp.Body, maxErrorBody))
 		if err != nil {
-			return nil, httpResp.StatusCode, transportError(err)
+			return nil, httpResp.StatusCode, p.transportError(err)
 		}
 		return nil, httpResp.StatusCode, p.statusError(httpResp, body)
 	}
 	body, err := io.ReadAll(io.LimitReader(httpResp.Body, maxResponseBody+1))
 	if err != nil {
-		return nil, httpResp.StatusCode, transportError(err)
+		return nil, httpResp.StatusCode, p.transportError(err)
 	}
 	if len(body) > maxResponseBody {
 		return nil, httpResp.StatusCode, fmt.Errorf("response body exceeds %d bytes", maxResponseBody)
@@ -405,11 +406,41 @@ func isTimeout(err error) bool {
 	return errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &ne) && ne.Timeout())
 }

-// transportError reports a failure to complete the HTTP exchange. The
-// underlying error names the URL, never the request headers.
-func transportError(err error) error {
-	if isTimeout(err) {
-		return fmt.Errorf("%w: %w", llm.ErrTimeout, err)
+// transportFailure is a failed HTTP exchange. It carries the cause's text
+// with the request URL removed and the API key redacted, and unwraps only to
+// the cause's category: llm.ErrTimeout, context.Canceled or nothing.
+type transportFailure struct {
+	detail string
+	kind   error
+}
+
+func (e *transportFailure) Error() string {
+	if e.kind == llm.ErrTimeout {
+		return fmt.Sprintf("%s: transport: %s", llm.ErrTimeout, e.detail)
 	}
-	return err
+	return "transport: " + e.detail
+}
+
+func (e *transportFailure) Unwrap() error { return e.kind }
+
+// transportError reports a failure to complete the HTTP exchange. A
+// *url.Error names the request URL, and a base URL path may hold the API
+// key, so only the wrapped cause is kept, rendered with the key redacted.
+func (p *Provider) transportError(err error) error {
+	var kind error
+	switch {
+	case isTimeout(err):
+		kind = llm.ErrTimeout
+	case errors.Is(err, context.Canceled):
+		kind = context.Canceled
+	}
+	var ue *url.Error
+	if errors.As(err, &ue) {
+		err = ue.Err
+	}
+	detail := err.Error()
+	if p.apiKey != "" {
+		detail = strings.ReplaceAll(detail, p.apiKey, "[REDACTED]")
+	}
+	return &transportFailure{detail: detail, kind: kind}
 }
@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -498,6 +499,48 @@ func TestTransportErrorHasNoSecret(t *testing.T) {
 	assertNoSecret(t, err)
 }

+// TestTransportErrorWithKeyInBaseURLPath fails the exchange against a base
+// URL whose path holds the key, which New accepts; net/http names the full
+// request URL in its transport errors.
+func TestTransportErrorWithKeyInBaseURLPath(t *testing.T) {
+	refused := newVendorServer(t, reply{status: 200})
+	refusedBase := refused.URL
+	refused.Close()
+
+	dropped := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		conn, _, err := w.(http.Hijacker).Hijack()
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		conn.Close()
+	}))
+	t.Cleanup(dropped.Close)
+
+	for name, base := range map[string]string{
+		"connection refused": refusedBase + "/v1/" + testKey,
+		"connection dropped": dropped.URL + "/v1/" + testKey,
+	} {
+		p := newProvider(t, base, JSONSchema, nil)
+		_, genErr := p.Generate(context.Background(), schemaRequest())
+		_, listErr := p.ListModels(context.Background())
+		verifyErr := p.Verify(context.Background())
+		for _, err := range []error{genErr, listErr, verifyErr} {
+			if err == nil {
+				t.Fatalf("%s: nil error", name)
+			}
+			assertNoSecret(t, err)
+			var ue *url.Error
+			if errors.As(err, &ue) {
+				t.Fatalf("%s: error chain exposes the request URL: %v", name, ue)
+			}
+			if errors.Is(err, llm.ErrTimeout) || errors.Is(err, llm.ErrProvider) {
+				t.Fatalf("%s: err = %v, want an untyped transport failure", name, err)
+			}
+		}
+	}
+}
+
 // TestKeyedErrorsNeverLeakKey drives every error path of the keyed OpenAI
 // and OpenRouter shapes with vendor bodies that echo the key.
 func TestKeyedErrorsNeverLeakKey(t *testing.T) {
@@ -0,0 +1,71 @@
+# OpenAI-compatible adapter fixtures
+
+These are the vendor response bodies the contract tests in `openai_test.go`
+replay. They are **not live captures**. Product AI is tested with fakes, and
+live vendor calls are opt-in and never use credentials, so no request was
+sent to a vendor to make them. Each fixture is adapted from the response
+shape in that vendor's public API documentation or open source, then
+sanitized:
+
+- IDs, timestamps, fingerprints and token counts are illustrative values in
+  the documented format, not values from a real account.
+- Message content is replaced with MealQuest dinner text.
+- No real credential appears anywhere. `{{KEY}}` is a placeholder the tests
+  replace with a fake key, to prove the adapter redacts a key echoed back in
+  a vendor body.
+
+The tests check the adapter against these bodies with independent
+assertions on both the request sent (path, headers, `response_format`,
+messages) and the response decoded. The request side does not depend on the
+fixtures.
+
+Below, *not in the public example* marks fields or values that were added
+for a test case and do not come from the vendor's documented example.
+
+## OpenAI
+
+Sources: API reference, "The chat completion object" and "List models"
+(<https://platform.openai.com/docs/api-reference/chat/object>,
+<https://platform.openai.com/docs/api-reference/models/list>); the
+Structured Outputs guide, for `refusal`
+(<https://platform.openai.com/docs/guides/structured-outputs>); and the error
+codes guide (<https://platform.openai.com/docs/guides/error-codes>).
+
+| Fixture | Shape | Not in the public example |
+| --- | --- | --- |
+| `openai_chat_json_schema.json` | Chat completion object with `usage` details, `service_tier`, `system_fingerprint`. | The JSON reply content. |
+| `openai_chat_refusal.json` | Structured Outputs refusal: `content: null`, `refusal` set. | — |
+| `openai_models.json` | `{object: list, data: [model]}`. | — |
+| `openai_error_invalid_key.json` | Error object, `code: invalid_api_key`, message echoing the key. | The full key in the message; OpenAI masks most of it. Kept whole to test redaction. |
+| `openai_error_server.json` | Error object, `type: server_error`. | The `Authorization: Bearer {{KEY}}` text; added to test redaction. |
+
+## OpenRouter
+
+Sources: API reference, chat completion response and errors
+(<https://openrouter.ai/docs/api-reference/overview>,
+<https://openrouter.ai/docs/api-reference/errors>), and "List available
+models" (<https://openrouter.ai/docs/api-reference/list-available-models>).
+
+| Fixture | Shape | Not in the public example |
+| --- | --- | --- |
+| `openrouter_chat_json_object.json` | Chat completion with `provider`, `native_finish_reason` and `message.reasoning`. | The JSON reply content. |
+| `openrouter_chat_upstream_error.json` | An `error` object (`code`, `message`, `metadata.provider_name`, `metadata.raw`) in an HTTP 200 body, which OpenRouter documents for upstream failures. | `user_id`. |
+| `openrouter_error_rate_limited.json` | Error object, `code: 429`, rate-limit `metadata`. | The `Key {{KEY}}` text; added to test redaction. |
+| `openrouter_models.json` | `{data: [model]}` with `name`, `context_length`, `architecture`, `pricing`, `supported_parameters`. | The second model is a minimal entry, to test optional fields. |
+
+## Ollama
+
+Ollama's OpenAI compatibility page
+(<https://github.com/ollama/ollama/blob/main/docs/openai.md>) documents
+requests but shows no response bodies. The shapes come from its `openai`
+middleware source (<https://github.com/ollama/ollama/blob/main/openai/openai.go>):
+`chatcmpl-<n>` IDs, `system_fingerprint: fp_ollama`, `owned_by: library` on
+models, and errors as `{error: {message, type: api_error, param, code}}`.
+
+| Fixture | Shape | Not in the public example |
+| --- | --- | --- |
+| `ollama_chat_text.json` | Plain chat completion. | — |
+| `ollama_chat_prompt_only.json` | Chat completion. | The reply text: prose and stray braces around a fenced JSON object, to test prompt-only extraction. |
+| `ollama_chat_truncated.json` | Chat completion with `finish_reason: length`. | The cut-off JSON reply. |
+| `ollama_models.json` | `{object: list, data: [model]}`. | — |
+| `ollama_error_not_found.json` | Middleware error object for a missing model. | — |