fix(rust): apply client custom headers to the OAuth token request - #17296
fix(rust): apply client custom headers to the OAuth token request#17296fern-api[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
AI Review Summary
The core fix is right: build the token request and run it through apply_custom_headers before execute, with a clear rationale for excluding request-level and auth headers. Main risks are in the new unit test that now ships into every generated SDK: it depends on tokio's net/io-util features being enabled in the generated Cargo.toml, and the read loop can hang with no timeout. Comments target generators/rust/base/src/asIs/http_client.rs; the same points apply to all 178 regenerated seed copies.
- 🟡 2 warning(s)
- 🔵 2 suggestion(s)
| #[tokio::test] | ||
| async fn test_oauth_token_request_sends_custom_headers() { | ||
| let listener = tokio::net::TcpListener::bind("127.0.0.1:0") | ||
| .await | ||
| .expect("bind"); | ||
| let base_url = format!("http://{}", listener.local_addr().expect("addr")); | ||
| let server = tokio::spawn(serve_one_token_request(listener)); |
There was a problem hiding this comment.
🟡 warning
This test ships into every generated SDK, so it hard-depends on tokio being available at test time with the net, io-util, rt-multi-thread and macros features. If any generated Cargo.toml template pins tokio with a narrower feature set (or as a non-dev dependency only), cargo test breaks for all consumers. Given cargo test couldn't be run locally, please confirm the generated Cargo.toml feature list before merging — a compile failure here lands in 178 SDKs at once.
| loop { | ||
| let read = socket.read(&mut buffer).await.expect("read"); | ||
| raw.extend_from_slice(&buffer[..read]); | ||
| if read == 0 || String::from_utf8_lossy(&raw).contains("\r\n\r\n") { | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 warning
The read loop has no timeout: if a client ever sends headers without a terminating \r\n\r\n and keeps the connection open, this hangs forever and takes CI with it. Wrap the accept/read in tokio::time::timeout (or at least bound the loop iterations) so a failure surfaces as a test failure rather than a hung job.
| let raw_request = server.await.expect("server"); | ||
| assert_eq!(access_token, "token-from-server"); | ||
| assert!( | ||
| raw_request.contains("x-gateway-token: sunflower"), |
There was a problem hiding this comment.
🔵 suggestion
Asserting the exact lowercase wire form couples the test to hyper's header serialization. A case-insensitive check is more robust:
| raw_request.contains("x-gateway-token: sunflower"), | |
| raw_request.to_ascii_lowercase().contains("x-gateway-token: sunflower"), |
|
|
||
| let (access_token, _) = client | ||
| .fetch_oauth_token( | ||
| &client.config.base_url.clone(), |
There was a problem hiding this comment.
🔵 suggestion
&client.config.base_url.clone() allocates a String just to take a &str of it. &client.config.base_url works via deref coercion (and avoids a redundant_clone clippy hit).
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Summary
The Rust client-credentials token exchange was the only request in the generated SDK that bypassed
apply_custom_headers:fetch_oauth_tokenfired straight off the sharedreqwest::Client, while every other request goes throughsend_request→apply_auth_headers+apply_custom_headers. Any API whose gateway requires a header on its token endpoint therefore 401s on the exchange, and the failure surfaces on the first SDK call rather than at the token request:X-Fern-SDK-*telemetry headers were also missing from token requests for the same reason.Only config-level headers are applied. Request-level
additional_headersare deliberately excluded: the token is cached inOAuthTokenProviderand shared across requests, so it must not depend on the options of whichever request happens to trigger the fetch. Auth headers are excluded because this request is what produces the credential they would carry (and applying them would recurse into the token fetch).Changes
generators/rust/base/src/asIs/http_client.rs— build the token request and run it throughapply_custom_headersbefore executing; documented why request-level and auth headers stay out.test_oauth_token_request_sends_custom_headers): binds a one-shot loopback TCP server, drivesfetch_oauth_tokenagainst it, and asserts the raw wire bytes contain the client's custom header. No new dependencies (tokiois already a full-feature dep of generated SDKs).seed/rust-sdk/**/src/core/http_client.rs+ 36seed/cli/**/src/core/http_client.rs(the CLI generator embeds the Rust SDK's asIs files).fix).Test plan
pnpm seed test --generator rust-sdk --local --skip-scripts→ 142/142 passpnpm seed test --generator cli --local --skip-scripts→ 141/141 passcargo teston a Rust seed fixture is blocked in this environment (crates.io is not reachable), so the new unit test has not been executed locally — relying on CI for that.Reported by a customer whose gateway requires a custom auth header on the token endpoint; the generated SDK could not complete the OAuth exchange at all.