feat(ext/fetch): Replace reqwest with reqwest_middleware

[maxmcd]

trafficstars

Related to: https://github.com/denoland/deno/issues/23516

This replaces uses of reqwest and reqwest::Client with the request_middleware alternatives so that middleware can be mounted. The intention is to allow tracing and other middleware to be added in Rust code.

I considered adding the middleware configuration to CreateHttpClientOptions, but it was a little tricky to plumb it through everywhere. Hopefully this use of once_cell is considered ok and fits well with the contract of this middleware, ie: "add middleware to all deno http requests".

Example of use:

diff --git a/cli/main.rs b/cli/main.rs
index a4e93ca31..61a969bb5 100644
--- a/cli/main.rs
+++ b/cli/main.rs
@@ -41,6 +41,9 @@ use deno_core::error::JsError;
 use deno_core::futures::FutureExt;
 use deno_core::unsync::JoinHandle;
 use deno_npm::resolution::SnapshotFromLockfileError;
+use deno_runtime::deno_fetch::add_middleware;
+use deno_runtime::deno_fetch::reqwest;
+use deno_runtime::deno_fetch::reqwest_middleware;
 use deno_runtime::fmt_errors::format_js_error;
 use deno_runtime::tokio_util::create_and_run_current_thread_with_maybe_metrics;
 use deno_terminal::colors;
@@ -50,6 +53,7 @@ use std::env;
 use std::env::current_exe;
 use std::future::Future;
 use std::path::PathBuf;
+use std::sync::Arc;

 /// Ensures that all subcommands return an i32 exit code and an [`AnyError`] error type.
 trait SubcommandOutput {
@@ -302,10 +306,28 @@ pub(crate) fn unstable_warn_cb(feature: &str, api_name: &str) {
     ))
   );
 }
+struct LoggingMiddleware;
+
+#[async_trait::async_trait]
+impl reqwest_middleware::Middleware for LoggingMiddleware {
+  async fn handle(
+    &self,
+    req: reqwest::Request,
+    extensions: &mut task_local_extensions::Extensions,
+    next: reqwest_middleware::Next<'_>,
+  ) -> reqwest_middleware::Result<reqwest::Response> {
+    println!("Request started {:?}", req);
+    let res = next.run(req, extensions).await;
+    println!("Result: {:?}", res);
+    res
+  }
+}

 pub fn main() {
   setup_panic_hook();

+  add_middleware(Arc::new(LoggingMiddleware)).unwrap();
+
   util::unix::raise_fd_limit();
   util::windows::ensure_stdio_open();
   #[cfg(windows)]

[CLAassistant]

All committers have signed the CLA.

[maxmcd]

Ah, the testing is tricky because I am using a global variable and the tokio tests are run async so the middleware is picked up by something else. Maybe there is a way to lock a specific test so I can ensure I have cleaned up?

[bartlomieju]

@maxmcd as we discussed previously on Discord, we moved away from using reqwest in https://github.com/denoland/deno/pull/24593, in favor of using hyper directly.

I'm happy to add a "hook" function that allows to customize execution of the HTTP client, but I think we need to take a fresh stab at it.

Is it something we could do using ext::fetch::Options::request_builder_hook maybe?

[maxmcd]

@bartlomieju nice, yes it does seem like I could hook into that. We deprioritized this a bit for now since we're planning on getting module tracing by sticking metadata into auth tokens.

Maybe best to close this for now and I'll check out request_builder_hook when we come back to this problem space.