Skip to content

Commit

Permalink
fs: fix error on bad listener type
Browse files Browse the repository at this point in the history
When the listener was truthy but NOT a function, fs.watchFile would
throw an error through the EventEmitter. This caused a problem because
it would only be thrown after the listener was started, which left the
listener on.

There should be no backwards compatability issues because the error was
always thrown, just in a different manner.

Also adds tests for this and other basic functionality.

PR-URL: #2093
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
brendanashworth committed Jul 10, 2015
1 parent 12bc397 commit 1afc0c9
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 8 deletions.
15 changes: 7 additions & 8 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1305,28 +1305,27 @@ StatWatcher.prototype.stop = function() {

const statWatchers = new Map();

fs.watchFile = function(filename) {
fs.watchFile = function(filename, options, listener) {
nullCheck(filename);
filename = pathModule.resolve(filename);
var stat;
var listener;

var options = {
var defaults = {
// Poll interval in milliseconds. 5007 is what libev used to use. It's
// a little on the slow side but let's stick with it for now to keep
// behavioral changes to a minimum.
interval: 5007,
persistent: true
};

if (arguments[1] !== null && typeof arguments[1] === 'object') {
options = util._extend(options, arguments[1]);
listener = arguments[2];
if (options !== null && typeof options === 'object') {
options = util._extend(defaults, options);
} else {
listener = arguments[1];
listener = options;
options = defaults;
}

if (!listener) {
if (typeof listener !== 'function') {
throw new Error('watchFile requires a listener function');
}

Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-fs-watchfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict';

const fs = require('fs');
const assert = require('assert');

// Basic usage tests.
assert.throws(function() {
fs.watchFile('./some-file');
}, /watchFile requires a listener function/);

assert.throws(function() {
fs.watchFile('./another-file', {}, 'bad listener');
}, /watchFile requires a listener function/);

assert.throws(function() {
fs.watchFile(new Object(), function() {});
}, /Path must be a string/);

0 comments on commit 1afc0c9

Please sign in to comment.