Use Reqwest instead of custom HTTP handler #510
Conversation
| match self { | ||
| Fetch::Url(url) => fetch_with_str(url.as_ref(), Some(signal)).await, | ||
| Fetch::Request(req) => fetch_with_request(req, Some(signal)).await, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| //#[cfg(feature = "http")] | ||
| async fn fetch_with_str(url: &str, signal: Option<&AbortSignal>) -> Result<Response> { |
There was a problem hiding this comment.
Might have to upstream some changes to pass in the abort signal https://github.com/seanmonstar/reqwest/blob/68a3f5803b03393938c39a3794a5a9e5f5264796/src/wasm/client.rs#L221
There was a problem hiding this comment.
This might be a bad question, but why does the reqwest repo contain .signal functionality and the Cargo package doesn't? I don't see it in the docs either when it's clearly in the repository.
https://docs.rs/reqwest/latest/reqwest/struct.Client.html
Is there a WIP branch or something of the sort to get at it?
There was a problem hiding this comment.
I think this is because the Wasm support just hooks up to fetch in the same way we do so, its only possible on Wasm.
https://github.com/seanmonstar/reqwest/blob/master/src/wasm/client.rs#L185
But you see it isn't plumbed through from the client or request object.
There was a problem hiding this comment.
Would it be acceptable to just use Drop and provide a handler? I am not sure if we necessarily need direct access to the signal.
Something like:
// Asynchronous loop to continuously check for cancellation signal
loop {
tokio::select! {
_ = sleep(Duration::from_secs(1)) => {
println!("Async task running...");
}
_ = cancel_receiver.recv() => {
println!("Cancellation signal received. Dropping task...");
break; // Break out of the loop when cancellation signal is received
}
}
}There was a problem hiding this comment.
I'm not sure what the tradeoffs are here. Using a JavaScript Abort Signal lets the runtime cancel the request rather which I think might be more efficient, but if there is no way to plumb this through it may be ok to go this route.
| match self { | ||
| Fetch::Url(url) => fetch_with_str(url.as_ref(), None).await, | ||
| Fetch::Request(req) => fetch_with_request(req, None).await, | ||
| } | ||
| } | ||
|
|
||
| /// Execute a Fetch call and receive a Response. | ||
| pub async fn send_with_signal(&self, signal: &AbortSignal) -> Result<WorkerResponse> { | ||
| pub async fn send_with_signal(&self, signal: &AbortSignal) -> Result<Response> { |
There was a problem hiding this comment.
I wonder if we want to convert reqwest::Response to http::Response<Body>. I think its not totally straightforward, but possible.
There was a problem hiding this comment.
What would be the purpose of that? Would it help with keeping the data format more flexible? I'm curious.
There was a problem hiding this comment.
Well, right now if they wanted to call fetch and then return the value, it would be the wrong type unless it was converted to http::Response.
As discussed in #490
I intend to recreate the Fetch API using Reqwest to add support for the HTTP crate.