Per-request middleware? (in RequestBuilder)

[aldanor]

(somewhat related to #259)

Would be nice to be able to do something like request_builder.with(my_middleware) which would replace the client it holds with a version that has the given middleware pushed on the stack (using the default shared client as the base if needed).

This would also make it possible to apply middleware to the default shared client on a per-request basis (which is arguably not the most efficient way to do so, but still):

surf::get("http://...").with(...)

[Fishrock123]

I think this is unlikely to happen - see https://github.com/Fishrock123/surf/pull/1, or rather, the discussion around it, since yes, it is technically possible even if less-than-pleasant.

Since clients share underlying resources anyways, you should probably just clone your client, or use one if you aren't.

[Fishrock123]

This would also make it possible to apply middleware to the default shared client on a per-request basis (which is arguably not the most efficient way to do so, but still):

surf::get("http://...").with(...)

If you are doing advanced usage already, you should probably not do this. It will always be less efficient somehow (even if that is just creating new memory for middleware to sit in). IMO, we should recommend using clients, and improve that story as much as possible.

[aldanor]

@Fishrock123 Thanks for taking time to answer!

I agree that promoting client usage as much as possible is a good idea (although it's currently not clear from the docs, and the first block of code you see in the readme uses surf::get as well). Although there will always be cases where you don't really care much about performance and prefer simpler API, I guess (like in the examples in the readme).

One example of a use case you've asked for would be more or less like this: you have a single client that holds connection pool, and your application makes multiple types/group of requests - e.g. A, B and C - where, say, you want to log requests A, and you also want to validate and log requests of type C in a different way, and requests B you don't care about. This would currently require you to either:

• Write one middleware that's supposed to know "everything about everyone"; i.e. in the example above, A, B and C. This doesn't scale too well as you keep extending the logic and the number of request types and groups.
• Clone the client every time before making a request? Just for the sake being use a different middleware with it? It's not obvious how cheap or expensive it is cloning a client and what does it do behind the curtain for each of the client types; at least it's not clearly documented. Either way, it seems like a bit of overkill, especially if you make many requests.
• Hold multiple clients, or clones, for each of request types/groups - this is possible, but gets quite clunky as well, making you write things like "get the client I'm using for request of type A, attach a middleware of type A to it, and send request A". As opposed to "form a request A, attach middleware of type A, send".

[Fishrock123]

Hold multiple clients, or clones, for each of request types/groups - this is possible, but gets quite clunky as well, making you write things like "get the client I'm using for request of type A, attach a middleware of type A to it, and send request A". As opposed to "form a request A, attach middleware of type A, send".

This will always be the best option, fwiw, even if per-RequestBuilder middleware becomes a thing.

I do something like this for multiple base urls all over the place, e.g. in this code.

Are you saying you have somewhere where you'd need different middleware for every endpoint? Or more like GET vs POST?

[aldanor]

Are you saying you have somewhere where you'd need different middleware for every endpoint? Or more like GET vs POST?

Neither really - say, there's a few dozen endpoints with the same base url, which form a number of groups that have to be handled differently middleware-wise (say, for example, half of them need to be logged, half of them need to be validated, half of them need to be serialized, etc - and these halves may overlap). Am I supposed to hold a few dozen clients and match them? This gets very clunky very fast.

Another point is - disallowing per-request middleware makes it hard passing the RequestBuilder around in case your requests are built in multiple stages and not inside one function which has the client in scope, in case you also want endpoint-dependent middleware. There's with_client but it's private.

[Fishrock123]

Another point is - disallowing per-request middleware makes it hard passing the RequestBuilder around in case your requests are built in multiple stages and not inside one function which has the client in scope, in case you also want endpoint-dependent middleware. There's with_client but it's private.

Oof that's a fear I had of keeping client::{method} in 2.0.0. Uh, if you are doing this it would seem to me you should build the request and send a Request around, and later send it from the client you want to send it from?

[aldanor]

Uh, if you are doing this it would seem to me you should build the request and send a Request around, and later send it from the client you want to send it from?

Generally - yes, agreed; but not always :) Since RequestBuilder holds the client in it, so you can pass it around and alter it as you please and then eventually send the request off without having to have the access to the client(s). So, e.g. if the request builder could attach middleware per request, the middleware could be attached by a separate piece of logic that doesn't need to know anything about the client.

[Fishrock123]

Yeah - I am saying that pattern is probably problematic (abstraction leaking?) and you may want to try doing it the other way around.

[aldanor]

@Fishrock123 Yes, you're probably right, I'm not arguing here :) Just trying to present the thoughts of someone new to surf (but who's read some of its sources out of confusion!). Like, "hey, so RequestBuilder looks like a thing to build Request, that's a common builder pattern, I know it. But, hold on, it also holds a client.. sometimes, or sometimes it doesn't. And a client can also return a RequestBuilder which can build requests which are then tied to that client?. So... I want to inspect a request and it appears that I need to build a separate middleware type which I can... only attach it to the client, so does it mean I have to attach it before I start building the request, is that the only way to do it? So I have to clone it, am I doing it right? Is cloning expensive, does it depend on client type? And should I stop using RequestBuilder and just use mutable methods on Request, but then what's the point of that builder type in the first place? (goes off to read the source)"

Out of curiosity (correct me if I'm wrong here) I've run a quick bench with the default client, creating a new client takes about 270us while cloning it is about 10-15us. Much faster and in most cases negligible, but in other cases 15us is too slow and a few orders of magnitude slower than pushing an object onto a vec. (unless you're using "pre-made clients" as we've discussed above, of course)

[Fishrock123]

If you want to be fast, you want to use a pre-made client.

I am becoming more sympathetic to the idea here, but I also feel it may be due to an error in the overall API design. maybe that ship has sailed though... unsure.

[Fishrock123]

See https://github.com/http-rs/surf/pull/266