search

http-rs / surf

Fast and friendly HTTP client framework for async Rust
https://docs.rs/surf
Apache License 2.0
1.45k stars 119 forks source link

Allow user access to Request before it's sent off by RequestBuilder? #258

Closed aldanor closed 3 years ago

aldanor commented 3 years ago

This could greatly improve traceability and logging options (unless I'm missing something, in which case please correct me). Note that other libraries like reqwest suffer from this kind of problem too so it's not surf-specific.

Basically, right now there's RequestBuilder.build() which builds a Request, and there's RequestBuilder.send() which calls build() and instantly passes it on to client.send() (with client being private).

There's no way for a user to inspect the Request that's being sent off, e.g. for creating a tracing::span for it or logging/instrumenting it in some other way conveniently.

You could call .build() and then call .send() but that would build the request twice which is surely a waste. You could try and add middleware but it would be a bit of an overkill for e.g. a single simple request, and is not very ergonomic in trivial cases (closures can't be used? https://github.com/http-rs/surf/issues/82). Note also that middleware is per-client and not per-request which also complicates things.

Maybe something like this? RequestBuilder::inspect_and_send(|req| ...)? This would be ok for logging but this also doesn't play overly nice with automatic span tracing as the closure will exit before the request is actually sent off. Maybe some other ideas? It would be nice if this was actually documented too, as in "here's canonical way to trace requests on a per-request basis if needed".

Fishrock123 commented 3 years ago

We could do impl AsMut<Request> for RequestBuilder, but that doesn't allow chaining and it breaks ergonomics.

If you need to do this it probably indicates you should either:

Fishrock123 commented 3 years ago

I don't think there is anything more do do here since anything else is covered by https://github.com/http-rs/surf/issues/261

aldanor commented 3 years ago

Use a client with a middleware which does this.

Since you can't do this on per-request basis, it doesn't really solve this. You may have to clone a client just in order to attach middleware, and you can't currently easily use closures either. So may also have to write a custom type in order to inspect your request. Also, if there's anything you want to pull out of the request back into the scope from which it was sent, it will become even clunkier (again, since you can't use simple closures).

Construct the Request before sending

l I think this doesn't play too well with Client's base_url. IIUC, the Request you'll get from builder.build() will not be what actually gets sent. That's a separate topic, but may be worth looking into.

Fishrock123 commented 3 years ago

Since you can't do this on per-request basis, it doesn't really solve this.

Yep, hence the link to your other issue which i think better encapsulates anything the other two cases doesn't.

I think this doesn't play too well with Client's base_url. IIUC, the Request you'll get from builder.build() will not be what actually gets sent. That's a separate topic, but may be worth looking into.

Your understanding is incorrect - the URL is built immediately:

https://github.com/http-rs/surf/blob/8f0039488b3877ca59592900bc7ad645a83e2886/src/client.rs#L510-L539