summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYusuke Tanaka <yusuktan@maguro.dev>2024-08-09 00:47:15 +0900
committerGitHub <noreply@github.com>2024-08-08 08:47:15 -0700
commite36b1a3aa88b31435b18a33448fc75eeb6dc8017 (patch)
treebc29141f8d59c74db46fee49426a9bc1a77c1d10
parent18b9b43c3631053e2c8b4c293b9e1f44dee7bfa8 (diff)
fix(ext/fetch): include TCP src/dst socket info in error messages (#24939)
This commit makes `fetch` error messages include source and destination TCP socket info i.e. port number and IP address for better debuggability. Closes #24922
-rw-r--r--Cargo.lock4
-rw-r--r--Cargo.toml2
-rw-r--r--ext/fetch/lib.rs38
-rw-r--r--tests/unit/fetch_test.ts59
4 files changed, 84 insertions, 19 deletions
diff --git a/Cargo.lock b/Cargo.lock
index aa989b210..238bfb89e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3630,9 +3630,9 @@ dependencies = [
[[package]]
name = "hyper-util"
-version = "0.1.6"
+version = "0.1.7"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3ab92f4f49ee4fb4f997c784b7a2e0fa70050211e0b6a287f898c3c9785ca956"
+checksum = "cde7055719c54e36e95e8719f95883f22072a48ede39db7fc17a4e1d5281e9b9"
dependencies = [
"bytes",
"futures-channel",
diff --git a/Cargo.toml b/Cargo.toml
index cbf9940ca..076ebdf4f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -124,7 +124,7 @@ http_v02 = { package = "http", version = "0.2.9" }
httparse = "1.8.0"
hyper = { version = "1.4.1", features = ["full"] }
hyper-rustls = { version = "0.27.2", default-features = false, features = ["http1", "http2", "tls12", "ring"] }
-hyper-util = { version = "=0.1.6", features = ["tokio", "client", "client-legacy", "server", "server-auto"] }
+hyper-util = { version = "=0.1.7", features = ["tokio", "client", "client-legacy", "server", "server-auto"] }
hyper_v014 = { package = "hyper", version = "0.14.26", features = ["runtime", "http1"] }
indexmap = { version = "2", features = ["serde"] }
ipnet = "2.3"
diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs
index 798acad0b..fa85824f4 100644
--- a/ext/fetch/lib.rs
+++ b/ext/fetch/lib.rs
@@ -62,11 +62,13 @@ use http::header::HOST;
use http::header::PROXY_AUTHORIZATION;
use http::header::RANGE;
use http::header::USER_AGENT;
+use http::Extensions;
use http::Method;
use http::Uri;
use http_body_util::BodyExt;
use hyper::body::Frame;
use hyper_util::client::legacy::connect::HttpConnector;
+use hyper_util::client::legacy::connect::HttpInfo;
use hyper_util::rt::TokioExecutor;
use hyper_util::rt::TokioIo;
use hyper_util::rt::TokioTimer;
@@ -1104,17 +1106,39 @@ impl ClientSendError {
pub fn is_connect_error(&self) -> bool {
self.source.is_connect()
}
+
+ fn http_info(&self) -> Option<HttpInfo> {
+ let mut exts = Extensions::new();
+ self.source.connect_info()?.get_extras(&mut exts);
+ exts.remove::<HttpInfo>()
+ }
}
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),
- )
+ // NOTE: we can use `std::error::Report` instead once it's stabilized.
+ let detail = error_reporter::Report::new(&self.source);
+
+ match self.http_info() {
+ Some(http_info) => {
+ write!(
+ f,
+ "error sending request from {src} for {uri} ({dst}): {detail}",
+ src = http_info.local_addr(),
+ uri = self.uri,
+ dst = http_info.remote_addr(),
+ detail = detail,
+ )
+ }
+ None => {
+ write!(
+ f,
+ "error sending request for url ({uri}): {detail}",
+ uri = self.uri,
+ detail = detail,
+ )
+ }
+ }
}
}
diff --git a/tests/unit/fetch_test.ts b/tests/unit/fetch_test.ts
index 5ebc0c86f..9b2463bcc 100644
--- a/tests/unit/fetch_test.ts
+++ b/tests/unit/fetch_test.ts
@@ -3,6 +3,7 @@ import {
assert,
assertEquals,
assertRejects,
+ assertStringIncludes,
assertThrows,
delay,
fail,
@@ -1977,14 +1978,24 @@ Deno.test(
});
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`,
+ const err = await assertRejects(() =>
+ fetch(url, {
+ body: stream,
+ method: "POST",
+ })
+ );
+
+ assert(err instanceof TypeError, `err was ${err}`);
+
+ assertStringIncludes(
+ err.message,
+ "error sending request from 127.0.0.1:",
+ `err.message was ${err.message}`,
+ );
+ assertStringIncludes(
+ err.message,
+ ` for http://localhost:${listenPort}/ (127.0.0.1:${listenPort}): client error (SendRequest): error from user's Body stream`,
+ `err.message was ${err.message}`,
);
assert(err.cause, `err.cause was null ${err}`);
@@ -2066,7 +2077,7 @@ Deno.test("URL authority is used as 'Authorization' header", async () => {
Deno.test(
{ permissions: { net: true } },
- async function errorMessageIncludesUrlAndDetails() {
+ async function errorMessageIncludesUrlAndDetailsWithNoTcpInfo() {
await assertRejects(
() => fetch("http://example.invalid"),
TypeError,
@@ -2074,3 +2085,33 @@ Deno.test(
);
},
);
+
+Deno.test(
+ { permissions: { net: true } },
+ async function errorMessageIncludesUrlAndDetailsWithTcpInfo() {
+ const listener = Deno.listen({ port: listenPort });
+ const server = (async () => {
+ const conn = await listener.accept();
+ listener.close();
+ // Immediately close the connection to simulate a connection error
+ conn.close();
+ })();
+
+ const url = `http://localhost:${listenPort}`;
+ const err = await assertRejects(() => fetch(url));
+
+ assert(err instanceof TypeError, `${err}`);
+ assertStringIncludes(
+ err.message,
+ "error sending request from 127.0.0.1:",
+ `${err.message}`,
+ );
+ assertStringIncludes(
+ err.message,
+ ` for http://localhost:${listenPort}/ (127.0.0.1:${listenPort}): client error (SendRequest): `,
+ `${err.message}`,
+ );
+
+ await server;
+ },
+);