extensions middleware and request extensions #56
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
conradludgate
added
the
reqwest-middleware
| /// impl RequestInitialiser for AuthInit { | ||
| /// fn init(&self, req: RequestBuilder) -> RequestBuilder { | ||
| /// req.bearer_auth("my_auth_token") | ||
| /// } | ||
| /// } |
There was a problem hiding this comment.
This can already be done with existing middleware via Request::headers_mut, maybe we should use a more motivating example that's harder to do with the Request struct?
There was a problem hiding this comment.
I can probably elaborate more - this is a client wide setting. Imagine you have a client that has many endpoints but all use the same auth settings, then those can be define in a RequestInitialiser on the top level connection
| /// impl Middleware for LoggingMiddleware { | ||
| /// async fn handle( | ||
| /// &self, | ||
| /// req: Request, | ||
| /// extensions: &mut Extensions, | ||
| /// next: Next<'_>, | ||
| /// ) -> Result<Response> { | ||
| /// // get the log name or default to "unknown" | ||
| /// let name = extensions | ||
| /// .get() | ||
| /// .map(|&LogName(name)| name) | ||
| /// .unwrap_or("unknown"); | ||
| /// println!("[{name}] Request started {req:?}"); | ||
| /// let res = next.run(req, extensions).await; | ||
| /// println!("[{name}] Result: {res:?}"); | ||
| /// res | ||
| /// } | ||
| /// } | ||
| /// | ||
| /// async fn run() { | ||
| /// let reqwest_client = Client::builder().build().unwrap(); | ||
| /// let client = ClientBuilder::new(reqwest_client) | ||
| /// .with_init(Extension(LogName("my-client"))) | ||
| /// .with(LoggingMiddleware) | ||
| /// .build(); |
There was a problem hiding this comment.
In this example couldn't the LogName have been baked into LoggingMiddleware. Or if the latter was a shared thing, a wrapper middleware used. I'm not seeing the motivating case for the new abstractions from this example very clearly.
There was a problem hiding this comment.
Ah, I thought I added the extra example. The case is where you might have a client wide extension, and then you can have a per-request configuration too.
let client = ClientBuilder::new(reqwest_client)
.with_init(Extension(LogName("my-client")))
.with(LoggingMiddleware)
.build();
// uses "my-client" as the log name
client.get("https://truelayer.com").send().await
// uses "create-payment" as the log name
client.post("https://api.truelayer.com/payment")
.with_extension(LogName("create-payment"))
.send()
.awaitThere was a problem hiding this comment.
And this is something I want to supplement #54 because I'm not keen on breaking the current SpanBackend trait to add &self just yet, but it does have access to the extensions.
The desired configuration by the author was to have client wide configuration of the span name, but I think a request specific span name might be more desirable in some situations. So this is the best of both worlds for configuration
Co-authored-by: Alex Butler <alexheretic@gmail.com>
A key difference is that |
As suggested in #54 (comment)
Adds a set of
ExtensionstoRequestBuilderso that requests can have some default extensions provided.Adds a new way to configure requests:
RequestInitialiser- it's a simple sync trait that just applies basic settings toRequestBuilder.Adds a
ExtensionRequestInitialiser which callsinsertduring the init chain