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

Add more helpful error message for failure to load extension #106

Merged
merged 2 commits into from
Nov 30, 2017

Conversation

murgatroid99
Copy link
Member

We've gotten multiple reports where the extension failed to load, and this will help us diagnose the problem in more cases.

var foundNames = fs.readdirSync(searchPath);
if (foundNames.indexOf(searchName) === -1) {
var message = `Failed to load gRPC binary module because it was not installed for the current system
Expected: ${searchName}
Copy link
Member

Choose a reason for hiding this comment

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

Didn't you mean binding_path here instead of searchName ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. searchName is the name of the directory that contains grpc_node.node. And that directory name contains the relevant information for diagnosing this problem. Plus, it can be directly (visually) compared with the contents of foundNames.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, then maybe rephrase it a tiny bit, because the words "gRPC binary module" to me means grpc_node.node. Maybe you need to add the word "location" next to "Expected" ?

@murgatroid99 murgatroid99 merged commit 89c60cb into grpc:master Nov 30, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants