creachadair / jrpc2

A Go implementation of a JSON-RPC 2.0 server and client.
BSD 3-Clause "New" or "Revised" License
62 stars 10 forks source link

Add config value for custom user agent string in jhttp #112

Closed Lukasz032 closed 11 months ago

Lukasz032 commented 11 months ago

When using a HTTP transport, requests have a certain User-Agent header. It's beneficial to set this to the application-related one.

A config value would be an ideal place for this, instead of hooking custom request interceptors in-between the normal request flow just for this task.

creachadair commented 11 months ago

I'm not convinced an additional option for this narrow use is worth adding to the library. There's already a channel option to override the client. It's only a few lines of code to shim that, something like:

// UAClient is a shim for http.DefaultClient that overrides the User-Agent.
type UAClient string

func (u UAClient) Do(req *http.Request) (*http.Response, error) {
    req.Header.Set("User-Agent", string(u))
    return http.DefaultClient.Do(req)
}

and then plug that in. Here is an example in the Go playground that demonstrates it.

There are lots of policy headers a client might want to set. In general, the jhttp package is meant to push JSON-RPC over HTTP as a transport, but an application that cares about other details of HTTP semantics might be better served by using HTTP directly instead of JSON-RPC.

Lukasz032 commented 11 months ago

You're quire right here about your last statement. Unfortunately I don't have control over the server we're making a client to, since it can be basically anything - I only require the integration our app is hooking into speaks proper JSON-RPC, and what our contractors put as the datacenter doesn't concern us.

Anyway, thanks for even simpler idea to hook custom user agent string into requests. My first solution was to extend a http.Client itself and hook a custom Do() calling a parent Do() at the end, but your solution works too and uses even less lines of code :)

creachadair commented 11 months ago

My example works as long as you wanted the default client anyway, which will often be the case. If you do want to wrap another client, though, it's not that much worse:

// warning: untested
type UAClient struct{
  userAgent string
  client    jhttp.HTTPClient
}

func (u UAClient) Do(req *http.Request) (*http.Response, error) {
   req.Header.Set("User-Agent", u.userAgent)
   return u.client.Do(req)
}

func NewUAClient(userAgent string, cli jhttp.HTTPClient) UAClient {
   return UAClient{userAgent: userAgent, client: cli}
}

or words to that effect. (This is why jhttp defines the client in terms of a restricted interface, since that's the only part it needs from the concrete type).