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

retry is not kicking in with npm 6.x #128

Closed
bartvde opened this issue Feb 10, 2020 · 4 comments · Fixed by #129
Closed

retry is not kicking in with npm 6.x #128

bartvde opened this issue Feb 10, 2020 · 4 comments · Fixed by #129
Assignees
Labels
bug Something isn't working

Comments

@bartvde
Copy link

bartvde commented Feb 10, 2020

Looks like the error message is not matched, is this a known issue?

bartvde@bartvde-XPS-13-9360:~/planet/git/audit-ci$ git diff
diff --git a/lib/audit.js b/lib/audit.js
index 501ea59..62c042e 100644
--- a/lib/audit.js
+++ b/lib/audit.js
@@ -7,9 +7,9 @@ function audit(pm, config, reporter) {
   const RETRY_ERROR_MSG = {
     npm: `${
       config.registry
-        ? `npm ERR! audit Your configured registry (${config.registry}) `
+        ? `code ENOAUDIT: Your configured registry (${config.registry}) `
         : ''
-    }does not support audit requests`,
+    }may not support audit requests, or the audit endpoint may be temporarily unavailable.`,
     yarn: '503 Service Unavailable',
   };
@bartvde
Copy link
Author

bartvde commented Feb 12, 2020

@djomaa since you did the original PR at #82, do you have any idea why the error is not getting matched with npm 6.13.7?

@quinnturner
Copy link
Member

quinnturner commented Feb 14, 2020

A bit of digging confirms that both are error messages:

This is certainly an issue. I am not a fan of processing the error message since it's very brittle, but a quick fix would be an if+elseif on the msg. Preferably, we could process the statusCode.

From the NPM audit src:

  if (err.statusCode >= 400) {
    let msg
    if (err.statusCode === 401) {
      msg = `Either your login credentials are invalid or your registry (${opts.registry}) does not support audit.`
    } else if (err.statusCode === 404) {
      msg = `Your configured registry (${opts.registry}) does not support audit requests.`
    } else {
      msg = `Your configured registry (${opts.registry}) may not support audit requests, or the audit endpoint may be temporarily unavailable.`
    }
    if (err.body.length) {
      msg += '\nThe server said: ' + err.body
    }
    const ne = new Error(msg)
    ne.code = 'ENOAUDIT'
    ne.wrapped = err
    throw ne
  }
  throw err

Note that his has been an issue for npm >= v6.10.2: nodejs/node@d7d321b

EDIT: Didn't mean to close!

@quinnturner quinnturner reopened this Feb 14, 2020
@quinnturner quinnturner added the bug Something isn't working label Feb 14, 2020
@quinnturner quinnturner self-assigned this Feb 14, 2020
@quinnturner
Copy link
Member

A quicker fix is simply to shorten the partial error message matching to not support audit. This way, all error messages are captures. Eventually, this should be cleaned up into proper error code handling, but this is an improvement over the current solution.

@bartvde
Copy link
Author

bartvde commented Feb 17, 2020

thanks @quinnturner !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants