isahc necessary?

[mwilliammyers]

Is isahc necessary? Can we just depend on reqwest?

[Hirevo]

Currently yes, but for a weird reason.

The reason is that the authentication mechanism is implemented using Tonic interceptors (to refresh tokens automatically when needed), but these cannot be async.
If we were to use the reqwest::blocking facilities in there, the program would always panic with the following message:

Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.

It is because reqwest starts a new runtime when we (or the user) already have one running.
isahc doesn't spawn any runtimes for its synchronous API, so it fixes this issue.

But I agree that it is awkward and I would like to only keep one HTTP client.

It appears that async interceptors are not intentionally not supported, according to this issue, so I am not sure how to work around it yet.

[mwilliammyers]

Ah, that makes sense!

I wonder if we can use ureq instead, which doesn't depend on libcurl...

Edit: it looks like it builds libcurl or produces a static binary from system libcurl?

[LucioFranco]

@Hirevo is it possible to do the async request for the tokens off task? For example, use arc-swap + a background task that can run an async call. You can then also use something like tokio::Notify to start the process as well. Just an idea! I'd like to support async resolvers for tonic. I have not looked much at the implementation in here but another option is to work at the service level rather than the interceptor level.

Happy to help explore! :)

[Hirevo]

@LucioFranco Sorry about the long silence on my end, I am not sure why I didn't came back to this.
I actually think we can get away with a simpler solution here, which I started to implement in #17.

We can ensure our tokens are still valid at the time we're constructing each request.

We now define the following function:

pub(crate) async fn construct_request<T: IntoRequest<T>>(
    &mut self,
    request: T,
) -> Result<Request<T>, Error> {
    let mut request = request.into_request();
    let token = self.token_manager.lock().await.token().await?; // <- ensures the token is up-to-date
    let metadata = request.metadata_mut();
    metadata.insert("authorization", token.parse().unwrap());
    Ok(request)
}

And we call it before sending each request like this:

let request = api::LookupRequest {
    // Fill request parameters ...
};
let request = self.construct_request(request).await?;
let response = self.service.lookup(request).await?;

It is a more pragmatic approach but I think it can be sufficient for our purposes.