Skip to content

Commit

Permalink
fs: watch signals for recursive incompatibility
Browse files Browse the repository at this point in the history
This pull request makes fs.watch throw exception,
whenever it is used in an incompatible platform.
For this change following changes were made to api:

1.a new error type has been introduced.
2.fs.watch has been changed accordingly.

Users who use recursive  on
non-windows and osx platforms,
will face a new exception.
For this reason, it's a breaking change.

Fixes: #29901

PR-URL: #29947
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
Eran Levin authored and Trott committed Jan 25, 2020
1 parent 4fefd18 commit 67e067e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 4 deletions.
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2412,6 +2412,12 @@ The `--entry-type=...` flag is not compatible with the Node.js REPL.
Used when an [ES Module][] loader hook specifies `format: 'dynamic'` but does
not provide a `dynamicInstantiate` hook.

<a id="ERR_FEATURE_UNAVAILABLE_ON_PLATFORM"></a>
#### `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM`

Used when a feature that is not available
to the current platform which is running Node.js is used.

<a id="ERR_STREAM_HAS_STRINGDECODER"></a>
#### `ERR_STREAM_HAS_STRINGDECODER`

Expand Down
2 changes: 2 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -3766,6 +3766,8 @@ The `fs.watch` API is not 100% consistent across platforms, and is
unavailable in some situations.

The recursive option is only supported on macOS and Windows.
An `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` exception will be thrown
when the option is used on a platform that does not support it.

#### Availability

Expand Down
7 changes: 5 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ const {
ERR_FS_FILE_TOO_LARGE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_CALLBACK
ERR_INVALID_CALLBACK,
ERR_FEATURE_UNAVAILABLE_ON_PLATFORM
},
uvException
} = require('internal/errors');
Expand Down Expand Up @@ -129,6 +130,7 @@ let FileReadStream;
let FileWriteStream;

const isWindows = process.platform === 'win32';
const isOSX = process.platform === 'darwin';


function showTruncateDeprecation() {
Expand Down Expand Up @@ -1359,7 +1361,8 @@ function watch(filename, options, listener) {

if (options.persistent === undefined) options.persistent = true;
if (options.recursive === undefined) options.recursive = false;

if (options.recursive && !(isOSX || isWindows))
throw new ERR_FEATURE_UNAVAILABLE_ON_PLATFORM('watch recursively');
if (!watchers)
watchers = require('internal/fs/watchers');
const watcher = new watchers.FSWatcher();
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,10 @@ E('ERR_FALSY_VALUE_REJECTION', function(reason) {
this.reason = reason;
return 'Promise was rejected with falsy value';
}, Error);
E('ERR_FEATURE_UNAVAILABLE_ON_PLATFORM',
'The feature %s is unavailable on the current platform' +
', which is being used to run Node.js',
TypeError);
E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than 2 GB', RangeError);
E('ERR_FS_INVALID_SYMLINK_TYPE',
'Symlink type must be one of "dir", "file", or "junction". Received "%s"',
Expand Down
7 changes: 5 additions & 2 deletions test/parallel/test-fs-watch-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

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

if (!(common.isOSX || common.isWindows))
common.skip('recursive option is darwin/windows specific');

const assert = require('assert');
const path = require('path');
Expand All @@ -20,6 +18,11 @@ const testsubdir = fs.mkdtempSync(testDir + path.sep);
const relativePathOne = path.join(path.basename(testsubdir), filenameOne);
const filepathOne = path.join(testsubdir, filenameOne);

if (!common.isOSX && !common.isWindows) {
assert.throws(() => { fs.watch(testDir, { recursive: true }); },
{ code: 'ERR_FEATURE_UNAVAILABLE_ON_PLATFORM' });
return;
}
const watcher = fs.watch(testDir, { recursive: true });

let watcherClosed = false;
Expand Down

0 comments on commit 67e067e

Please sign in to comment.