Add ability to set a default User-Agent string to be used w/ each req…

[quinnj]

…uest. Fixes #161

cc: @oxinabox, @samoconnor

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@61e6b05). Click here to learn what that means.
The diff coverage is 33.33%.

@@           Coverage Diff            @@
##             master    #266   +/-   ##
========================================
  Coverage          ?   69.5%           
========================================
  Files             ?      35           
  Lines             ?    1951           
  Branches          ?       0           
========================================
  Hits              ?    1356           
  Misses            ?     595           
  Partials          ?       0

Impacted Files	Coverage Δ
src/MessageRequest.jl	92.3% <33.33%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61e6b05...733318b. Read the comment docs.

[samoconnor]

Is the user API HTTP.MessageRequest.setuseragent!?
Should it maybe be imported into HTTP so the user can just say HTTP.setuseragent!?

[quinnj]

@samoconnor, it's exported from MessageRequest, so HTTP.setuseragent! will indeed be the user API

[oxinabox]

Does thus need docs?

I feel like being a good net citizen means alnist all use HTTP should be followed by setuseragent!(ProjectName)

[oxinabox]

Delete commented bit?

[samoconnor]

@oxinabox, I'd say that being a good net citizen means:

• Not defining User-Agent in software that is not a user agent, i.e. A library that uses HTTP.jl to implement webdav is not a user agent; and a library that uses the webdav library to implement some acme.com file sharing API is not a user agent either. However, if I build "Sams Super Stats App" in Julia and I have an option to save stats .CSV files to acem.com, then I should set the User-Agent to "Sams Super Stats...".
• Not setting User-Agent to something generic that ends up being duplicated across many user agents. If someone adds a User-Agent rule to a proxy to deal with a single badly behave Julia app, we don't want every other Julia app to experience weird behaviour just because they all say "User-Agent: Juila HTTP". User-Agent is only useful if it allows people to implement fine-grained work-arounds for bugs in particular apps.
• If you are going to set User-Agent, set it to something well defined and so that consumers can reliably interpret the content. e.g. Mozilla has something like User-Agent: Mozilla/<version> (<system-information>) <platform> (<platform-details>) <extensions>

[samoconnor]

Re curl:
curl is a user agent (it's main use is as a command line tool) so it makes sense for it to have a User-Agent header. When curl is used as part of some larger application the -A option is used so set User-Agent to something more specific.

[oxinabox]

@samoconnor sounds like you've got half the doc I was suggesting written there :-)

[oxinabox]

If you are going to set User-Agent, set it to something well defined and so that consumers can reliably interpret the content. e.g. Mozilla has something like User-Agent: Mozilla/ () ()

It was on that notion,
that I though it might be good for all calls to setuseragent! to append.
Which would basically ensure that like Mozilla has every extension listed,
this would mean every loaded package that uses HTTP and setuseragent!.
(and then including the Julia version, and the HTTP.jl version I think would also make sense, sicne they match to platform information)

But that is not standard practice, so it is prob not a good idea to be the first.

[samoconnor]

I've got to say, this still seems like an anti-feature to me.
I agree pretty much with this: nodejs/node-v0.x-archive#4552 (comment)

We have a generic interface for sending headers with requests.
Users are free to define an app-local get function that sets whatever app-specific headers they'd like to include in all their requests.

It doesn't feel right to me for the HTTP Message Layer to have knowledge of the User-Agent header. The Message Layer deals with message encapsulation (Content-Length, Transfer-Encoding) and mandatory message headers (Host). It seems arbitrary to have special support for User-Agent at this layer.

[oxinabox]

This just sounds plain more convenient than declaring a wrapper function(/s) to use everywhere in your program.
And declaring such a wrapper function wouldn't propagate to calls that are made from libraries you've used, which is probably bad.

Lets say someone is made a package called GoogleDrive.jl built on using Swagger.jl which is inturn (in updated to be) built on HTTP.jl
All the get requests are in Swagger.jl
But they should be use the user-agent "GoogleDrive.jl v0.1.1".
(For the sake of this agument assume GoogleDrive.jl is a actual User-Agent,

So rather than wrapping HTTP.get we have to make a bunch of Swagger.jl's functions take a USER-AGENT arguement, and then we have to wrap them.

Then say someone uses GoogleDrive.jl in a package, maybe they want to set the user-agent themself because reasons. Now the wrapper functions must continue upwards.
etc.

This seems intensely inelegant.

This is kinda different to the Node.js thread, since this particular PR does not actually set the user-agent for HTTP.jl. Just provides a method allowing users to set it.

[samoconnor]

@oxinabox, I agree that the use case of over-riding User-Agent in a multi-library stack is one that should be supported elegantly.

This is kinda different to the Node.js thread, since this particular PR does not actually set the user-agent for HTTP.jl. Just provides a method allowing users to set it.

That's true. But the vibe I get from the Node.js thread is that the maintainers of the library see User-Agent as an application-level thing, not a low-level HTTP stack thing. So I think it is relevant.

I'd be happier if this was not done at the Message Layer and was made more generic. e.g. the interface could be HTTP.setdefaultheader!("User-Agent", "AcmeBitcoinWorm").

The implementation could be done in the top level HTTP.request function, or it could simply be a seperate layer something like this:

function request(::Type{DefaultHeadersLayer{Next}},
                 method::String, url::URI, headers, body; kw...) where Next
    for h in default_headers
        defaultbyfirst(headers, h)
    end
    return request(Next, method, url, headers, body; kw...)
end

Another alternative would be to use this as a use-case for adding a custom layer.
I had always planned to have some kind of HTTP.insertlayer function to allow applications to insert custom layers into the request stack. This might look something like...

abstract type GoogleDriveLayer{Next <: Layer} <: Layer end

function request(::Type{GoogleDriveLayer{Next}},
                 url, req, body; kw...) where Next
    setheader(req, "User-Agent" => "GoogleDrive.jl v0.1.1")
    return request(Next, url, req, body; kw...)
end

HTTP.insertlayerafter(HTTP.MessageLayer, GoogleDriveLayer)

One of my motivations for being a bit anal about what does and does not belong at a certain layer is to ensure that custom layers can be created without too much angst about "I want to be before thing X, but after thing Y, but I can't because they're done at the same layer".