Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: make failed addon loading more informative #90

Closed
wants to merge 1 commit into from

Conversation

aqrln
Copy link
Member

@aqrln aqrln commented Feb 26, 2017

  • 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.

* 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.
Copy link
Member

@belochub belochub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

aqrln added a commit that referenced this pull request Feb 27, 2017
* 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
@aqrln
Copy link
Member Author

aqrln commented Feb 27, 2017

Landed in 2004786.

@aqrln aqrln closed this Feb 27, 2017
@aqrln aqrln deleted the verbose-safe-require branch February 27, 2017 14:42
aqrln added a commit that referenced this pull request Mar 13, 2017
* 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
@aqrln aqrln mentioned this pull request Mar 13, 2017
aqrln added a commit that referenced this pull request Mar 13, 2017
* 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
belochub pushed a commit that referenced this pull request Jan 22, 2018
* 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
belochub pushed a commit that referenced this pull request Jan 22, 2018
* 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
@belochub belochub mentioned this pull request Jan 22, 2018
belochub pushed a commit to metarhia/mdsf that referenced this pull request Jul 19, 2018
* 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: metarhia/jstp#90
belochub pushed a commit to metarhia/mdsf that referenced this pull request Jul 19, 2018
* 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: metarhia/jstp#90
belochub pushed a commit to metarhia/mdsf that referenced this pull request Jul 21, 2018
* 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: metarhia/jstp#90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants