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

fix: expose call graph creation errors #73

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

muscar
Copy link
Contributor

@muscar muscar commented Jul 17, 2020

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

This does two things (I know, I know):

  • it accepts a timeout from the client, and passes it to the call graph generator; and
  • it catches call graph generator errors, and reports them to the client, but continues the dep work.

This is meant to improve the experience around reachable vulns by:

  • giving control to the CLI on how long the call graph generator can run;
  • running the user command (e.g. monitor, test) even if the call graph generation failed;
  • allowing for better error messages is the call graph generation fails.

This should only be merged after snyk/java-call-graph-builder#21 and snyk/snyk-cli-interface#36, and after updating the dependencies to the appropriate versions.

What are the relevant tickets?

https://snyksec.atlassian.net/browse/FLOW-336

@CLAassistant
Copy link

CLAassistant commented Jul 17, 2020

CLA assistant check
All committers have signed the CLA.

lib/index.ts Outdated Show resolved Hide resolved
@muscar muscar force-pushed the fix/flow-336-expose-call-graph-creation-errors branch from 20895de to 2229f08 Compare July 20, 2020 07:54
@muscar muscar force-pushed the fix/flow-336-expose-call-graph-creation-errors branch 2 times, most recently from a982e8e to a0d5d8b Compare July 20, 2020 08:49
@muscar muscar marked this pull request as ready for review July 20, 2020 08:49
@muscar muscar requested a review from a team as a code owner July 20, 2020 08:49
@ghost ghost requested review from aviadhahami and gitphill July 20, 2020 08:49
@muscar muscar force-pushed the fix/flow-336-expose-call-graph-creation-errors branch 4 times, most recently from f9bcd08 to f13d24d Compare July 20, 2020 10:03
lib/index.ts Outdated Show resolved Hide resolved
dkontorovskyy
dkontorovskyy previously approved these changes Jul 20, 2020
Copy link

@dkontorovskyy dkontorovskyy left a comment

Choose a reason for hiding this comment

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

LGTM

@muscar muscar requested a review from darmalovan July 20, 2020 11:12
@muscar muscar force-pushed the fix/flow-336-expose-call-graph-creation-errors branch from f13d24d to d143bdb Compare July 20, 2020 11:24
darmalovan
darmalovan previously approved these changes Jul 20, 2020
@muscar muscar force-pushed the fix/flow-336-expose-call-graph-creation-errors branch from d143bdb to 680baa7 Compare July 20, 2020 11:46
@muscar muscar force-pushed the fix/flow-336-expose-call-graph-creation-errors branch from 680baa7 to 652d236 Compare July 20, 2020 11:51
Copy link

@dkontorovskyy dkontorovskyy left a comment

Choose a reason for hiding this comment

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

LGTM

@muscar muscar merged commit b55aa33 into master Jul 20, 2020
@muscar muscar deleted the fix/flow-336-expose-call-graph-creation-errors branch July 20, 2020 13:27
@snyksec
Copy link

snyksec commented Jul 20, 2020

🎉 This PR is included in version 2.17.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants