Allow custom request timeouts to be passed to OkHttp client (#86)

[finn-block]

Allows customizing the OkHttpClient's request timeout. The default is still to not have any timeout.

[codecov]

Codecov Report

Merging #134 (ce9307c) into main (a433d95) will decrease coverage by 0.37%.
Report is 9 commits behind head on main.
The diff coverage is 50.00%.

❗ Current head ce9307c differs from pull request most recent head d50f535. Consider uploading reports for the commit d50f535 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #134      +/-   ##
==========================================
- Coverage   78.64%   78.27%   -0.37%     
==========================================
  Files          30       30              
  Lines         693      672      -21     
  Branches       65       65              
==========================================
- Hits          545      526      -19     
+ Misses        114      112       -2     
  Partials       34       34              

Components	Coverage Δ
protocol	85.53% <ø> (+0.43%)	⬆️
httpclient	82.14% <50.00%> (-0.84%)	⬇️

[phoebe-lew]

So, we originally made TbdexHttpClient an object because we didn't think people needed to instantiate it/one would be enough. However, if we're making the timeout customizable, I think we could make it a normal class, and leave OkHttpClient as val. So effectively you could pass in the timeout as an argument to TbdexHttpClient's constructor, which would set the timeout on OkHttpClient.

The reason for doing this is that it isn't immediately clear when you call setTimeout that you are constructing a new OkHttpClient every time. (Builder.build returns a new instance of the client).

[phoebe-lew]

Check out this section about init blocks for how you can use arguments from the constructor when creating a new instance of a class: https://kotlinlang.org/docs/classes.html#constructors

[phoebe-lew]

Maybe we should default to 10s or something!

[finn-block]

since okhttp has default timeouts IMO we should just leave them unless we have some reason to change them. It might make sense to document the existence of the default timeouts from the library

[mistermoe]

Is there a way to pass timeouts per method vs as a constructor arg?

[mistermoe]

cc: @angiejones

This would be a breaking change if yall have guides using the http client

[finn-block]

Easy enough to add an optional param to all the methods with a timeout option. Should that a replace the constructor argument i've got currently? or just another way to do set the timeout (presumably overriding the constructor arg).

As for this being a breaking change: Maybe it's my unfamiliarity with Kotlin but I thought by providing a secondary constructor with no arguments it would make existing code work exactly how it currently does:

constructor() : this(Duration.ZERO)

[phoebe-lew]

@mistermoe what's the reasoning for wanting this per-method? timeouts are configured at a client instance level, so if we want the timeout to be different on every call, we essentially need to reinstantiate the client every time.