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: Improve error message for scanning an image that doesn't exist #1508

Conversation

ahmed-agabani-snyk
Copy link
Contributor

@ahmed-agabani-snyk ahmed-agabani-snyk commented Nov 4, 2020

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

What does this PR do?

Converts an internal error into an actionable human readable message.

How should this be manually tested?

  • snyk container monitor doesnotexist
  • snyk container test doesnotexist

What are the relevant tickets?

Screenshots

Screenshot 2020-11-04 at 16 00 44

@ahmed-agabani-snyk ahmed-agabani-snyk force-pushed the fix/improve-error-message-for-scanning-an-image-that-doesnt-exist branch 3 times, most recently from c036c58 to f0b2e3f Compare November 4, 2020 15:57
@ahmed-agabani-snyk ahmed-agabani-snyk marked this pull request as ready for review November 4, 2020 16:39
@ghost ghost requested review from anthogez and MegaBean November 4, 2020 16:39
src/lib/errors/docker-image-not-found-error.ts Outdated Show resolved Hide resolved
@ahmed-agabani-snyk ahmed-agabani-snyk force-pushed the fix/improve-error-message-for-scanning-an-image-that-doesnt-exist branch from f0b2e3f to a5ccb25 Compare November 5, 2020 13:16
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2020

Expected release notes (by @ahmed-agabani-snyk)

fixes:
Improve error message for scanning an image that doesn't exist (a5ccb25)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

private static ERROR_CODE = 404;

constructor(image: string) {
const message = `Failed to scan image "${image}". Please make sure the image and/or repository exist.`;

Choose a reason for hiding this comment

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

I think this can also be a login (to docker hub) issue, and not necessarily the image not existing. maybe we need to rephrase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tl;dr:
Product decided on Failed to scan image "<image-name>". Please make sure the image and/or repository exist.

Product's rationale:

The thinking was not to overload information (so removed the auth part) + make it actionable enough (so they'll check whether it exists)

Personal thoughts:
As an engineer, the first thing I would do if I see this message is to run docker pull <image-name> to check if it exists. Docker will provide a more detailed error.

I do not think it should be the Snyk CLI's responsibility to accurately diagnose a problem with docker.

History:

  • current message snyk cli provides: authentication required
  • current message docker cli provides: Error response from daemon: pull access denied for doesnotexist, repository does not exist or may require 'docker login': denied: requested access to the resource is denied
  • message I used when developing this fix:
    Pull access denied for 'doesnotexist', repository does not exist or may require 'docker login'

@ahmed-agabani-snyk ahmed-agabani-snyk merged commit 90b61dd into master Nov 5, 2020
@ahmed-agabani-snyk ahmed-agabani-snyk deleted the fix/improve-error-message-for-scanning-an-image-that-doesnt-exist branch November 5, 2020 15:05
@snyksec
Copy link

snyksec commented Nov 5, 2020

🎉 This PR is included in version 1.424.3 🎉

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
5 participants