fix(runtime): Node-shaped errors from Buffer-mode fs reads and read streams

changelog.d/10539-fs-read-error-shapes.md

@@ -0,0 +1,44 @@
+**Buffer-mode `fs` reads throw on failure, and read-stream errors carry Node's
+`code`/`errno`/`syscall`/`path`** (#10452, #10451).
+
+`fs.readFileSync(path)`, `readFileSync(path, {})`, `fs.promises.readFile(path)`
+and `import { readFile } from "node:fs/promises"` returned or resolved
+`null`/`undefined` for a missing file. `try { cfg = readFileSync(optional) }
+catch { defaults }` took the wrong branch, and `.catch(e => e.code === "ENOENT")`
+never ran. The `'utf8'` and callback forms did report the error.
+
+Every `readFile` form reads through `read_file_bytes_with_options`, which folded
+each failure into `None`. The Buffer entry points
+(`js_fs_read_file_binary{,_options}`) turned that into a null `BufferHeader`, so
+the value was `null` or `undefined` depending on the call shape. The string
+entry point re-read the file to get an `io::Error` back and always reported it
+as `open`. The reader now returns the OS error and the syscall that failed, and
+every form reports it the way Node does: ENOENT or EACCES as `open '<path>'`,
+and a directory as `EISDIR: illegal operation on a directory, read` with no
+path. Before, a directory read returned a Buffer or reported `open`. The
+callback form now reads once. Its old `stat` pre-check missed directories, so a
+directory gave `(null, undefined)` in Buffer mode and a synchronous throw in
+string mode. `FileHandle.readFile()` now rejects when the read fails.
+
+`fs.createReadStream` open and read failures emitted a plain `Error` with only
+the Rust message (`No such file or directory (os error 2)`) and no own
+properties. The write side already stored a Node-shaped `error_value` (#9493).
+The read side now does too. The constructor's open failure is held as the OS
+error until the stream state is registered (the registry is the GC root), then
+turned into the error value. The pump's read failures are stored the same way.
+`fs.writeFile(dest, failingReadStream)` also rejects with that error now. The
+stream error helpers moved to `fs/stream/stream_errors.rs` so `stream.rs` stays
+under the 2000-line cap.
+
+fs error messages used Rust's `Display` text. They now use libuv's description
+of the errno (`ENOENT: no such file or directory, open '<path>'`), as Node
+does, for every error built by `build_fs_error_value*`.
+
+Validation: `test_gap_10452_fs_read_error_shapes` covers sync, callback and
+promise reads (with and without an encoding), `FileHandle.readFile`, and read
+and write streams, for a missing file and a directory, plus successful-read
+controls. It matches Node byte for byte and fails on the parent commit. EACCES
+was checked by hand as an unprivileged user. New `fs::errors` unit tests cover
+the libuv text and the open-vs-read failure shape. User-space instructions for
+50k `readFileSync` calls on a small file: Buffer form −0.6 %, UTF-8 form −9.5 %
+(it no longer decodes the path twice per call).

crates/perry-runtime/src/fs/callbacks.rs

@@ -10,34 +10,18 @@ pub extern "C" fn js_fs_read_file_callback(path_value: f64, encoding: f64, callb
     const TAG_NULL: u64 = 0x7FFC_0000_0000_0002;
 
     let cb_ptr = callback_from_options_arg(encoding, callback);
-    unsafe {
-        if let Some(err_val) = fs_callback_read_error(path_value, "open") {
-            if !cb_ptr.is_null() {
-                defer_fs_callback_chain(cb_ptr, &[err_val, f64::from_bits(TAG_UNDEFINED)], 4);
-            }
-            return f64::from_bits(TAG_UNDEFINED);
-        }
-    }
     let encoding_is_callback = !extract_closure_ptr(encoding).is_null();
     let want_buffer = encoding_is_callback || read_file_encoding(encoding).is_none();
-    let data_val = if want_buffer {
-        let buf = js_fs_read_file_binary_options(path_value, encoding);
-        if buf.is_null() {
-            f64::from_bits(TAG_UNDEFINED)
-        } else {
-            f64::from_bits(crate::value::JSValue::pointer(buf as *const u8).bits())
-        }
-    } else {
-        let str_ptr = js_fs_read_file_sync_options(path_value, encoding);
-        if str_ptr.is_null() {
-            f64::from_bits(TAG_UNDEFINED)
-        } else {
-            f64::from_bits(crate::value::js_nanbox_string(str_ptr as i64).to_bits())
-        }
+    // One read decides both arms. The old pre-flight `stat` probe only saw a
+    // missing path, so a directory reached the reader, whose failure became
+    // `(null, undefined)` for a Buffer and a synchronous throw for a string
+    // (#10452); Node reports `EISDIR ... read` through the callback.
+    let args = match unsafe { read_file_value_result(path_value, encoding, !want_buffer) } {
+        Ok(data_val) => [f64::from_bits(TAG_NULL), data_val],
+        Err(err_val) => [err_val, f64::from_bits(TAG_UNDEFINED)],
     };
-
     if !cb_ptr.is_null() {
-        defer_fs_callback_chain(cb_ptr, &[f64::from_bits(TAG_NULL), data_val], 4);
+        defer_fs_callback_chain(cb_ptr, &args, 4);
     }
     f64::from_bits(TAG_UNDEFINED)
 }

crates/perry-runtime/src/fs/errors.rs

@@ -4,6 +4,15 @@
 use super::*;
 
 pub(crate) fn io_error_code(err: &std::io::Error) -> &'static str {
+    // #10539 review: on Windows `raw_os_error` is a Win32 code, not an errno,
+    // so it goes through libuv's own translation table.
+    #[cfg(windows)]
+    if let Some((_, code)) = err
+        .raw_os_error()
+        .and_then(crate::util_syserr::win32_error_to_uv)
+    {
+        return code;
+    }
     #[cfg(unix)]
     if let Some(raw) = err.raw_os_error() {
         match raw {
@@ -59,6 +68,13 @@ pub(crate) fn io_error_errno(err: &std::io::Error) -> i32 {
     if let Some(raw) = err.raw_os_error() {
         return -raw;
     }
+    // Windows has no errno to negate: libuv gives each code a fixed negative
+    // number there (`ENOENT` is -4058, not -2), and that is what node reports.
+    #[cfg(windows)]
+    {
+        const UV_WINDOWS_EIO: i32 = -4070;
+        return crate::util_syserr::uv_windows_errno(io_error_code(err)).unwrap_or(UV_WINDOWS_EIO);
+    }
     #[cfg(unix)]
     match io_error_code(err) {
         "ENOENT" => -libc::ENOENT,
@@ -84,7 +100,7 @@ pub(crate) fn io_error_errno(err: &std::io::Error) -> i32 {
         "EXDEV" => -libc::EXDEV,
         _ => -libc::EIO,
     }
-    #[cfg(not(unix))]
+    #[cfg(not(any(unix, windows)))]
     match io_error_code(err) {
         "ENOENT" => -2,
         "EACCES" => -13,
@@ -150,14 +166,29 @@ unsafe fn attach_fs_error_props(
     }
 }
 
+/// The description Node puts in an fs error message: libuv's fixed lowercase
+/// phrasing for the errno ("no such file or directory"), not Rust's `Display`,
+/// which reads "No such file or directory (os error 2)" (#10451). An error
+/// synthesized without an OS errno keeps its own text.
+fn fs_error_description(err: &std::io::Error) -> String {
+    if err.raw_os_error().is_some() {
+        let code = io_error_errno(err) as i64;
+        if let Some(message) = crate::util_syserr::system_error_message_for_code(code) {
+            return message.to_string();
+        }
+    }
+    err.to_string()
+}
+
 pub(crate) unsafe fn build_fs_error_value(
     err: &std::io::Error,
     syscall: &'static str,
     path: &str,
 ) -> f64 {
     let code = io_error_code(err);
     let errno = io_error_errno(err);
-    let msg = format!("{}: {}, {} '{}'", code, err, syscall, path);
+    let desc = fs_error_description(err);
+    let msg = format!("{}: {}, {} '{}'", code, desc, syscall, path);
     let msg_ptr = js_string_from_bytes(msg.as_ptr(), msg.len() as u32);
     let err_ptr = crate::error::js_error_new_with_message(msg_ptr);
     attach_fs_error_props(err_ptr, code, errno, syscall, Some(path), None);
@@ -175,7 +206,8 @@ pub(crate) unsafe fn build_fs_error_value_with_dest(
 ) -> f64 {
     let code = io_error_code(err);
     let errno = io_error_errno(err);
-    let msg = format!("{}: {}, {} '{}' -> '{}'", code, err, syscall, path, dest);
+    let desc = fs_error_description(err);
+    let msg = format!("{}: {}, {} '{}' -> '{}'", code, desc, syscall, path, dest);
     let msg_ptr = js_string_from_bytes(msg.as_ptr(), msg.len() as u32);
     let err_ptr = crate::error::js_error_new_with_message(msg_ptr);
     attach_fs_error_props(err_ptr, code, errno, syscall, Some(path), Some(dest));
@@ -188,13 +220,73 @@ pub(crate) unsafe fn build_fs_error_value_no_path(
 ) -> f64 {
     let code = io_error_code(err);
     let errno = io_error_errno(err);
-    let msg = format!("{}: {}, {}", code, err, syscall);
+    let desc = fs_error_description(err);
+    let msg = format!("{}: {}, {}", code, desc, syscall);
     let msg_ptr = js_string_from_bytes(msg.as_ptr(), msg.len() as u32);
     let err_ptr = crate::error::js_error_new_with_message(msg_ptr);
     attach_fs_error_props(err_ptr, code, errno, syscall, None, None);
     crate::value::js_nanbox_pointer(err_ptr as i64)
 }
 
+/// An OS "no such file or directory" error. `libc::ENOENT` is 2 on Windows too,
+/// where `from_raw_os_error` reads it as `ERROR_FILE_NOT_FOUND` — which libuv
+/// also translates to `ENOENT`.
+pub(crate) fn enoent_os_error() -> std::io::Error {
+    std::io::Error::from_raw_os_error(libc::ENOENT)
+}
+
+/// An OS "bad file descriptor" error. On Windows this must be
+/// `ERROR_INVALID_HANDLE`, the Win32 error libuv translates to `EBADF`:
+/// `libc::EBADF` (9) is `ERROR_INVALID_BLOCK` there and translates to nothing.
+pub(crate) fn ebadf_os_error() -> std::io::Error {
+    #[cfg(windows)]
+    {
+        const ERROR_INVALID_HANDLE: i32 = 6;
+        std::io::Error::from_raw_os_error(ERROR_INVALID_HANDLE)
+    }
+    #[cfg(not(windows))]
+    {
+        std::io::Error::from_raw_os_error(libc::EBADF)
+    }
+}
+
+/// A failed file read (`readFile`, a read stream): the OS error plus the
+/// syscall Node reports it under. Node opens before it reads, so a missing file
+/// fails the `open` and names the path, while a directory opens fine and fails
+/// the `read`, which Node reports without a path
+/// (`EISDIR: illegal operation on a directory, read`).
+pub(crate) struct FsReadFailure {
+    pub(super) err: std::io::Error,
+    syscall: &'static str,
+    path: Option<String>,
+}
+
+impl FsReadFailure {
+    pub(crate) fn open(err: std::io::Error, path: &str) -> Self {
+        let path = Some(path.to_string());
+        Self {
+            err,
+            syscall: "open",
+            path,
+        }
+    }
+
+    pub(crate) fn read(err: std::io::Error) -> Self {
+        Self {
+            err,
+            syscall: "read",
+            path: None,
+        }
+    }
+
+    pub(crate) unsafe fn error_value(&self) -> f64 {
+        match &self.path {
+            Some(path) => build_fs_error_value(&self.err, self.syscall, path),
+            None => build_fs_error_value_no_path(&self.err, self.syscall),
+        }
+    }
+}
+
 /// Probe a path for read access and produce a NaN-boxed Error if the
 /// underlying syscall would fail. Returns `None` on success.
 pub(crate) unsafe fn fs_callback_read_error(path_value: f64, syscall: &'static str) -> Option<f64> {
@@ -237,3 +329,65 @@ pub(crate) unsafe fn fs_callback_write_parent_error(
         Err(err) => Some(build_fs_error_value(&err, syscall, &path)),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// #10451: an fs error message carries libuv's description of the errno,
+    /// as Node's does, not Rust's "No such file or directory (os error 2)".
+    #[cfg(unix)]
+    #[test]
+    fn fs_error_description_uses_libuv_text() {
+        let described = |errno| fs_error_description(&std::io::Error::from_raw_os_error(errno));
+        assert_eq!(described(libc::ENOENT), "no such file or directory");
+        assert_eq!(described(libc::EISDIR), "illegal operation on a directory");
+        assert_eq!(described(libc::EACCES), "permission denied");
+        // A synthesized error has no errno to describe and keeps its own text.
+        let custom = std::io::Error::new(std::io::ErrorKind::NotFound, "parent is not a directory");
+        assert_eq!(fs_error_description(&custom), "parent is not a directory");
+    }
+
+    /// #10452: every `readFile` form reads through `read_file_bytes_with_options`,
+    /// whose failures used to be a bare `None` the Buffer forms returned as
+    /// `null`/`undefined`. A missing file must fail the `open` and name the path;
+    /// a directory opens and must fail the `read`, which Node reports pathless.
+    #[cfg(unix)]
+    #[test]
+    fn read_file_failures_keep_the_os_error_and_failing_syscall() {
+        let _global = crate::gc::global_side_table_test_lock();
+        let dir =
+            std::env::temp_dir().join(format!("perry_fs_read_failure_{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let file = dir.join("ok.txt");
+        std::fs::write(&file, b"hello").unwrap();
+        let missing = dir.join("missing.txt");
+        let path_value = |path: &std::path::Path| {
+            let path = path.to_str().unwrap();
+            let ptr = js_string_from_bytes(path.as_ptr(), path.len() as u32);
+            crate::value::js_nanbox_string(ptr as i64)
+        };
+        let undefined = f64::from_bits(crate::value::TAG_UNDEFINED);
+
+        let failure = read_file_bytes_with_options(path_value(&missing), undefined)
+            .err()
+            .expect("a missing file must fail");
+        assert_eq!(failure.err.raw_os_error(), Some(libc::ENOENT));
+        assert_eq!(
+            (failure.syscall, failure.path.as_deref()),
+            ("open", missing.to_str())
+        );
+
+        let failure = read_file_bytes_with_options(path_value(&dir), undefined)
+            .err()
+            .expect("a directory must fail");
+        assert_eq!(failure.err.raw_os_error(), Some(libc::EISDIR));
+        assert_eq!((failure.syscall, failure.path.as_deref()), ("read", None));
+
+        let bytes = read_file_bytes_with_options(path_value(&file), undefined)
+            .ok()
+            .expect("a regular file reads");
+        assert_eq!(bytes, b"hello");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+}

crates/perry-runtime/src/fs/filehandle.rs

@@ -1144,7 +1144,11 @@ pub(crate) extern "C" fn filehandle_read_file_impl(
             return promise_value_fs(f64::from_bits(crate::value::TAG_UNDEFINED));
         };
         let mut bytes = Vec::new();
-        let _ = file.read_to_end(&mut bytes);
+        if let Err(err) = file.read_to_end(&mut bytes) {
+            // #10452: a failed read rejects (`EISDIR ... read` for a directory)
+            // instead of resolving with whatever was read before it failed.
+            return promise_rejected_fs(unsafe { build_fs_error_value_no_path(&err, "read") });
+        }
         if read_file_encoding(encoding).is_none() {
             let buf = crate::buffer::js_buffer_alloc(bytes.len() as i32, 0);
             if !buf.is_null() {