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

snyk: append the error text to thrown errors #13

Merged
merged 1 commit into from
Feb 26, 2018

Conversation

michael-go
Copy link
Contributor

@michael-go michael-go commented Feb 26, 2018

instead of current 'An unknown error occurred.' when e.g. sbt is not in the PATH

@michael-go michael-go self-assigned this Feb 26, 2018
@CLAassistant
Copy link

CLAassistant commented Feb 26, 2018

CLA assistant check
All committers have signed the CLA.

lib/index.js Outdated
@@ -37,7 +37,7 @@ function inspect(root, targetFile, options) {
if (thrownError) {
throw thrownError;
}
throw new Error('An unknown error occurred.');
throw new Error('snyk-sbt-plugin error: ' + error);
Copy link
Contributor

@adrukh adrukh Feb 26, 2018

Choose a reason for hiding this comment

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

Converting an error to a string will only keep its message. How about just throw error; to keep the stack-trace etc.?

Also, how will the CLI behave on a standard error being thrown by the plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CLI will just print the message and quit (without the stacktrace)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

b.t.w the existing code assumes the error is a string ..

@michael-go michael-go force-pushed the fix/throw-more-informative-erros branch from efb057c to 6317c93 Compare February 26, 2018 08:37
instead of current 'An unknown error occurred.' when e.g. `sbt` in not
in the PATH
@michael-go michael-go force-pushed the fix/throw-more-informative-erros branch from 6317c93 to ebf08c3 Compare February 26, 2018 08:40
if (thrownError) {
throw thrownError;
}
throw new Error('snyk-sbt-plugin error: ' + error);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd do throw (thrownError ? thrownError : new Error('snyk-sbt-plugin error: ' + error));, but I'm weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈 it becomes longer than 80 chars so I'll stick with my verbose version 😅

@michael-go michael-go merged commit 8052079 into master Feb 26, 2018
@michael-go michael-go deleted the fix/throw-more-informative-erros branch February 26, 2018 08:45
@michael-go michael-go restored the fix/throw-more-informative-erros branch February 26, 2018 09:04
@michael-go michael-go deleted the fix/throw-more-informative-erros branch February 26, 2018 09:28
miiila pushed a commit to snyk/cli that referenced this pull request Mar 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants