From c0251dd2ce5638c2c8534b873022ae091d14e137 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 20 Sep 2026 18:52:06 +0000 Subject: [PATCH 1/2] fix: do not throw when fs.close is getter-only Bundlers and ESM interop (Vite/esbuild) expose fs methods as getter-only properties, so `fs.close = ...` throws TypeError. Only assign when the property is writable or configurable, and materialize getter-only properties on the cloned export so it can still be patched. Fixes #257 Co-authored-by: David --- clone.js | 14 +++- graceful-fs.js | 130 +++++++++++++++++++++++++++++--------- test/close-getter-only.js | 75 ++++++++++++++++++++++ 3 files changed, 187 insertions(+), 32 deletions(-) create mode 100644 test/close-getter-only.js diff --git a/clone.js b/clone.js index dff3cc8..60f5178 100644 --- a/clone.js +++ b/clone.js @@ -16,7 +16,19 @@ function clone (obj) { var copy = Object.create(null) Object.getOwnPropertyNames(obj).forEach(function (key) { - Object.defineProperty(copy, key, Object.getOwnPropertyDescriptor(obj, key)) + var desc = Object.getOwnPropertyDescriptor(obj, key) + // Getter-only properties (common on bundler/ESM `fs` shims) cannot + // be assigned on the clone either unless we materialize them. + if (desc && desc.get && !desc.set) { + Object.defineProperty(copy, key, { + value: obj[key], + writable: true, + enumerable: desc.enumerable, + configurable: true + }) + } else { + Object.defineProperty(copy, key, desc) + } }) return copy diff --git a/graceful-fs.js b/graceful-fs.js index 8d5b89e..a88246d 100644 --- a/graceful-fs.js +++ b/graceful-fs.js @@ -39,6 +39,98 @@ else if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) console.error(m) } +// Some bundlers (Vite/esbuild) and ESM interop layers expose `fs` +// methods as getter-only properties. Assigning then throws +// "Cannot set property close of # which has only a getter". +function setProp (obj, key, value) { + var desc + try { + desc = Object.getOwnPropertyDescriptor(obj, key) + } catch (er) {} + + if (desc) { + if (desc.writable || typeof desc.set === 'function') { + try { + obj[key] = value + return true + } catch (er) { + return false + } + } + if (desc.configurable) { + try { + Object.defineProperty(obj, key, { + value: value, + writable: true, + enumerable: desc.enumerable, + configurable: true + }) + return true + } catch (er) { + return false + } + } + return false + } + + try { + obj[key] = value + return true + } catch (er) { + try { + Object.defineProperty(obj, key, { + value: value, + writable: true, + enumerable: true, + configurable: true + }) + return true + } catch (er2) { + return false + } + } +} + +function patchClose (target) { + var fs$close = target.close + if (typeof fs$close !== 'function' || fs$close[previousSymbol]) + return + + function close (fd, cb) { + return fs$close.call(fs, fd, function (err) { + // This function uses the graceful-fs shared queue + if (!err) { + resetQueue() + } + + if (typeof cb === 'function') + cb.apply(this, arguments) + }) + } + + Object.defineProperty(close, previousSymbol, { + value: fs$close + }) + setProp(target, 'close', close) +} + +function patchCloseSync (target) { + var fs$closeSync = target.closeSync + if (typeof fs$closeSync !== 'function' || fs$closeSync[previousSymbol]) + return + + function closeSync (fd) { + // This function uses the graceful-fs shared queue + fs$closeSync.apply(fs, arguments) + resetQueue() + } + + Object.defineProperty(closeSync, previousSymbol, { + value: fs$closeSync + }) + setProp(target, 'closeSync', closeSync) +} + // Once time initialization if (!fs[gracefulQueue]) { // This queue can be shared by multiple loaded instances @@ -49,37 +141,8 @@ if (!fs[gracefulQueue]) { // to retry() whenever a close happens *anywhere* in the program. // This is essential when multiple graceful-fs instances are // in play at the same time. - fs.close = (function (fs$close) { - function close (fd, cb) { - return fs$close.call(fs, fd, function (err) { - // This function uses the graceful-fs shared queue - if (!err) { - resetQueue() - } - - if (typeof cb === 'function') - cb.apply(this, arguments) - }) - } - - Object.defineProperty(close, previousSymbol, { - value: fs$close - }) - return close - })(fs.close) - - fs.closeSync = (function (fs$closeSync) { - function closeSync (fd) { - // This function uses the graceful-fs shared queue - fs$closeSync.apply(fs, arguments) - resetQueue() - } - - Object.defineProperty(closeSync, previousSymbol, { - value: fs$closeSync - }) - return closeSync - })(fs.closeSync) + patchClose(fs) + patchCloseSync(fs) if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) { process.on('exit', function() { @@ -365,6 +428,11 @@ function patch (fs) { } } + // If the real `fs.close` was getter-only and could not be assigned, + // still wrap close on this lookalike so EMFILE retries keep working. + patchClose(fs) + patchCloseSync(fs) + return fs } diff --git a/test/close-getter-only.js b/test/close-getter-only.js new file mode 100644 index 0000000..20fd832 --- /dev/null +++ b/test/close-getter-only.js @@ -0,0 +1,75 @@ +var fs = require('fs') +var path = require('path') +var test = require('tap').test +var origClose = fs.close +var origCloseSync = fs.closeSync +var globalPatch = !!process.env.TEST_GRACEFUL_FS_GLOBAL_PATCH +var self = path.resolve(__filename) + +function defineGetterOnly (name, fn, configurable) { + Object.defineProperty(fs, name, { + get: function () { return fn }, + enumerable: true, + configurable: configurable + }) +} + +if (process.argv.indexOf('--nonconfig') !== -1) { + defineGetterOnly('close', origClose, false) + defineGetterOnly('closeSync', origCloseSync, false) + + test('non-configurable getter-only close does not throw', function (t) { + var gfs + t.doesNotThrow(function () { + gfs = require('../') + }) + t.equal(fs.close, origClose, 'real fs.close left unchanged') + t.equal(fs.closeSync, origCloseSync, 'real fs.closeSync left unchanged') + if (!globalPatch) { + t.match(gfs.close.toString(), /graceful-fs shared queue/, + 'export close still patched') + t.match(gfs.closeSync.toString(), /graceful-fs shared queue/, + 'export closeSync still patched') + } + t.end() + }) +} else { + test('clone materializes getter-only properties as writable', function (t) { + var clone = require('../clone.js') + var src = {} + var fn = function () { return 1 } + Object.defineProperty(src, 'close', { + get: function () { return fn }, + enumerable: true + }) + var copy = clone(src) + t.equal(copy.close, fn) + var replacement = function () { return 2 } + copy.close = replacement + t.equal(copy.close, replacement, 'clone close is writable') + t.end() + }) + + defineGetterOnly('close', origClose, true) + defineGetterOnly('closeSync', origCloseSync, true) + + test('configurable getter-only close is patched', function (t) { + var gfs = require('../') + t.match(fs.close.toString(), /graceful-fs shared queue/, 'patch fs.close') + t.match(fs.closeSync.toString(), /graceful-fs shared queue/, + 'patch fs.closeSync') + t.match(gfs.close.toString(), /graceful-fs shared queue/, 'patch gfs.close') + t.match(gfs.closeSync.toString(), /graceful-fs shared queue/, + 'patch gfs.closeSync') + t.end() + }) + + test('non-configurable getter-only close', function (t) { + var env = {} + Object.keys(process.env).forEach(function (k) { + env[k] = process.env[k] + }) + t.spawn(process.execPath, [self, '--nonconfig'], { env: env }) + t.end() + }) +} From eafe1b2deb0bdf41555190b965b38dedb0bf3c74 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 20 Sep 2026 18:53:54 +0000 Subject: [PATCH 2/2] test: assert getter-only close still opens and closes files Co-authored-by: David --- test/close-getter-only.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/close-getter-only.js b/test/close-getter-only.js index 20fd832..fe85983 100644 --- a/test/close-getter-only.js +++ b/test/close-getter-only.js @@ -61,7 +61,16 @@ if (process.argv.indexOf('--nonconfig') !== -1) { t.match(gfs.close.toString(), /graceful-fs shared queue/, 'patch gfs.close') t.match(gfs.closeSync.toString(), /graceful-fs shared queue/, 'patch gfs.closeSync') - t.end() + + var fd = gfs.openSync(__filename, 'r') + t.doesNotThrow(function () { gfs.closeSync(fd) }, 'patched closeSync works') + gfs.open(__filename, 'r', function (er, fd2) { + t.error(er) + gfs.close(fd2, function (er2) { + t.error(er2, 'patched close works') + t.end() + }) + }) }) test('non-configurable getter-only close', function (t) {