diff --git a/src/jsc/SystemError.rs b/src/jsc/SystemError.rs index b2fd3c3b13df..2ff6e2d3bffa 100644 --- a/src/jsc/SystemError.rs +++ b/src/jsc/SystemError.rs @@ -8,6 +8,7 @@ use crate::{JSGlobalObject, JSPromise, JSValue}; #[repr(C)] #[derive(Clone)] pub struct SystemError { + /// [`SystemError::NO_ERRNO`] = the JS error gets `errno: undefined` pub errno: c_int, /// label for errno pub code: OwnedString, @@ -80,6 +81,12 @@ unsafe extern "C" { } impl SystemError { + /// `errno` for an error that has no numeric system error code, only a + /// string `code`. Node reports such errors (a c-ares resolver failure, for + /// example) with an own `errno` property whose value is `undefined`, and + /// the C++ side (`systemErrorToErrorInstance`) emits exactly that. + pub const NO_ERRNO: c_int = c_int::MIN; + /// Converts to a JS `Error`, consuming `self`. C++ only borrows the string /// fields; `Drop` releases them when `self` goes out of scope. `.clone()` /// first when two `Error`s are genuinely wanted. diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 147f9a5ec434..293da5cf2b57 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -2569,6 +2569,15 @@ JSC::EncodedJSValue JSGlobalObject__createOutOfMemoryError(JSC::JSGlobalObject* return JSValue::encode(exception); } +// `SystemError::NO_ERRNO` on the Rust side: the error has a string `code` but +// no numeric errno, and Node reports those with `errno: undefined`. +static JSC::JSValue systemErrorErrnoValue(const SystemError& err) +{ + if (err.errno_ == std::numeric_limits::min()) + return JSC::jsUndefined(); + return JSC::jsNumber(err.errno_); +} + static JSC::EncodedJSValue systemErrorToErrorInstance(const SystemError* arg0, JSC::JSGlobalObject* globalObject, JSC::ErrorType errorType) { SystemError err = *arg0; @@ -2637,7 +2646,7 @@ static JSC::EncodedJSValue systemErrorToErrorInstance(const SystemError* arg0, J } } - result->putDirect(vm, names.errnoPublicName(), jsNumber(err.errno_), JSC::PropertyAttribute::DontDelete | 0); + result->putDirect(vm, names.errnoPublicName(), systemErrorErrnoValue(err), JSC::PropertyAttribute::DontDelete | 0); return JSC::JSValue::encode(result); } @@ -2683,8 +2692,9 @@ JSC::EncodedJSValue SystemError__toErrorInstanceWithInfoObject(const SystemError info->putDirect(vm, clientData->builtinNames().codePublicName(), jsString(vm, codeString), JSC::PropertyAttribute::DontDelete | 0); info->putDirect(vm, vm.propertyNames->message, jsString(vm, messageString), JSC::PropertyAttribute::DontDelete | 0); - info->putDirect(vm, clientData->builtinNames().errnoPublicName(), jsNumber(err.errno_), JSC::PropertyAttribute::DontDelete | 0); - result->putDirect(vm, clientData->builtinNames().errnoPublicName(), jsNumber(err.errno_), JSC::PropertyAttribute::DontDelete | 0); + JSC::JSValue errnoValue = systemErrorErrnoValue(err); + info->putDirect(vm, clientData->builtinNames().errnoPublicName(), errnoValue, JSC::PropertyAttribute::DontDelete | 0); + result->putDirect(vm, clientData->builtinNames().errnoPublicName(), errnoValue, JSC::PropertyAttribute::DontDelete | 0); return JSC::JSValue::encode(result); } diff --git a/src/jsc/bindings/headers-handwritten.h b/src/jsc/bindings/headers-handwritten.h index 699d909ad7fd..f4a7f036f9d7 100644 --- a/src/jsc/bindings/headers-handwritten.h +++ b/src/jsc/bindings/headers-handwritten.h @@ -144,6 +144,7 @@ typedef struct ErrorableResolvedSource { } ErrorableResolvedSource; typedef struct SystemError { + /// MinInt (`SystemError::NO_ERRNO` in Rust) = the error gets `errno: undefined` int errno_; BunString code; BunString message; diff --git a/src/runtime/dns_jsc/cares_jsc.rs b/src/runtime/dns_jsc/cares_jsc.rs index 7492e72296e9..5beddf1441fd 100644 --- a/src/runtime/dns_jsc/cares_jsc.rs +++ b/src/runtime/dns_jsc/cares_jsc.rs @@ -640,6 +640,18 @@ fn any_reply_to_js( } // ── Error ────────────────────────────────────────────────────────────────── + +/// Node only gives a DNS error a numeric `errno` when libuv reported one +/// (`getaddrinfo`/`getnameinfo`). A c-ares resolver failure (`query*`, +/// `getHostByAddr`) only has a string `code`, so its `errno` is undefined. +fn errno_for_syscall(this: c_ares::Error, syscall: &[u8]) -> c_int { + if strings::has_prefix_comptime(syscall, b"query") || strings::eql(syscall, b"getHostByAddr") { + SystemError::NO_ERRNO + } else { + this as c_int + } +} + pub(crate) struct ErrorDeferred { pub errno: c_ares::Error, pub syscall: &'static [u8], @@ -679,7 +691,7 @@ impl ErrorDeferred { )) }; let system_error = SystemError { - errno: self.errno as i32, + errno: errno_for_syscall(self.errno, self.syscall), code: bstr::String::static_(code).into(), message: message.into(), syscall: bstr::String::clone_utf8(self.syscall).into(), @@ -765,7 +777,7 @@ pub(crate) fn error_to_js_with_syscall( ) -> JsResult { let code = this.code(); let instance = SystemError { - errno: this as i32, + errno: errno_for_syscall(this, syscall), code: bstr::String::static_(&code[4..]).into(), syscall: bstr::String::static_(syscall).into(), message: bstr::String::create_format(format_args!( @@ -797,7 +809,7 @@ pub(crate) fn system_error_with_syscall_and_hostname( ) -> SystemError { let code = this.code(); SystemError { - errno: this as i32, + errno: errno_for_syscall(this, syscall), code: bstr::String::static_(&code[4..]).into(), message: bstr::String::create_format(format_args!( "{} {} {}", diff --git a/test/js/node/dns/node-dns.test.js b/test/js/node/dns/node-dns.test.js index 0e0853c216c6..1bf8e42fad7c 100644 --- a/test/js/node/dns/node-dns.test.js +++ b/test/js/node/dns/node-dns.test.js @@ -157,6 +157,101 @@ test.skipIf(isWindows)("dns.resolveSrv accepts compressed target in RDATA", asyn } }); +// Node only sets a numeric `errno` on DNS errors when libuv reported a +// numeric error (getaddrinfo/getnameinfo). c-ares resolver failures carry a +// string code and leave `errno` undefined. https://github.com/oven-sh/bun/issues/37320 +test.skipIf(isWindows)("resolver query errors leave errno undefined, lookup errors keep a number", async () => { + const socket = dgram.createSocket("udp4"); + try { + socket.on("message", (query, rinfo) => { + // Reply NXDOMAIN to every query. + const res = Buffer.from(query); + res[2] = 0x81; // QR=1, RD=1 + res[3] = 0x83; // RA=1, RCODE=3 (NXDOMAIN) + res.fill(0, 6, 12); // ANCOUNT/NSCOUNT/ARCOUNT = 0 + socket.send(res, rinfo.port, rinfo.address); + }); + socket.bind(0, "127.0.0.1"); + await once(socket, "listening"); + const { port } = socket.address(); + + const servers = ["127.0.0.1:" + port]; + const resolver = new dns.Resolver({ timeout: 1000, tries: 1 }); + resolver.setServers(servers); + const promisesResolver = new dns.promises.Resolver({ timeout: 1000, tries: 1 }); + promisesResolver.setServers(servers); + + const unexpectedSuccess = new Error("expected the query to fail"); + const callbackError = query => + new Promise((resolve, reject) => query(err => (err ? resolve(err) : reject(unexpectedSuccess)))); + const promiseError = promise => + promise.then( + () => { + throw unexpectedSuccess; + }, + err => err, + ); + + const errors = { + resolve4: await callbackError(cb => resolver.resolve4("invalid.invalid", cb)), + resolveAny: await callbackError(cb => resolver.resolveAny("invalid.invalid", cb)), + reverse: await callbackError(cb => resolver.reverse("192.0.2.1", cb)), + promisesResolve4: await promiseError(promisesResolver.resolve4("invalid.invalid")), + promisesResolveAny: await promiseError(promisesResolver.resolveAny("invalid.invalid")), + promisesReverse: await promiseError(promisesResolver.reverse("192.0.2.1")), + // dns.lookup() rejects a name with a NUL byte before it reaches a resolver + // backend, so this getaddrinfo failure does not need the network either. + lookup: await promiseError(dns.promises.lookup("invalid.invalid\0")), + }; + + // Node keeps `errno` as an own property of the error and only leaves its + // value undefined, so check the property is still there. + const shapes = Object.fromEntries( + Object.entries(errors).map(([name, err]) => [ + name, + { + code: err.code, + syscall: err.syscall, + hostname: err.hostname, + errno: err.errno, + hasOwnErrno: Object.hasOwn(err, "errno"), + }, + ]), + ); + const query = syscall => ({ + code: "ENOTFOUND", + syscall, + hostname: "invalid.invalid", + errno: undefined, + hasOwnErrno: true, + }); + const reverse = { + code: "ENOTFOUND", + syscall: "getHostByAddr", + hostname: "192.0.2.1", + errno: undefined, + hasOwnErrno: true, + }; + expect(shapes).toEqual({ + resolve4: query("queryA"), + resolveAny: query("queryAny"), + reverse, + promisesResolve4: query("queryA"), + promisesResolveAny: query("queryAny"), + promisesReverse: reverse, + lookup: { + code: "ENOTFOUND", + syscall: "getaddrinfo", + hostname: "invalid.invalid\0", + errno: expect.any(Number), + hasOwnErrno: true, + }, + }); + } finally { + socket.close(); + } +}); + test("dns.resolveTxt (txt.socketify.dev)", () => { const { promise, resolve, reject } = Promise.withResolvers(); dns.resolveTxt("txt.socketify.dev", (err, results) => { @@ -404,6 +499,8 @@ test("dns.lookup bad (qedjp3f4q4jgjh4d6vaf3fd2hbfhg6upt2bscrfe.com)", () => { expect(err).not.toBeNull(); expect(err.syscall).toEqual("getaddrinfo"); expect(err.code).toEqual("ENOTFOUND"); + // Unlike resolver query errors, lookup errors keep a numeric errno in Node. + expect(err.errno).toBeNumber(); expect(address).toBeUndefined(); expect(family).toBeUndefined(); resolve();