iloire / watchmen

A simple node.js service monitor
MIT License
941 stars 195 forks source link

Use `path.join` for constructing path to `package.json` #55

Closed kimar closed 8 years ago

kimar commented 8 years ago

Using Node.js v5.7 I'm receiving the following error when trying to run the app:

Using storage env:  development
module.js:341
    throw err;
    ^

Error: Cannot find module '/private/tmp/watchmen/webserver/routes../../../package.json'
    at Function.Module._resolveFilename (module.js:339:15)
    at Function.Module._load (module.js:290:25)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at Object.module.exports.getRoutes (/private/tmp/watchmen/webserver/routes/api-ping-plugins-route.js:8:17)
    at module.exports (/private/tmp/watchmen/webserver/app.js:42:35)
    at Object.<anonymous> (/private/tmp/watchmen/run-web-server.js:23:11)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)

It appears as if joining the path using + has a different outcome, I'd say it's better in any case to join it using the path-module.

iloire commented 8 years ago

Hi Marcus, thanks for the PR!

It seems like the require internal implementation changed in node 5 and it is not accepting anymore the weird relative path that it was provided as a result of doing string concatenation between __dirnameand a relative path (that was pretty ugly actually)

Using path.join is indeed a much better option but I think path.resolve would be better suited for this scenario. What do you think?

iloire commented 8 years ago

I changed it here: https://github.com/iloire/watchmen/commit/45108473494712c6a0de2a6f605a70b75f38b622 thanks again!

kimar commented 8 years ago

:+1: path.resolve indeed sounds like what we want to use for this.