Reqwest: Adding a set of Event Hooks for a Request

[seanmonstar]

There is desire sometimes to extend the functionality of reqwest at certain points in the request lifecycle, and having event hooks may a good solution to do so. For instance, #87 wants to be able to add some custom logging at different events, and something like NTLM could be supported be exposing some connect event.

We could use a trait, like Events, with default method implementations of doing nothing. That would allow adding new methods without breaking existing code.

An example:

struct Logger;

impl reqwest::Events for Logger {
    fn on_frobnicate(&self, frob: &Frobnicator) {
        info!("frobnicated, {}", frob.details());
    }
}

client.get("https://hyper.rs")
    .events(Logger);
    .send()?;

Some questions that need resolving:

• Should this be added on the Client, and the same instance used for all the requests and responses, or should they be added on a per Request basis?
• What events are useful, and what arguments should they receive? Care should be taken that we don't expose implementation details that would prevent us from changing internal things.

[lucab]

I think I have similar usecase here (adding custom auth headers to a request built internally in rust-tuf library), which I think would fit in this Event Hooks design if you allow the hook to mutate the request itself.

Providing some partial datapoints to your questions:

• those hooks are set on the Client, and they internally have some logic to apply different actions to specific requests
• pre-send event in my case, and enough arguments to introspect&mutate a request before sending it (common tasks would be setting headers and adding query parameters to url)

[theduke]

I think one should definitely be able to set hooks on the whole client.
Overwriting/setting on the request builder could be an additional option.o

Hooks I definitely want:

• pre_send(&self, &mut Request)

• on_result(&self, &Request, &Result<Response>)

Or alternatively:

• on_ok(&self, &Request, &Response)
• on_err(&self, &Request, &Error)

The Request is probably being consumed while sending right now?
I'd be able to live with the on_* hooks only receiving the Result/Response/Error, but that's a bit limiting.

For example, in debug mode I might want to print out the request url, headers and body along with the response only if the request failed.

The handler would either have to be Sync to allow &self, or implement Clone and be cloned for each request. I guess the right way to go here is to require Sync.

[echochamber]

Just want to make sure I understand, hooking into the process would just be for "read only" reasons right?

Exposing a hook which would allow a listener to edit the request object on the pre_send. That seems like it could be abused for some really surprising behavior.

[seanmonstar]

I don't know if it should be readonly or get mutable access. I don't have many use cases myself, but collecting them here would help determine what is needed or useful.

[theduke]

My primary use case would be to add authorization headers.

[KodrAus]

I like the idea of being able to log various happenings in reqwest that I couldn't necessarily reach otherwise. How would we expect this feature to interact with other features that span multiple requests like redirects?

I also agree with @echochamber with regards to mutability. I think it's really nice and convenient to be able to register hooks that manage cross-cutting concerns for all requests, but when you lose locality of mutability to what's being mutated then it becomes harder to reason about what's going on.

[leoschwarz]

Maybe mutable interjection in the request is a feature orthogonal to the fine grained logging/status reporting requested here, the later could be added at most places without increasing complexity too much. The mutability would serve more to provide a means to implement something like a "middleware" ("interceptor"/"rewriter" or whatever to call it) rewriting requests and responses (so it needs two access points). In that case maybe these concerns belong to a different issue?

The usefulness of such a middleware API would be that the alternative of wrapping the client and handling stuff like auth or mocking (#154) is not composable, so everyone has to write it once by hand, while with middlewares could be reused between crates. Downside is that it's probably rather hard to make a good API, because for example for mocking requests completely I'd like to be able to return a success response without any HTTP request having made at all.

[mattgathu]

What events are useful, and what arguments should they receive?

At the moment, it would be useful for me to know:

• when the client connects via a http proxy. (_when the connection to the proxy server is established_).
• when a target url has been resolved and the target ip address is known.

[mattias-p]

I'd like to have on_redirect(&self, &Result, &Response). I'd use it to log the value of the Location header. This kind of overlaps with #281. If both are implemented I'd only use one of them.

[ticky]

Aside from mocking which would definitely be useful, a generic API for intercepting and optionally interrupting and responding to a request would be useful for implementing, for example, a caching plugin.

[arcrose]

People's points about generic request interception and modification not being reqwest's responsibility make a lot of sense, and I find them pretty agreeable. Adding specific handlers just for a couple of cases (even to hyper rather than reqwest) might be a better fit. My personal want for a feature like this is to be able to do some work in between the establishing of a TLS connection (i.e. playing with the CONNECT request).

[ticky]

I’d be okay with this being Hyper’s responsibility, but we’d then require some way to gain access to the underlying Hyper instance, which I don’t believe reqwest currently exposes any such hooks.

[danieleades]

how do we feel about the middleware approach that Surf has used?