fs: remove watcher state errors for fs.watch

- Remove ERR_FS_WATCHER_ALREADY_STARTED and
  ERR_FS_WATCHER_NOT_STARTED because those two situations should
  result in noop instead of errors for consistency with the
  documented behavior of fs.watchFile.
  This partially reverts #19089
- Update comments about this behavior.

Refs: #19089

PR-URL: #19345
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

doc/api/errors.md

@@ -783,18 +783,6 @@ falsy value.
 An invalid symlink type was passed to the [`fs.symlink()`][] or
 [`fs.symlinkSync()`][] methods.
 
-<a id="ERR_FS_WATCHER_ALREADY_STARTED"></a>
-### ERR_FS_WATCHER_ALREADY_STARTED
-
-An attempt was made to start a watcher returned by `fs.watch()` that has
-already been started.
-
-<a id="ERR_FS_WATCHER_NOT_STARTED"></a>
-### ERR_FS_WATCHER_NOT_STARTED
-
-An attempt was made to initiate operations on a watcher returned by
-`fs.watch()` that has not yet been started.
-
 <a id="ERR_HTTP_HEADERS_SENT"></a>
 ### ERR_HTTP_HEADERS_SENT
 

lib/fs.js

@@ -36,8 +36,6 @@ const fs = exports;
 const { Buffer } = require('buffer');
 const errors = require('internal/errors');
 const {
-  ERR_FS_WATCHER_ALREADY_STARTED,
-  ERR_FS_WATCHER_NOT_STARTED,
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_CALLBACK,
   ERR_OUT_OF_RANGE
@@ -1342,25 +1340,26 @@ util.inherits(FSWatcher, EventEmitter);
 
 // FIXME(joyeecheung): this method is not documented.
 // At the moment if filename is undefined, we
-// 1. Throw an Error from C++ land if it's the first time .start() is called
-// 2. Return silently from C++ land if .start() has already been called
+// 1. Throw an Error if it's the first time .start() is called
+// 2. Return silently if .start() has already been called
 //    on a valid filename and the wrap has been initialized
+// This method is a noop if the watcher has already been started.
 FSWatcher.prototype.start = function(filename,
                                      persistent,
                                      recursive,
                                      encoding) {
   lazyAssert()(this._handle instanceof FSEvent, 'handle must be a FSEvent');
   if (this._handle.initialized) {
-    throw new ERR_FS_WATCHER_ALREADY_STARTED();
+    return;
   }
 
   filename = getPathFromURL(filename);
   validatePath(filename, 'filename');
 
-  var err = this._handle.start(pathModule.toNamespacedPath(filename),
-                               persistent,
-                               recursive,
-                               encoding);
+  const err = this._handle.start(pathModule.toNamespacedPath(filename),
+                                 persistent,
+                                 recursive,
+                                 encoding);
   if (err) {
     const error = errors.uvException({
       errno: err,
@@ -1372,10 +1371,11 @@ FSWatcher.prototype.start = function(filename,
   }
 };
 
+// This method is a noop if the watcher has not been started.
 FSWatcher.prototype.close = function() {
   lazyAssert()(this._handle instanceof FSEvent, 'handle must be a FSEvent');
   if (!this._handle.initialized) {
-    throw new ERR_FS_WATCHER_NOT_STARTED();
+    return;
   }
   this._handle.close();
 };

lib/internal/errors.js

@@ -654,10 +654,6 @@ E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value', Error);
 E('ERR_FS_INVALID_SYMLINK_TYPE',
   'Symlink type must be one of "dir", "file", or "junction". Received "%s"',
   Error); // Switch to TypeError. The current implementation does not seem right
-E('ERR_FS_WATCHER_ALREADY_STARTED',
-  'The watcher has already been started', Error);
-E('ERR_FS_WATCHER_NOT_STARTED',
-  'The watcher has not been started', Error);
 E('ERR_HTTP2_ALTSVC_INVALID_ORIGIN',
   'HTTP/2 ALTSVC frames require a valid origin', TypeError);
 E('ERR_HTTP2_ALTSVC_LENGTH',

test/parallel/test-fs-watch.js

@@ -65,16 +65,10 @@ for (const testCase of cases) {
       assert.strictEqual(eventType, 'change');
     assert.strictEqual(argFilename, testCase.fileName);
 
-    common.expectsError(() => watcher.start(), {
-      code: 'ERR_FS_WATCHER_ALREADY_STARTED',
-      message: 'The watcher has already been started'
-    });
+    watcher.start();  // starting a started watcher should be a noop
     // end of test case
     watcher.close();
-    common.expectsError(() => watcher.close(), {
-      code: 'ERR_FS_WATCHER_NOT_STARTED',
-      message: 'The watcher has not been started'
-    });
+    watcher.close(); // closing a closed watcher should be a noop
   }));
 
   // long content so it's actually flushed. toUpperCase so there's real change.