Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

config.js file watching broken on OSX #182

Closed
ciaranj opened this Issue · 5 comments

4 participants

@ciaranj

commit: 19814bd appears to break OSX . The API call appears incorrect. Could be down to the version of node.js I'm using, but fs.watch takes function( event, filename) [filename not present in OSX incidentally] not function( curr, prev ) as fs.watchFile does.

This means I see the following errors:

/Users/ciaran/Documents/Development/statsd/lib/config.js:25
    if (curr.ino != prev.ino) { self.updateConfig(); }
                        ^
TypeError: Cannot read property 'ino' of null
    at FSWatcher.Configurator (/Users/ciaran/Documents/Development/statsd/lib/config.js:25:25)
    at FSWatcher.EventEmitter.emit (events.js:96:17)
    at FSEvent._handle.onchange (fs.js:826:12)

Now, I would provide a patch to fix it, but since the commit that introduces the changes gives no hints as to what was being fixed, I can't make sure that it remains fixed.

If I had to guess I'd say this is broken for everyone, but on Windows & Linux the second argument (filename) does actually exist so prev.ino resolves to undefined, rather than trying to find .ino on undefined as in my case (OSX)

@jgoulah
Owner

please provide the version of node.js that is installed on your system

@ciaranj

v0.8.12, also I meant to include this link previously:

http://nodejs.org/docs/latest/api/fs.html#fs_fs_watch_filename_options_listener

(sorry)

@peschuster

It was changed in pull request #145, because fs.watchFile is not available on windows.
But you're right. The callback function needs to be changed accordingly.

I'd suggest:

fs.watch(file, function (event, filename) {
    if (event != 'rename') { self.updateConfig(); }
});
@ciaranj ciaranj referenced this issue from a commit in ciaranj/statsd
@ciaranj ciaranj Fixes Issues #182 - Broken config file watching.
Implements fix as suggested by Peter Schuster in comment thread.

Signed-off-by: ciaranj <ciaranj@gmail.com>
fd6db82
@ciaranj

I've submitted a pull request with @peschuster ' s suggested fix.

@mrtazz
Owner

closing this since we have pull request #184

@mrtazz mrtazz closed this
@ciaranj ciaranj referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@ciaranj ciaranj referenced this issue from a commit in ciaranj/statsd
@ciaranj ciaranj Fixes Issues #182 - Broken config file watching.
Implements fix as suggested by Peter Schuster and Daniel
Schauenberg in comment thread.

Signed-off-by: ciaranj <ciaranj@gmail.com>
ad011af
@hbouvier hbouvier referenced this issue from a commit in hbouvier/statsd
@ciaranj ciaranj Fixes Issues #182 - Broken config file watching.
Implements fix as suggested by Peter Schuster and Daniel
Schauenberg in comment thread.

Signed-off-by: ciaranj <ciaranj@gmail.com>
5fd2201
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.