refactor: ClientBuilder are diverging

[Sytten]

Since https://github.com/0xPlaygrounds/rig/pull/875, client builders have now diverged.

• Some return a Result on build, some don't. This should be uniform and always return a Result for the future compatibility.
• Someone did re-add the new_with_client (thanks for that!), but it should have been kept an Option IMO with client building in the build to catch error in the building of clients.
• Not all providers re-export ClientBuilder

[linear[bot]]

RIG-1038

[joshua-mo-143]

I'm unavailable over the next couple of days as I'll be doing talks and stuff, but whenever I'm available I should be able to get around to this (if nobody else wants it).

[FayCarsons]

Hi @Sytten we're in the middle of a big refactor of the whole client system, consolidating it into one central Client + ClientBuilder which providers can slot an extension into.

I'm wondering if you can expand on your point about the internal HTTP client being an Option? Are you saying that the http_client field of the ClientBuilder should be an Option<BackingHttpClient>? Because I think that's a step away from type-safety which sort of clashes with our priorities. Would something like a Client::from_http_client with the signature HttpClient -> ClientBuilder<HttpClient> work for your use cases?

[Sytten]

@FayCarsons No stress, yes it should be an Option<BackingHttpClient> because it doesn't matter here for safety since you will create the client in the build function if it is missing (thus the need for build to return Result). The big advantages are:

• You don't build a client if you don't need to
• You don't have to panic when you build the reqwest client. The Default reqwest can panic (https://github.com/seanmonstar/reqwest/blob/9c4999d60761c5863e8a54d5389a9f049d095a3c/src/async_impl/client.rs#L2380-L2382) for example when the TLS provider is not present

[Sytten]

This is done now that the builder is centralized