Skip to content

fs: add disposable mkdtemp #58516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1308,6 +1308,37 @@ characters directly to the `prefix` string. For instance, given a directory
`prefix` must end with a trailing platform-specific path separator
(`require('node:path').sep`).
### `fsPromises.mkdtempDisposable(prefix[, options])`
<!-- YAML
added: REPLACEME
-->
* `prefix` {string|Buffer|URL}
* `options` {string|Object}
* `encoding` {string} **Default:** `'utf8'`
* Returns: {Promise} Fulfills with a Promise for an async-disposable Object:
* `path` {string} The path of the created directory.
* `remove` {AsyncFunction} A function which removes the created directory.
* `[Symbol.asyncDispose]` {AsyncFunction} The same as `remove`.
Comment on lines +1320 to +1323
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a pattern here like what we do with the various pseudo-classes for params/options cases in Web Crypto. https://nodejs.org/docs/latest/api/webcrypto.html#algorithm-parameters ... these aren't actual classes in the code but are documented as such to improve navigation and documentability in the docs. Essentially, while this new method is actually returning an anonymous object, it can still be documented as if it were a named class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this, but where should the docs go? fs has a "Common Objects" section but that says

The common objects are shared by all of the file system API variants (promise, callback, and synchronous).

which is not true of these objects - the sync and async versions return objects with a different shape, specific to that function and no other. My inclination would be to make a sub-heading within the fs.mkdtempDisposableSync and fsPromises.mkdtempDisposable sections, but that's not really how any of the other docs look.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just put them there in the Common Objects section. We can shift things around later if necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But really it's up to you. Wherever you feel most comfortable with it.

The resulting Promise holds an async-disposable object whose `path` property
holds the created directory path. When the object is disposed, the directory
and its contents will be removed asynchronously if it still exists. If the
directory cannot be deleted, disposal will throw an error. The object has an
async `remove()` method which will perform the same task.
Both this function and the disposal function on the resulting object are
async, so it should be used with `await` + `await using` as in
`await using dir = await fsPromises.mkdtempDisposable('prefix')`.
<!-- TODO: link MDN docs for disposables once https://github.com/mdn/content/pull/38027 lands -->
For detailed information, see the documentation of [`fsPromises.mkdtemp()`][].
The optional `options` argument can be a string specifying an encoding, or an
object with an `encoding` property specifying the character encoding to use.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really should be expanded to indicate what the encoding is used for. This might be an existing problem in the documentation.

### `fsPromises.open(path, flags[, mode])`
<!-- YAML
Expand Down Expand Up @@ -5902,6 +5933,36 @@ this API: [`fs.mkdtemp()`][].
The optional `options` argument can be a string specifying an encoding, or an
object with an `encoding` property specifying the character encoding to use.
### `fs.mkdtempDisposableSync(prefix[, options])`
<!-- YAML
added: REPLACEME
-->
* `prefix` {string|Buffer|URL}
* `options` {string|Object}
* `encoding` {string} **Default:** `'utf8'`
* Returns: {Object} A disposable object:
* `path` {string} The path of the created directory.
* `remove` {Function} A function which removes the created directory.
* `[Symbol.dispose]` {Function} The same as `remove`.
Returns a disposable object whose `path` property holds the created directory
path. When the object is disposed, the directory and its contents will be
removed if it still exists. If the directory cannot be deleted, disposal will
throw an error. The object has a `remove()` method which will perform the same
task.
<!-- TODO: link MDN docs for disposables once https://github.com/mdn/content/pull/38027 lands -->
For detailed information, see the documentation of [`fs.mkdtemp()`][].
There is no callback-based version of this API because it is designed for use
with the `using` syntax.
The optional `options` argument can be a string specifying an encoding, or an
object with an `encoding` property specifying the character encoding to use.
### `fs.opendirSync(path[, options])`
<!-- YAML
Expand Down Expand Up @@ -8494,6 +8555,7 @@ the file contents.
[`fs.writev()`]: #fswritevfd-buffers-position-callback
[`fsPromises.access()`]: #fspromisesaccesspath-mode
[`fsPromises.copyFile()`]: #fspromisescopyfilesrc-dest-mode
[`fsPromises.mkdtemp()`]: #fspromisesmkdtempprefix-options
[`fsPromises.open()`]: #fspromisesopenpath-flags-mode
[`fsPromises.opendir()`]: #fspromisesopendirpath-options
[`fsPromises.rm()`]: #fspromisesrmpath-options
Expand Down
32 changes: 32 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const {
StringPrototypeCharCodeAt,
StringPrototypeIndexOf,
StringPrototypeSlice,
SymbolDispose,
uncurryThis,
} = primordials;

Expand Down Expand Up @@ -3028,6 +3029,36 @@ function mkdtempSync(prefix, options) {
return binding.mkdtemp(prefix, options.encoding);
}

/**
* Synchronously creates a unique temporary directory.
* The returned value is a disposable object which removes the
* directory and its contents when disposed.
* @param {string | Buffer | URL} prefix
* @param {string | { encoding?: string; }} [options]
* @returns {object} A disposable object with a "path" property.
*/
function mkdtempDisposableSync(prefix, options) {
options = getOptions(options);

prefix = getValidatedPath(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);

const path = binding.mkdtemp(prefix, options.encoding);
// Stash the full path in case of process.chdir()
const fullPath = pathModule.resolve(process.cwd(), path);

const remove = () => {
binding.rmSync(fullPath, 0 /* maxRetries */, true /* recursive */, 100 /* retryDelay */);
};
return {
path,
remove,
[SymbolDispose]() {
remove();
},
};
}

/**
* Asynchronously copies `src` to `dest`. By
* default, `dest` is overwritten if it already exists.
Expand Down Expand Up @@ -3237,6 +3268,7 @@ module.exports = fs = {
mkdirSync,
mkdtemp,
mkdtempSync,
mkdtempDisposableSync,
open,
openSync,
openAsBlob,
Expand Down
34 changes: 34 additions & 0 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,39 @@ async function mkdtemp(prefix, options) {
);
}

async function mkdtempDisposable(prefix, options) {
options = getOptions(options);

prefix = getValidatedPath(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);

const cwd = process.cwd();
const path = await PromisePrototypeThen(
binding.mkdtemp(prefix, options.encoding, kUsePromises),
undefined,
handleErrorFromBinding,
);
// Stash the full path in case of process.chdir()
const fullPath = pathModule.resolve(cwd, path);

const remove = async () => {
const rmrf = lazyRimRaf();
await rmrf(fullPath, {
maxRetries: 0,
recursive: true,
retryDelay: 0,
});
};
return {
__proto__: null,
path,
remove,
async [SymbolAsyncDispose]() {
await remove();
},
};
}

async function writeFile(path, data, options) {
options = getOptions(options, {
encoding: 'utf8',
Expand Down Expand Up @@ -1293,6 +1326,7 @@ module.exports = {
lutimes,
realpath,
mkdtemp,
mkdtempDisposable,
writeFile,
appendFile,
readFile,
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/permission/fs-write.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER;
});
}

// fs.mkdtemp
{
assert.throws(() => {
fs.mkdtempSync(path.join(blockedFolder, 'any-folder'));
Expand All @@ -212,6 +213,16 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER;
}));
}

// fs.mkdtempDisposableSync
{
assert.throws(() => {
fs.mkdtempDisposableSync(path.join(blockedFolder, 'any-folder'));
},{
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}

// fs.rename
{
assert.throws(() => {
Expand Down
92 changes: 92 additions & 0 deletions test/parallel/test-fs-mkdtempDisposableSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { isMainThread } = require('worker_threads');

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

// Basic usage
{
const result = fs.mkdtempDisposableSync(tmpdir.resolve('foo.'));

assert.strictEqual(path.basename(result.path).length, 'foo.XXXXXX'.length);
assert.strictEqual(path.dirname(result.path), tmpdir.path);
assert(fs.existsSync(result.path));

result.remove();

assert(!fs.existsSync(result.path));

// Second removal does not throw error
result.remove();
}

// Usage with [Symbol.dispose]()
{
const result = fs.mkdtempDisposableSync(tmpdir.resolve('foo.'));

assert(fs.existsSync(result.path));

result[Symbol.dispose]();

assert(!fs.existsSync(result.path));

// Second removal does not throw error
result[Symbol.dispose]();
}

// `chdir`` does not affect removal
// Can't use chdir in workers
if (isMainThread) {
const originalCwd = process.cwd();

process.chdir(tmpdir.path);
const first = fs.mkdtempDisposableSync('first.');
const second = fs.mkdtempDisposableSync('second.');

const fullFirstPath = path.join(tmpdir.path, first.path);
const fullSecondPath = path.join(tmpdir.path, second.path);

assert(fs.existsSync(fullFirstPath));
assert(fs.existsSync(fullSecondPath));

process.chdir(fullFirstPath);
second.remove();

assert(!fs.existsSync(fullSecondPath));

process.chdir(tmpdir.path);
first.remove();
assert(!fs.existsSync(fullFirstPath));

process.chdir(originalCwd);
}

// Errors from cleanup are thrown
// It is difficult to arrange for rmdir to fail on windows
if (!common.isWindows) {
const base = fs.mkdtempDisposableSync(tmpdir.resolve('foo.'));

// On Unix we can prevent removal by making the parent directory read-only
const child = fs.mkdtempDisposableSync(path.join(base.path, 'bar.'));

const originalMode = fs.statSync(base.path).mode;
fs.chmodSync(base.path, 0o444);

assert.throws(() => {
child.remove();
}, /EACCES|EPERM/);

fs.chmodSync(base.path, originalMode);

// Removal works once permissions are reset
child.remove();
assert(!fs.existsSync(child.path));

base.remove();
assert(!fs.existsSync(base.path));
}
97 changes: 97 additions & 0 deletions test/parallel/test-fs-promises-mkdtempDisposable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const fsPromises = require('fs/promises');
const path = require('path');
const { isMainThread } = require('worker_threads');

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

async function basicUsage() {
const result = await fsPromises.mkdtempDisposable(tmpdir.resolve('foo.'));

assert.strictEqual(path.basename(result.path).length, 'foo.XXXXXX'.length);
assert.strictEqual(path.dirname(result.path), tmpdir.path);
assert(fs.existsSync(result.path));

await result.remove();

assert(!fs.existsSync(result.path));

// Second removal does not throw error
result.remove();
}

async function symbolAsyncDispose() {
const result = await fsPromises.mkdtempDisposable(tmpdir.resolve('foo.'));

assert(fs.existsSync(result.path));

await result[Symbol.asyncDispose]();

assert(!fs.existsSync(result.path));

// Second removal does not throw error
await result[Symbol.asyncDispose]();
}

async function chdirDoesNotAffectRemoval() {
// Can't use chdir in workers
if (!isMainThread) return;

const originalCwd = process.cwd();

process.chdir(tmpdir.path);
const first = await fsPromises.mkdtempDisposable('first.');
const second = await fsPromises.mkdtempDisposable('second.');

const fullFirstPath = path.join(tmpdir.path, first.path);
const fullSecondPath = path.join(tmpdir.path, second.path);

assert(fs.existsSync(fullFirstPath));
assert(fs.existsSync(fullSecondPath));

process.chdir(fullFirstPath);
await second.remove();

assert(!fs.existsSync(fullSecondPath));

process.chdir(tmpdir.path);
await first.remove();
assert(!fs.existsSync(fullFirstPath));

process.chdir(originalCwd);
}

async function errorsAreReThrown() {
// It is difficult to arrange for rmdir to fail on windows
if (common.isWindows) return;
const base = await fsPromises.mkdtempDisposable(tmpdir.resolve('foo.'));

// On Unix we can prevent removal by making the parent directory read-only
const child = await fsPromises.mkdtempDisposable(path.join(base.path, 'bar.'));

const originalMode = fs.statSync(base.path).mode;
fs.chmodSync(base.path, 0o444);

await assert.rejects(child.remove(), /EACCES|EPERM/);

fs.chmodSync(base.path, originalMode);

// Removal works once permissions are reset
await child.remove();
assert(!fs.existsSync(child.path));

await base.remove();
assert(!fs.existsSync(base.path));
}

(async () => {
await basicUsage();
await symbolAsyncDispose();
await chdirDoesNotAffectRemoval();
await errorsAreReThrown();
})().then(common.mustCall());
Loading
Loading