Skip to content

refactor!: confine reqwest to one transport module; auth flows go through HttpClientExt #2396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gold-silver-copper merged 1 commit into from
Aug 21, 2026
Merged

refactor!: confine reqwest to one transport module; auth flows go through HttpClientExt #2396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions MIGRATING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,43 @@ handed back a silently short list.

## 0.41 → next

### `AuthError::Http` carries `http_client::Error`, and `Authenticator::auth_context` takes the transport

`rig::providers::copilot::auth::AuthError` / `rig::providers::chatgpt::auth::AuthError`
(`providers::internal::auth::AuthError`) wrapped a raw `reqwest::Error`. The
OAuth/device-code flows now run through the client's own `HttpClientExt`
transport instead of an ad-hoc `reqwest::Client`, so the variant carries the
transport-agnostic `http_client::Error`: a non-success response is one of its
status-bearing variants, a response-less failure is `Instance`.

```rust
// before
Err(AuthError::Http(e)) => e.status()

// after
Err(AuthError::Http(e)) => match e {
http_client::Error::InvalidStatusCode(status)
| http_client::Error::InvalidStatusCodeWithMessage(status, _)
| http_client::Error::InvalidStatusCodeWithDetails { status, .. } => Some(status),
_ => None,
}
```

`Authenticator::auth_context()` accordingly takes the transport to use:
`auth.auth_context(client.http_client()).await`. The provider clients'
`authorize()` helpers are unchanged. `Client::http_client()` is new: it
borrows the transport a `Client<Ext, H>` sends through.

### `http_client::ReqwestClient` / `from_reqwest` live in a reqwest-only module

Both still resolve at `rig::http_client::ReqwestClient` and
`rig::http_client::from_reqwest`; they are re-exported from the bundled
reqwest transport module, which is now the only place rig-core names a reqwest
type. `http_client::Error::non_success_with_details(status, headers, body)` is
the new transport-agnostic constructor for the headers-preserving non-success
error — custom `HttpClientExt` implementations should build their errors with
it so `non_success_headers()` keeps working for retry policies.

### `VectorStoreError::ReqwestError` is now `VectorStoreError::Http(http_client::Error)`

The variant carried a raw `reqwest::Error`, which tied rig-core's public
Expand Down
9 changes: 8 additions & 1 deletion crates/rig-core/src/client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ pub(crate) use impl_provider_client;
/// `new` is pinned to `H = reqwest::Client` so the call site infers without an explicit `H`
/// annotation. Callers who want a different backend should go through [`Client::builder`] and
/// chain [`ClientBuilder::http_client`] before [`ClientBuilder::build`].
// bevy-prep: this reqwest-pinned construction surface (together with `builder()`'s inference
// This reqwest-pinned construction surface (together with `builder()`'s inference
// anchor and `ClientBuilder::build`'s default backend) relocates to the `rig` facade in the
// transport-crate split.
impl<Ext> Client<Ext, reqwest::Client>
Expand Down Expand Up @@ -524,6 +524,13 @@ impl<Ext, H> Client<Ext, H> {
&self.ext
}

/// The HTTP transport this client sends through, for callers that must
/// talk to an absolute URL outside the provider's API base (OAuth/device
/// flows) with the same transport.
pub fn http_client(&self) -> &H {
&self.http_client
}

/// Reuse this client's base URL, headers, and HTTP backend with a different extension.
pub fn with_ext<NewExt>(self, new_ext: NewExt) -> Client<NewExt, H> {
Client {
Expand Down
223 changes: 15 additions & 208 deletions crates/rig-core/src/http_client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ use crate::http_client::sse::BoxedStream;
use bytes::Bytes;
pub use http::{HeaderMap, HeaderValue, Method, Request, Response, Uri, request::Builder};
use http::{HeaderName, StatusCode};
use reqwest::Body;
pub mod multipart;
mod reqwest_transport;
pub mod retry;
pub mod sse;
use crate::wasm_compat::*;
pub use multipart::MultipartForm;
pub use reqwest::Client as ReqwestClient;
use std::pin::Pin;
pub use reqwest_transport::{ReqwestClient, from_reqwest};

#[derive(Debug, thiserror::Error)]
pub enum Error {
Expand Down Expand Up @@ -68,6 +67,18 @@ impl Error {
}
}

/// Build the headers-preserving non-success error from a failed
/// response's parts. Transports call this with the status, headers and
/// body they read off the wire, so provider layers can recover transport
/// metadata — request ids, rate-limit headers — from the error (rig#2314).
pub fn non_success_with_details(status: StatusCode, headers: HeaderMap, body: String) -> Self {
Self::InvalidStatusCodeWithDetails {
status,
body,
headers: Box::new(headers),
}
}

/// Returns the failed response's headers, when this error preserved them.
///
/// Rig's bundled HTTP clients capture the full [`HeaderMap`] whenever a
Expand Down Expand Up @@ -110,27 +121,10 @@ pub(crate) fn instance_error<E: std::error::Error + Send + Sync + 'static>(error
}

#[cfg(target_family = "wasm")]
fn instance_error<E: std::error::Error + 'static>(error: E) -> Error {
pub(crate) fn instance_error<E: std::error::Error + 'static>(error: E) -> Error {
Error::Instance(error.into())
}

async fn non_success_status_error(response: reqwest::Response) -> Error {
let status = response.status();
// Preserve the failed response's headers: provider layers read their
// request-id contract off them (rig#2314). The Display is identical to
// the header-less variant, so surfaced error text is unchanged.
let headers = Box::new(response.headers().clone());
let body = response
.text()
.await
.unwrap_or_else(|error| format!("failed to read error response body: {error}"));
Error::InvalidStatusCodeWithDetails {
status,
body,
headers,
}
}

pub type LazyBytes = WasmBoxedFuture<'static, Result<Bytes>>;
pub type LazyBody<T> = WasmBoxedFuture<'static, Result<T>>;

Expand All @@ -145,27 +139,6 @@ impl From<NoBody> for Bytes {
}
}

impl From<NoBody> for Body {
fn from(_: NoBody) -> Self {
reqwest::Body::default()
}
}

/// Map a transport-level `reqwest::Error` onto the transport-agnostic
/// [`Error`].
///
/// A failure that carries a status (an `error_for_status` rejection) keeps
/// it as [`Error::InvalidStatusCode`] so provider retry and error-inspection
/// paths can still read the code; a response-less failure (connect, decode,
/// timeout) becomes [`Error::Instance`].
// bevy-prep: moves to `rig-reqwest` in the transport-crate split.
pub fn from_reqwest(err: reqwest::Error) -> Error {
match err.status() {
Some(status) => Error::InvalidStatusCode(status),
None => Error::Instance(Box::new(err)),
}
}

pub async fn text(response: Response<LazyBody<Vec<u8>>>) -> Result<String> {
let text = response.into_body().await?;
Ok(String::from(String::from_utf8_lossy(&text)))
Expand Down Expand Up @@ -217,176 +190,10 @@ pub trait HttpClientExt: WasmCompatSend + WasmCompatSync {
T: Into<Bytes> + WasmCompatSend;
}

async fn into_lazy_response<U>(response: reqwest::Response) -> Result<Response<LazyBody<U>>>
where
U: From<Bytes>,
U: WasmCompatSend + 'static,
{
if !response.status().is_success() {
return Err(non_success_status_error(response).await);
}

let mut res = Response::builder().status(response.status());

if let Some(headers) = res.headers_mut() {
*headers = response.headers().clone();
}

let body: LazyBody<U> = Box::pin(async {
let bytes = response.bytes().await.map_err(instance_error)?;
Ok(U::from(bytes))
});

res.body(body).map_err(Error::Protocol)
}

macro_rules! impl_http_client_ext {
($(#[$attribute:meta])* $client:ty) => {
$(#[$attribute])*
impl HttpClientExt for $client {
fn send<T, U>(
&self,
req: Request<T>,
) -> impl Future<Output = Result<Response<LazyBody<U>>>> + WasmCompatSend + 'static
where
T: Into<Bytes>,
U: From<Bytes> + WasmCompatSend + 'static,
{
let (parts, body) = req.into_parts();
let req = self
.request(parts.method, parts.uri.to_string())
.headers(parts.headers)
.body(body.into());

async move {
let response = req.send().await.map_err(instance_error)?;
into_lazy_response(response).await
}
}

fn send_multipart<U>(
&self,
req: Request<MultipartForm>,
) -> impl Future<Output = Result<Response<LazyBody<U>>>> + WasmCompatSend + 'static
where
U: From<Bytes>,
U: WasmCompatSend + 'static,
{
let (parts, body) = req.into_parts();
let body = reqwest::multipart::Form::from(body);

let req = self
.request(parts.method, parts.uri.to_string())
.headers(parts.headers)
.multipart(body);

async move {
let response = req.send().await.map_err(instance_error)?;
into_lazy_response(response).await
}
}

fn send_streaming<T>(
&self,
req: Request<T>,
) -> impl Future<Output = Result<StreamingResponse>> + WasmCompatSend
where
T: Into<Bytes> + WasmCompatSend,
{
let (parts, body) = req.into_parts();

let client = self.clone();

async move {
let req = self
.request(parts.method, parts.uri.to_string())
.headers(parts.headers)
.body(body.into())
.build()
.map_err(|error| Error::Instance(error.into()))?;
let response: reqwest::Response =
client.execute(req).await.map_err(instance_error)?;
if !response.status().is_success() {
return Err(non_success_status_error(response).await);
}

#[cfg(not(target_family = "wasm"))]
let mut res = Response::builder()
.status(response.status())
.version(response.version());

#[cfg(target_family = "wasm")]
let mut res = Response::builder().status(response.status());

if let Some(hs) = res.headers_mut() {
*hs = response.headers().clone();
}

use futures::StreamExt;

let mapped_stream: Pin<
Box<dyn WasmCompatSendStream<InnerItem = Result<Bytes>>>,
> = Box::pin(
response
.bytes_stream()
.map(|chunk| chunk.map_err(|e| Error::Instance(Box::new(e)))),
);

res.body(mapped_stream).map_err(Error::Protocol)
}
}
}
};
}

impl_http_client_ext!(reqwest::Client);

impl_http_client_ext!(
#[cfg(feature = "reqwest-middleware")]
#[cfg_attr(docsrs, doc(cfg(feature = "reqwest-middleware")))]
reqwest_middleware::ClientWithMiddleware
);

#[cfg(test)]
mod non_success_header_tests {
use super::*;

/// rig#2210: the bundled transport's own error constructor is where the
/// headers are captured, so drive it with a real `reqwest::Response`.
#[tokio::test]
async fn non_success_status_error_preserves_response_headers() {
let response = http::Response::builder()
.status(StatusCode::TOO_MANY_REQUESTS)
.header("retry-after", "20")
.header("x-ratelimit-remaining", "0")
.body(r#"{"error":{"message":"rate limited"}}"#)
.expect("valid response");

let error = non_success_status_error(reqwest::Response::from(response)).await;

assert_eq!(
error.non_success_status(),
Some(StatusCode::TOO_MANY_REQUESTS)
);
assert_eq!(
error.non_success_body(),
Some(r#"{"error":{"message":"rate limited"}}"#)
);
let headers = error
.non_success_headers()
.expect("headers captured at error construction");
assert_eq!(
headers.get("retry-after").and_then(|v| v.to_str().ok()),
Some("20")
);
assert_eq!(
headers
.get("x-ratelimit-remaining")
.and_then(|v| v.to_str().ok()),
Some("0")
);
}

/// `None` means "not captured" and must not be confused with an empty map:
/// every other shape of this error reports it.
#[test]
Expand Down
Loading
Loading