-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
fs: support abortsignal in writeFile #35993
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4385,6 +4385,10 @@ details. | |
<!-- YAML | ||
added: v0.1.29 | ||
changes: | ||
- version: REPLACEME | ||
pr-url: https://github.com/nodejs/node/pull/35993 | ||
description: The options argument may include an AbortSignal to abort an | ||
ongoing writeFile request. | ||
- version: v14.12.0 | ||
pr-url: https://github.com/nodejs/node/pull/34993 | ||
description: The `data` parameter will stringify an object with an | ||
|
@@ -4419,6 +4423,7 @@ changes: | |
* `encoding` {string|null} **Default:** `'utf8'` | ||
* `mode` {integer} **Default:** `0o666` | ||
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`. | ||
* `signal` {AbortSignal} allows aborting an in-progress writeFile | ||
* `callback` {Function} | ||
* `err` {Error} | ||
|
||
|
@@ -4450,6 +4455,28 @@ It is unsafe to use `fs.writeFile()` multiple times on the same file without | |
waiting for the callback. For this scenario, [`fs.createWriteStream()`][] is | ||
recommended. | ||
|
||
Similarly to `fs.readFile` - `fs.writeFile` is a convenience method that | ||
performs multiple `write` calls internally to write the buffer passed to it. | ||
For performance sensitive code consider using [`fs.createWriteStream()`][]. | ||
|
||
It is possible to use an {AbortSignal} to cancel an `fs.writeFile()`. | ||
Cancelation is "best effort", and some amount of data is likely still | ||
to be written. | ||
|
||
```js | ||
const controller = new AbortController(); | ||
const { signal } = controller; | ||
const data = new Uint8Array(Buffer.from('Hello Node.js')); | ||
fs.writeFile('message.txt', data, { signal }, (err) => { | ||
// When a request is aborted - the callback is called with an AbortError | ||
}); | ||
// When the request should be aborted | ||
controller.abort(); | ||
``` | ||
|
||
Aborting an ongoing request does not abort individual operating | ||
system requests but rather the internal buffering `fs.writeFile` performs. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Users who are unfamiliar with how writeFile works under the covers may find this sentence confusing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you propose we deal with this? Add an extra note to writeFile explaining how it works and suggesting a createWriteStream for performacne sensitive stuff like we do with readFile? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to explain how this works underneath instead, and then include the above sentence. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
### Using `fs.writeFile()` with file descriptors | ||
|
||
When `file` is a file descriptor, the behavior is almost identical to directly | ||
|
@@ -5717,6 +5744,10 @@ The `atime` and `mtime` arguments follow these rules: | |
<!-- YAML | ||
added: v10.0.0 | ||
changes: | ||
- version: REPLACEME | ||
pr-url: https://github.com/nodejs/node/pull/35993 | ||
description: The options argument may include an AbortSignal to abort an | ||
ongoing writeFile request. | ||
- version: v14.12.0 | ||
pr-url: https://github.com/nodejs/node/pull/34993 | ||
description: The `data` parameter will stringify an object with an | ||
|
@@ -5733,6 +5764,7 @@ changes: | |
* `encoding` {string|null} **Default:** `'utf8'` | ||
* `mode` {integer} **Default:** `0o666` | ||
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`. | ||
* `signal` {AbortSignal} allows aborting an in-progress writeFile | ||
* Returns: {Promise} | ||
|
||
Asynchronously writes data to a file, replacing the file if it already exists. | ||
|
@@ -5746,7 +5778,34 @@ If `options` is a string, then it specifies the encoding. | |
Any specified `FileHandle` has to support writing. | ||
|
||
It is unsafe to use `fsPromises.writeFile()` multiple times on the same file | ||
without waiting for the `Promise` to be resolved (or rejected). | ||
without waiting for the `Promise` to be fulfilled (or rejected). | ||
|
||
Similarly to `fsPromises.readFile` - `fsPromises.writeFile` is a convenience | ||
method that performs multiple `write` calls internally to write the buffer | ||
passed to it. For performance sensitive code consider using | ||
[`fs.createWriteStream()`][]. | ||
|
||
It is possible to use an {AbortSignal} to cancel an `fsPromises.writeFile()`. | ||
Cancelation is "best effort", and some amount of data is likely still | ||
to be written. | ||
|
||
```js | ||
const controller = new AbortController(); | ||
const { signal } = controller; | ||
const data = new Uint8Array(Buffer.from('Hello Node.js')); | ||
(async () => { | ||
try { | ||
await fs.writeFile('message.txt', data, { signal }); | ||
} catch (err) { | ||
// When a request is aborted - err is an AbortError | ||
} | ||
})(); | ||
// When the request should be aborted | ||
controller.abort(); | ||
``` | ||
|
||
Aborting an ongoing request does not abort individual operating | ||
system requests but rather the internal buffering `fs.writeFile` performs. | ||
|
||
## FS constants | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a bit difficult, and is likely something that we should tackle later in a separate PR, but it would be helpful if this reported the number of bytes known to have been written at the point of failure/cancelation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I asked in the description it's actually pretty easy to do - but I remember there were really good arguments regarding whether or not we should do this (in the summit, by @addaleax IIRC) and I don't remember what the conclusion was.
Adding the number of bytes written is actually technically pretty easy here if we just want to approximate how much was written since the last
write
since we don't actually abort writes in progress only writeAllThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right – I think we basically agreed to see cancellation is a way of expressing disinterest in the result of an operation. Not reporting the number of written bytes goes in line with that. More importantly, if we add cancellation support to the actual fs operations underneath – as we should – then these numbers become inherently unreliable. I would not be in favour of adding this, if somebody wants it, then they should use
fs.write()
directly without cancelling.