From b9df2e314f1d02d3ea4802479016b603bf059837 Mon Sep 17 00:00:00 2001 From: vedchaudhari Date: Thu, 15 Oct 2026 15:21:02 +0530 Subject: [PATCH] fs: restore stack traces of fs/promises readFile and writeFile errors Errors from readFile(), writeFile() and appendFile() have had no stack frames since they started using a single thread pool round trip, as the stack was captured from a C++ callback with no JS frames on it. Capture it after the await instead, like the rest of fs/promises. Refs: https://github.com/nodejs/node/pull/65327 Refs: https://github.com/nodejs/node/pull/65489 Assisted-by: claude:opus-5.5 Signed-off-by: vedchaudhari --- lib/internal/fs/promises.js | 58 +++++++++++++------ test/parallel/test-fs-promises.js | 25 ++++++++ ...test-permission-fs-promises-stack-trace.js | 42 ++++++++++++++ 3 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-permission-fs-promises-stack-trace.js diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 47574a5a4711..32eaac3a51e3 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -2226,8 +2226,14 @@ async function writeFile(path, data, options) { if (!flush && !isCustomIterable(data) && data.byteLength <= kWriteFileMaxChunkSize) { path = getValidatedPath(path); - await writeFileInOneRoundTrip(path, stringToFlags(flag, 'options.flag'), - parseFileMode(options.mode, 'mode', 0o666), data); + const flagsNumber = stringToFlags(flag, 'options.flag'); + const mode = parseFileMode(options.mode, 'mode', 0o666); + try { + await writeFileInOneRoundTrip(path, flagsNumber, mode, data); + } catch (err) { + ErrorCaptureStackTrace(err); + throw err; + } checkAborted(options.signal); // An abort during the write still wins. return; } @@ -2244,6 +2250,10 @@ async function writeFile(path, data, options) { /** * Open + write + close as one thread pool round trip. + * Throws synchronously if the permission model denies access, while the + * caller's frames are still on the stack. The caller is responsible for + * capturing the stack trace of a rejection, as no JS frames are on the stack + * when the job completes. * @param {string|Buffer} path Validated path * @param {number} flagsNumber * @param {number} mode @@ -2251,22 +2261,21 @@ async function writeFile(path, data, options) { * @returns {Promise} */ function writeFileInOneRoundTrip(path, flagsNumber, mode, data) { - return new Promise((resolve, reject) => { - const job = new binding.WriteFileJob(path, flagsNumber, mode, data); + const job = new binding.WriteFileJob(path, flagsNumber, mode, data); + const promise = new Promise((resolve, reject) => { job.ondone = (err) => { if (err != null) { - ErrorCaptureStackTrace(err, writeFileInOneRoundTrip); reject(err); } else { resolve(); } }; - const accessError = job.run(path); - if (accessError !== undefined) { - ErrorCaptureStackTrace(accessError, writeFileInOneRoundTrip); - reject(accessError); - } }); + const accessError = job.run(path); + if (accessError !== undefined) { + throw accessError; + } + return promise; } function isCustomIterable(obj) { @@ -2309,7 +2318,15 @@ async function readFile(path, options) { // up to one chunk; larger files come back as an open fd + size and are // read by readFileHandle() as before. path = getValidatedPath(path); - const { 0: buffer, 1: fd, 2: size } = await readFileInOneRoundTrip(path, stringToFlags(flag)); + const flagsNumber = stringToFlags(flag); + let result; + try { + result = await readFileInOneRoundTrip(path, flagsNumber); + } catch (err) { + ErrorCaptureStackTrace(err); + throw err; + } + const { 0: buffer, 1: fd, 2: size } = result; if (fd === -1) { checkAborted(options.signal); // An abort during the read still wins. return options.encoding ? buffer.toString(options.encoding) : buffer; @@ -2323,28 +2340,31 @@ async function readFile(path, options) { } /** + * Throws synchronously if the permission model denies access, while the + * caller's frames are still on the stack. The caller is responsible for + * capturing the stack trace of a rejection, as no JS frames are on the stack + * when the job completes. * @param {string|Buffer} path Validated path * @param {number} flagsNumber * @returns {Promise<[Buffer|undefined, number, number|undefined]>} [buffer, -1] or [undefined, fd, size] */ function readFileInOneRoundTrip(path, flagsNumber) { - return new Promise((resolve, reject) => { - const job = new binding.ReadFileJob(path, flagsNumber, kReadFileBufferLength); + const job = new binding.ReadFileJob(path, flagsNumber, kReadFileBufferLength); + const promise = new Promise((resolve, reject) => { job.ondone = (err, buffer, fd, size, closeErr) => { const error = err ?? closeErr; if (error != null) { - ErrorCaptureStackTrace(error, readFileInOneRoundTrip); reject(error); } else { resolve([buffer, fd, size]); } }; - const accessError = job.run(path); - if (accessError !== undefined) { - ErrorCaptureStackTrace(accessError, readFileInOneRoundTrip); - reject(accessError); - } }); + const accessError = job.run(path); + if (accessError !== undefined) { + throw accessError; + } + return promise; } async function* _watch(filename, options = kEmptyObject) { diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index 07f01187e89b..6ddc4cd3cb36 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -9,6 +9,7 @@ const fs = require('fs'); const fsPromises = fs.promises; const { access, + appendFile, chmod, chown, copyFile, @@ -62,6 +63,30 @@ assert.strictEqual( } ).then(common.mustCall()); + assert.rejects( + readFile('this file does not exist'), + { + code: 'ENOENT', + stack: /at async assert\.rejects/ + } + ).then(common.mustCall()); + + assert.rejects( + writeFile(path.join('this dir does not exist', 'file'), 'data'), + { + code: 'ENOENT', + stack: /at async assert\.rejects/ + } + ).then(common.mustCall()); + + assert.rejects( + appendFile(path.join('this dir does not exist', 'file'), 'data'), + { + code: 'ENOENT', + stack: /at async assert\.rejects/ + } + ).then(common.mustCall()); + assert.rejects( access(__filename, 8), { diff --git a/test/parallel/test-permission-fs-promises-stack-trace.js b/test/parallel/test-permission-fs-promises-stack-trace.js new file mode 100644 index 000000000000..7160473173ea --- /dev/null +++ b/test/parallel/test-permission-fs-promises-stack-trace.js @@ -0,0 +1,42 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +// The permission model denies access before the first await, so the stack +// still has the frames of a caller that does not await the promise. +const { status, stderr } = spawnSync( + process.execPath, + [ + '--permission', + '-e', + ` + const assert = require('assert'); + const { readFile, writeFile, appendFile } = require('fs/promises'); + const target = ${JSON.stringify(tmpdir.resolve('file'))}; + + function readCaller() { return readFile(target); } + function writeCaller() { return writeFile(target, 'data'); } + function appendCaller() { return appendFile(target, 'data'); } + async function awaitAppendCaller() { await appendFile(target, 'data'); } + + const cases = [ + [readCaller, /at readCaller /], + [writeCaller, /at writeCaller /], + [appendCaller, /at appendCaller /], + [awaitAppendCaller, /at awaitAppendCaller /], + ]; + let checked = 0; + for (const [caller, stack] of cases) { + assert.rejects(caller(), { code: 'ERR_ACCESS_DENIED', stack }) + .then(() => checked++); + } + process.on('exit', () => assert.strictEqual(checked, cases.length)); + `, + ], +); +assert.strictEqual(status, 0, stderr.toString());