[object Object]

← back to Cli Printing Press

fix(cli): propagate --timeout to surf transport ResponseHeaderTimeout (#1212)

69cefbe9db5219d8c6e03e51e74f218a36c567f8 · 2026-05-13 17:58:40 -0400 · klatt42

* fix(cli): propagate --timeout to surf transport ResponseHeaderTimeout

The browser-impersonate transport (UsesBrowserHTTPTransport branch) sets
httpClient.Timeout = timeout but never touches the underlying *http.Transport
that surf builds, which has ResponseHeaderTimeout set to surf's package
default (10s in v1.0.199). That per-stage timeout caps how long the
client waits for the FIRST response byte regardless of the overall
Timeout, so slow-streaming endpoints (RAG queries, LLM completions,
server-rendered briefings) fail with "net/http: timeout awaiting response
headers" at the surf default no matter how the user-facing --timeout
is set.

Type-assert surfClient.GetTransport() to *enetxhttp.Transport and set
ResponseHeaderTimeout to the user-supplied timeout. Surf's transport is
*github.com/enetx/http.Transport (not stdlib *net/http.Transport — surf
uses an HTTP fork for impersonate features), so the assertion targets
the enetx package's Transport type. enetxhttp is added as a conditional
import in the UsesBrowserHTTPTransport branch only.

Verified locally:
- go test ./... — full suite green (TestGenerateBrowserChromeH3Transport
  in particular compiles the generated CLI with go build, confirming
  the import + assertion compile cleanly)
- TestBrowserTransport_OverridesResponseHeaderTimeout pins the emit
- TestNonBrowserTransport_DoesNotEmitOverride pins the conditional —
  plain transport CLIs do not emit the override or the enetxhttp import
- scripts/golden.sh verify — 17 cases pass (no golden fixtures use
  the browser-transport path, so no fixture diff)

Discovered during the rok-cockpit-cli pilot. The CLI hard-failed on
Cockpit's /api/briefings/request (a ~65s server-side LLM call) at ~41s
with the exact "timeout awaiting response headers" error; the
operator's --timeout flag had no effect because it never reached the
transport layer. With this fix the user's --timeout flows end-to-end.

* test(cli): assert ResponseHeaderTimeout override emits on H3 path too

Per Greptile observation on #1212. TestBrowserTransport_OverridesResponseHeaderTimeout
already pins the override on the default (H2) browser-impersonate path,
but the H3 variant flows through the same UsesBrowserHTTPTransport
template branch and would silently lose the override if a future
refactor split the branches. Extends TestGenerateBrowserChromeH3Transport
with three assertions:

  - enetxhttp import emits
  - surfClient.GetTransport().(*enetxhttp.Transport) cast emits
  - t.ResponseHeaderTimeout = timeout emits

Belt-and-suspenders coverage at zero runtime cost.

Files touched

Diff

commit 69cefbe9db5219d8c6e03e51e74f218a36c567f8
Author: klatt42 <134792614+klatt42@users.noreply.github.com>
Date:   Wed May 13 17:58:40 2026 -0400

    fix(cli): propagate --timeout to surf transport ResponseHeaderTimeout (#1212)
    
    * fix(cli): propagate --timeout to surf transport ResponseHeaderTimeout
    
    The browser-impersonate transport (UsesBrowserHTTPTransport branch) sets
    httpClient.Timeout = timeout but never touches the underlying *http.Transport
    that surf builds, which has ResponseHeaderTimeout set to surf's package
    default (10s in v1.0.199). That per-stage timeout caps how long the
    client waits for the FIRST response byte regardless of the overall
    Timeout, so slow-streaming endpoints (RAG queries, LLM completions,
    server-rendered briefings) fail with "net/http: timeout awaiting response
    headers" at the surf default no matter how the user-facing --timeout
    is set.
    
    Type-assert surfClient.GetTransport() to *enetxhttp.Transport and set
    ResponseHeaderTimeout to the user-supplied timeout. Surf's transport is
    *github.com/enetx/http.Transport (not stdlib *net/http.Transport — surf
    uses an HTTP fork for impersonate features), so the assertion targets
    the enetx package's Transport type. enetxhttp is added as a conditional
    import in the UsesBrowserHTTPTransport branch only.
    
    Verified locally:
    - go test ./... — full suite green (TestGenerateBrowserChromeH3Transport
      in particular compiles the generated CLI with go build, confirming
      the import + assertion compile cleanly)
    - TestBrowserTransport_OverridesResponseHeaderTimeout pins the emit
    - TestNonBrowserTransport_DoesNotEmitOverride pins the conditional —
      plain transport CLIs do not emit the override or the enetxhttp import
    - scripts/golden.sh verify — 17 cases pass (no golden fixtures use
      the browser-transport path, so no fixture diff)
    
    Discovered during the rok-cockpit-cli pilot. The CLI hard-failed on
    Cockpit's /api/briefings/request (a ~65s server-side LLM call) at ~41s
    with the exact "timeout awaiting response headers" error; the
    operator's --timeout flag had no effect because it never reached the
    transport layer. With this fix the user's --timeout flows end-to-end.
    
    * test(cli): assert ResponseHeaderTimeout override emits on H3 path too
    
    Per Greptile observation on #1212. TestBrowserTransport_OverridesResponseHeaderTimeout
    already pins the override on the default (H2) browser-impersonate path,
    but the H3 variant flows through the same UsesBrowserHTTPTransport
    template branch and would silently lose the override if a future
    refactor split the branches. Extends TestGenerateBrowserChromeH3Transport
    with three assertions:
    
      - enetxhttp import emits
      - surfClient.GetTransport().(*enetxhttp.Transport) cast emits
      - t.ResponseHeaderTimeout = timeout emits
    
    Belt-and-suspenders coverage at zero runtime cost.
---
 internal/generator/generator_test.go         | 12 ++++
 internal/generator/templates/client.go.tmpl  | 17 ++++++
 internal/generator/transport_timeout_test.go | 91 ++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 3c440b33..d311f1dc 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -1503,6 +1503,18 @@ func TestGenerateBrowserChromeH3Transport(t *testing.T) {
 	// competing default.
 	assert.NotContains(t, string(clientGo), `req.Header.Set("Accept",`)
 
+	// ResponseHeaderTimeout override must emit on the H3 path too —
+	// surf's per-stage timeout (10s default) caps any browser-impersonate
+	// transport regardless of the H2/H3 variant. Without these asserts a
+	// future refactor could silently strip the override from the H3
+	// branch and slow-streaming H3 endpoints would fail at surf's default
+	// with no test catching it. Mirrors the assertions in
+	// TestBrowserTransport_OverridesResponseHeaderTimeout (which exercises
+	// the H2 default).
+	assert.Contains(t, string(clientGo), `enetxhttp "github.com/enetx/http"`)
+	assert.Contains(t, string(clientGo), "surfClient.GetTransport().(*enetxhttp.Transport)")
+	assert.Contains(t, string(clientGo), "t.ResponseHeaderTimeout = timeout")
+
 	runGoCommand(t, outputDir, "mod", "tidy")
 	runGoCommand(t, outputDir, "test", "./internal/client")
 }
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index 59023d2d..3141ed7e 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -31,6 +31,7 @@ import (
 	"time"
 
 {{- if .UsesBrowserHTTPTransport}}
+	enetxhttp "github.com/enetx/http"
 	"github.com/enetx/surf"
 {{ end}}
 	"{{modulePath}}/internal/cliutil"
@@ -287,6 +288,22 @@ func newHTTPClient(timeout time.Duration, jar http.CookieJar) *http.Client {
 		builder = builder.Session()
 	}
 	surfClient := builder.Build().Unwrap()
+	// Surf's underlying *http.Transport sets ResponseHeaderTimeout to
+	// its package default (10s in surf v1.x), which caps how long we
+	// wait for the FIRST response byte independent of the overall
+	// client Timeout. Slow-streaming endpoints (RAG queries, LLM
+	// completions, server-side rendering) routinely take longer than
+	// that to emit headers; without this override the user-facing
+	// --timeout flag has no effect on transport-layer cutoffs and
+	// requests fail with "net/http: timeout awaiting response headers"
+	// at the package default regardless of --timeout. Override so the
+	// transport-layer ceiling tracks the user's intent.
+	// surf's GetTransport returns *enetxhttp.Transport (surf's HTTP fork
+	// used for browser-impersonate features), NOT *net/http.Transport,
+	// so the assertion targets the enetx package's Transport type.
+	if t, ok := surfClient.GetTransport().(*enetxhttp.Transport); ok {
+		t.ResponseHeaderTimeout = timeout
+	}
 	httpClient := surfClient.Std()
 	httpClient.Timeout = timeout
 	if jar != nil {
diff --git a/internal/generator/transport_timeout_test.go b/internal/generator/transport_timeout_test.go
new file mode 100644
index 00000000..08431af5
--- /dev/null
+++ b/internal/generator/transport_timeout_test.go
@@ -0,0 +1,91 @@
+package generator
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/mvanhorn/cli-printing-press/v4/internal/naming"
+	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+	"github.com/stretchr/testify/require"
+)
+
+// TestBrowserTransport_OverridesResponseHeaderTimeout asserts the generator
+// emits the surf transport's ResponseHeaderTimeout override in every CLI
+// that uses the browser-impersonate transport (SpecSource="sniffed" triggers
+// it). Without the override, the user-facing --timeout flag flows into
+// httpClient.Timeout but never reaches the underlying *http.Transport's
+// per-stage ResponseHeaderTimeout, which surf sets to its 10s package
+// default. Slow-streaming endpoints (RAG queries, LLM completions) fail
+// with "net/http: timeout awaiting response headers" at the surf default
+// regardless of how --timeout is set.
+//
+// This canary asserts the structural fix: surfClient.GetTransport() is
+// type-asserted to *http.Transport and ResponseHeaderTimeout is set to
+// the requested timeout, before the wrapping Std() client is built.
+func TestBrowserTransport_OverridesResponseHeaderTimeout(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := &spec.APISpec{
+		Name:       "transport-timeout-canary",
+		Version:    "0.1.0",
+		BaseURL:    "https://www.example.com",
+		SpecSource: "sniffed", // triggers UsesBrowserHTTPTransport
+		Owner:      "test-owner",
+		OwnerName:  "Test Author",
+		Auth:       spec.AuthConfig{Type: "none"},
+		Config: spec.ConfigSpec{
+			Format: "toml",
+			Path:   "~/.config/transport-timeout-canary-pp-cli/config.toml",
+		},
+		Resources: map[string]spec.Resource{
+			"posts": {
+				Description: "Browse posts",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/", Description: "List posts"},
+				},
+			},
+		},
+	}
+	outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	clientSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
+	require.NoError(t, err)
+	src := string(clientSrc)
+
+	// Sanity: the test fixture must actually exercise the browser path.
+	require.Contains(t, src, "Impersonate()",
+		"test fixture must trigger UsesBrowserHTTPTransport — Impersonate() should be in the emitted client")
+
+	require.Contains(t, src, `enetxhttp "github.com/enetx/http"`,
+		"client.go must import enetx/http aliased so the transport type assertion has a name")
+	require.Contains(t, src, "surfClient.GetTransport().(*enetxhttp.Transport)",
+		"surfClient.GetTransport() must be type-asserted to *enetxhttp.Transport — surf returns the enetx HTTP fork's RoundTripper, not stdlib's, so a stdlib type-assertion is impossible")
+	require.Contains(t, src, "t.ResponseHeaderTimeout = timeout",
+		"ResponseHeaderTimeout must be set to the user-supplied timeout so --timeout reaches the transport layer")
+}
+
+// TestNonBrowserTransport_DoesNotEmitOverride asserts the override only
+// fires inside the browser-transport branch. Vanilla *http.Client CLIs
+// already honor --timeout via http.Client.Timeout — they have no surf
+// middleware overriding ResponseHeaderTimeout, so emitting the override
+// there would be dead code (and would fail compilation since the surf
+// package isn't imported in non-browser branches).
+func TestNonBrowserTransport_DoesNotEmitOverride(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("plain-transport-canary")
+	// Default SpecSource ("") does NOT trigger UsesBrowserHTTPTransport.
+	outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	clientSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
+	require.NoError(t, err)
+	src := string(clientSrc)
+
+	require.NotContains(t, src, "Impersonate()",
+		"sanity: plain transport CLIs must not emit Impersonate()")
+	require.NotContains(t, src, "ResponseHeaderTimeout",
+		"plain transport CLIs must not emit the surf-specific ResponseHeaderTimeout override")
+}

← 3819b67f fix(cli): support $-prefixed pagination params for Socrata-s  ·  back to Cli Printing Press  ·  ci: exempt release-please PRs from Greptile review requireme 3b283e04 →