Skip to content

Commit

Permalink
lib: make failed addon loading more informative
Browse files Browse the repository at this point in the history
* Make `common.safeRequire` print the reason of failed require() to
  stderr. The error wasn't passed anywhere so the reason was lost
  and could not be delivered to user. When the Record Serialization
  parser could not load its native addon, it always stated that it
  isn't built though the actual reason might be different.

* Fix somewhat misleading language in the error message in
  `lib/record-serialization.js`. The addon is not only used on server.

For example, when the addon was compiled with a wrong version of
`node-gyp`, JSTP used to print:

> JSTP native addon is not built. Run `npm install` in order to build
> it, otherwise you will get poor server performance under load.

and it continued to print the same message however many times `npm
install` was invoked, which is certainly not very helpful.

Now the error message in the same situation looks like this:

> Error: The module
> '/Users/alex/projects/metarhia/JSTP/build/Release/jstp.node'
> was compiled against a different Node.js version using
> NODE_MODULE_VERSION 51. This version of Node.js requires
> NODE_MODULE_VERSION 53. Please try re-compiling or re-installing
> the module (for instance, using `npm rebuild` or `npm install`).
> Error: Cannot find module '../build/Debug/jstp'
> JSTP native addon is not built or is not functional. Run `npm install`
> in order to build it, otherwise you will get poor performance.

and is certainly more helpful since it contains an obvious reference to
ABI incompatibility and makes it possible to troubleshoot it.

PR-URL: #90
  • Loading branch information
aqrln committed Mar 13, 2017
1 parent de5ffe5 commit 943bd79
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
1 change: 1 addition & 0 deletions lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ common.safeRequire = (moduleName) => {
try {
return require(moduleName);
} catch (err) {
console.warn(err.toString());
return null;
}
};
Expand Down
4 changes: 2 additions & 2 deletions lib/record-serialization.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ if (jstpNative) {
}
} else {
console.warn(
'JSTP native addon is not built. ' +
'JSTP native addon is not built or is not functional. ' +
'Run `npm install` in order to build it, otherwise you will get ' +
'poor server performance under load.'
'poor performance.'
);
module.exports = require('./record-serialization-fallback');
}

0 comments on commit 943bd79

Please sign in to comment.