Setting headers in an AgentBuilder

[sztomi]

I used pre-2.0 ureq before. I noticed that Agent now doesn't have a set method. The Changelog says this:

Create AgentBuilder to separate the process of building an agent from using the resulting agent. Headers can be set on an AgentBuilder, not the resulting Agent.

But I don't see any way to set a header on the AgentBuilder either. Am I missing something?

[algesten]

Hi sztomi! Welcome to ureq!

I think that line is misleading. In the olden days, ureq used to be able to set headers on the agent, but we dropped it because it's probably not a good idea. See comment here for instance: #308 (comment)

What's the use case?

[sztomi]

The use-case is exactly what you describe there. I don't think that accidentally reusing the agent is a real risk in many real-world applications. In any case, I don't think ureq should drop that feature instead of documenting this risk.

Also, connection pooling could be implemented in an agent that would improve the performance of requests. See Session objects in the requests library in Python.

[sigaloid]

I would like this as well. I don't believe that it's something that should be necessarily guarded against by limiting the functionality (maybe a docs warning).

If you only need the Auth header for a single request, then attach it for that one. If you need it for one hostname, then don't use the agent for any others.

Setting cookies already implies that it might not be designed to be used as a sort of global Agent that all requests use to all endpoints IMO; user-agent maybe not as much as auth typically doesn't happen via a user agent in a well-designed system.

[algesten]

Setting fixed headers would work for some scenarios, but for more complicated stuff, like renewing oauth refresh tokens, having a bit more support would be helpful.

Wonder if it would be better to implement this as some kind of "plugin" trait that gets a chance to modify each request being dispatched by the agent. A bit like nodejs express middleware.

[sztomi]

@algesten a middleware-kinda thing has merit, but if it's only for this feature I think that would be overkill. Many http libraries have a notion of a semi-permanent "session" or "client" that can be used to make requests with similar settings or headers, so from the user's perspective it would be better to add back the feature to Client. If there is still worry about the risk of leaking tokens, maybe add it behind a feature flag that's named appropriately.

[algesten]

I'm always interested in prior art. Is there a library in particular you're thinking of?

[sztomi]

@algesten I think this style of API for a HTTP client originates from the Python requests library. This is their rendition of this feature: https://2.python-requests.org/en/master/user/advanced/#session-objects

In rust, reqwest (https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html) and attohttpc (https://docs.rs/attohttpc/latest/attohttpc/struct.Session.html) also implements it.

[algesten]

@sztomi Thanks for those links. I got a splurge of creativity going so I implemented a PR for middleware. Whether we want that or not is to be discussed. I'll review the provided APIs meanwhile.

I specifically did a doc entry showing how to add headers with this

[sztomi]

@algesten that looks pretty good! Initially when you said middleware I thought it would be something heavier, but if it's some kind of Fn it will be great. How can I help to move this forward?

[algesten]

I just want a sense check from @jsha whether this is something we believe is useful. The alternative is of course to (re-)implement some sort of default_headers on the agent level – which arguably is much less code to maintain going forward.

[jsha]

I really like the idea of supporting middleware. This can support use cases like self-refreshing authorization tokens and metrics collection, and encoding / decoding (like providing additional decompression formats). I'll provide implementation feedback on the PR.