diff options
| -rw-r--r-- | Cargo.lock | 7 | ||||
| -rw-r--r-- | cli/http_util.rs | 17 | ||||
| -rw-r--r-- | ext/fetch/26_fetch.js | 7 | ||||
| -rw-r--r-- | ext/fetch/Cargo.toml | 1 | ||||
| -rw-r--r-- | ext/fetch/lib.rs | 70 | ||||
| -rw-r--r-- | tests/specs/cert/localhost_unsafe_ssl/localhost_unsafe_ssl.ts.out | 2 | ||||
| -rw-r--r-- | tests/testdata/run/fetch_async_error_stack.ts.out | 2 | ||||
| -rw-r--r-- | tests/unit/fetch_test.ts | 26 |
8 files changed, 98 insertions, 34 deletions
diff --git a/Cargo.lock b/Cargo.lock index b962ef131..fc6c16e70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1493,6 +1493,7 @@ dependencies = [ "deno_permissions", "deno_tls", "dyn-clone", + "error_reporter", "fast-socks5", "http 1.1.0", "http-body-util", @@ -2699,6 +2700,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a0474425d51df81997e2f90a21591180b38eccf27292d755f3e30750225c175b" [[package]] +name = "error_reporter" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "31ae425815400e5ed474178a7a22e275a9687086a12ca63ec793ff292d8fdae8" + +[[package]] name = "escape8259" version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" diff --git a/cli/http_util.rs b/cli/http_util.rs index b47e91c70..9ec90dd61 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -390,10 +390,10 @@ impl HttpClient { let response = match self.client.clone().send(request).await { Ok(resp) => resp, Err(err) => { - if is_error_connect(&err) { + if err.is_connect_error() { return Ok(FetchOnceResult::RequestError(err.to_string())); } - return Err(err); + return Err(err.into()); } }; @@ -531,7 +531,7 @@ impl HttpClient { .clone() .send(req) .await - .map_err(DownloadError::Fetch)?; + .map_err(|e| DownloadError::Fetch(e.into()))?; let status = response.status(); if status.is_redirection() { for _ in 0..5 { @@ -551,7 +551,7 @@ impl HttpClient { .clone() .send(req) .await - .map_err(DownloadError::Fetch)?; + .map_err(|e| DownloadError::Fetch(e.into()))?; let status = new_response.status(); if status.is_redirection() { response = new_response; @@ -567,13 +567,6 @@ impl HttpClient { } } -fn is_error_connect(err: &AnyError) -> bool { - err - .downcast_ref::<hyper_util::client::legacy::Error>() - .map(|err| err.is_connect()) - .unwrap_or(false) -} - async fn get_response_body_with_progress( response: http::Response<deno_fetch::ResBody>, progress_guard: Option<&UpdateGuard>, @@ -685,7 +678,7 @@ impl RequestBuilder { pub async fn send( self, ) -> Result<http::Response<deno_fetch::ResBody>, AnyError> { - self.client.send(self.req).await + self.client.send(self.req).await.map_err(Into::into) } pub fn build(self) -> http::Request<deno_fetch::ReqBody> { diff --git a/ext/fetch/26_fetch.js b/ext/fetch/26_fetch.js index 674d99709..e2809187b 100644 --- a/ext/fetch/26_fetch.js +++ b/ext/fetch/26_fetch.js @@ -65,7 +65,7 @@ const REQUEST_BODY_HEADER_NAMES = [ /** * @param {number} rid - * @returns {Promise<{ status: number, statusText: string, headers: [string, string][], url: string, responseRid: number, error: string? }>} + * @returns {Promise<{ status: number, statusText: string, headers: [string, string][], url: string, responseRid: number, error: [string, string]? }>} */ function opFetchSend(rid) { return op_fetch_send(rid); @@ -177,8 +177,9 @@ async function mainFetch(req, recursive, terminator) { } } // Re-throw any body errors - if (resp.error) { - throw new TypeError("body failed", { cause: new Error(resp.error) }); + if (resp.error !== null) { + const { 0: message, 1: cause } = resp.error; + throw new TypeError(message, { cause: new Error(cause) }); } if (terminator.aborted) return abortedNetworkError(); diff --git a/ext/fetch/Cargo.toml b/ext/fetch/Cargo.toml index f0e06c87f..75a358b2d 100644 --- a/ext/fetch/Cargo.toml +++ b/ext/fetch/Cargo.toml @@ -21,6 +21,7 @@ deno_core.workspace = true deno_permissions.workspace = true deno_tls.workspace = true dyn-clone = "1" +error_reporter = "1" http.workspace = true http-body-util.workspace = true hyper.workspace = true diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs index 7c717ccec..798acad0b 100644 --- a/ext/fetch/lib.rs +++ b/ext/fetch/lib.rs @@ -26,6 +26,7 @@ use deno_core::futures::Future; use deno_core::futures::FutureExt; use deno_core::futures::Stream; use deno_core::futures::StreamExt; +use deno_core::futures::TryFutureExt; use deno_core::op2; use deno_core::unsync::spawn; use deno_core::url::Url; @@ -429,7 +430,7 @@ where let mut request = http::Request::new(body); *request.method_mut() = method.clone(); - *request.uri_mut() = uri; + *request.uri_mut() = uri.clone(); if let Some((username, password)) = maybe_authority { request.headers_mut().insert( @@ -469,8 +470,15 @@ where let cancel_handle = CancelHandle::new_rc(); let cancel_handle_ = cancel_handle.clone(); - let fut = - async move { client.send(request).or_cancel(cancel_handle_).await }; + let fut = { + async move { + client + .send(request) + .map_err(Into::into) + .or_cancel(cancel_handle_) + .await + } + }; let request_rid = state.resource_table.add(FetchRequestResource { future: Box::pin(fut), @@ -532,7 +540,11 @@ pub struct FetchResponse { pub content_length: Option<u64>, pub remote_addr_ip: Option<String>, pub remote_addr_port: Option<u16>, - pub error: Option<String>, + /// This field is populated if some error occurred which needs to be + /// reconstructed in the JS side to set the error _cause_. + /// In the tuple, the first element is an error message and the second one is + /// an error cause. + pub error: Option<(String, String)>, } #[op2(async)] @@ -558,16 +570,16 @@ pub async fn op_fetch_send( // reconstruct an error chain (eg: `new TypeError(..., { cause: new Error(...) })`). // TODO(mmastrac): it would be a lot easier if we just passed a v8::Global through here instead let mut err_ref: &dyn std::error::Error = err.as_ref(); - while let Some(err) = std::error::Error::source(err_ref) { - if let Some(err) = err.downcast_ref::<hyper::Error>() { - if let Some(err) = std::error::Error::source(err) { + while let Some(err_src) = std::error::Error::source(err_ref) { + if let Some(err_src) = err_src.downcast_ref::<hyper::Error>() { + if let Some(err_src) = std::error::Error::source(err_src) { return Ok(FetchResponse { - error: Some(err.to_string()), + error: Some((err.to_string(), err_src.to_string())), ..Default::default() }); } } - err_ref = err; + err_ref = err_src; } return Err(type_error(err.to_string())); @@ -1082,11 +1094,41 @@ type Connector = proxy::ProxyConnector<HttpConnector>; #[allow(clippy::declare_interior_mutable_const)] const STAR_STAR: HeaderValue = HeaderValue::from_static("*/*"); +#[derive(Debug)] +pub struct ClientSendError { + uri: Uri, + source: hyper_util::client::legacy::Error, +} + +impl ClientSendError { + pub fn is_connect_error(&self) -> bool { + self.source.is_connect() + } +} + +impl std::fmt::Display for ClientSendError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!( + f, + "error sending request for url ({uri}): {source}", + uri = self.uri, + // NOTE: we can use `std::error::Report` instead once it's stabilized. + source = error_reporter::Report::new(&self.source), + ) + } +} + +impl std::error::Error for ClientSendError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.source) + } +} + impl Client { pub async fn send( self, mut req: http::Request<ReqBody>, - ) -> Result<http::Response<ResBody>, AnyError> { + ) -> Result<http::Response<ResBody>, ClientSendError> { req .headers_mut() .entry(USER_AGENT) @@ -1098,7 +1140,13 @@ impl Client { req.headers_mut().insert(PROXY_AUTHORIZATION, auth.clone()); } - let resp = self.inner.oneshot(req).await?; + let uri = req.uri().clone(); + + let resp = self + .inner + .oneshot(req) + .await + .map_err(|e| ClientSendError { uri, source: e })?; Ok(resp.map(|b| b.map_err(|e| anyhow!(e)).boxed())) } } diff --git a/tests/specs/cert/localhost_unsafe_ssl/localhost_unsafe_ssl.ts.out b/tests/specs/cert/localhost_unsafe_ssl/localhost_unsafe_ssl.ts.out index f98c7e4e4..c7bdfde0e 100644 --- a/tests/specs/cert/localhost_unsafe_ssl/localhost_unsafe_ssl.ts.out +++ b/tests/specs/cert/localhost_unsafe_ssl/localhost_unsafe_ssl.ts.out @@ -1,3 +1,3 @@ DANGER: TLS certificate validation is disabled for: deno.land -error: Import 'https://localhost:5545/subdir/mod2.ts' failed: client error[WILDCARD] +error: Import 'https://localhost:5545/subdir/mod2.ts' failed: error sending request for url (https://localhost:5545/subdir/mod2.ts): client error[WILDCARD] at file:///[WILDCARD]/cafile_url_imports.ts:[WILDCARD] diff --git a/tests/testdata/run/fetch_async_error_stack.ts.out b/tests/testdata/run/fetch_async_error_stack.ts.out index 06d92d15a..2722cc8a7 100644 --- a/tests/testdata/run/fetch_async_error_stack.ts.out +++ b/tests/testdata/run/fetch_async_error_stack.ts.out @@ -1,4 +1,4 @@ -error: Uncaught (in promise) TypeError: client error[WILDCARD] +error: Uncaught (in promise) TypeError: error sending request for url (https://nonexistent.deno.land/): client error[WILDCARD] await fetch("https://nonexistent.deno.land/"); ^[WILDCARD] at async fetch (ext:[WILDCARD]) diff --git a/tests/unit/fetch_test.ts b/tests/unit/fetch_test.ts index 09cbb5cd2..5ebc0c86f 100644 --- a/tests/unit/fetch_test.ts +++ b/tests/unit/fetch_test.ts @@ -1976,14 +1976,17 @@ Deno.test( }, }); - const err = await assertRejects(() => - fetch(`http://localhost:${listenPort}/`, { - body: stream, - method: "POST", - }) + const url = `http://localhost:${listenPort}/`; + const err = await assertRejects( + () => + fetch(url, { + body: stream, + method: "POST", + }), + TypeError, + `error sending request for url (${url}): client error (SendRequest): error from user's Body stream`, ); - assert(err instanceof TypeError, `err was not a TypeError ${err}`); assert(err.cause, `err.cause was null ${err}`); assert( err.cause instanceof Error, @@ -2060,3 +2063,14 @@ Deno.test("URL authority is used as 'Authorization' header", async () => { await server.finished; assertEquals(authHeader, "Basic ZGVubzpsYW5k"); }); + +Deno.test( + { permissions: { net: true } }, + async function errorMessageIncludesUrlAndDetails() { + await assertRejects( + () => fetch("http://example.invalid"), + TypeError, + "error sending request for url (http://example.invalid/): client error (Connect): dns error: ", + ); + }, +); |