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

Npm Registry Access Error (non-5xx) Raises Invalid Configuration Error #1341

Closed
ghost opened this issue Jan 3, 2018 · 10 comments
Closed

Npm Registry Access Error (non-5xx) Raises Invalid Configuration Error #1341

ghost opened this issue Jan 3, 2018 · 10 comments
Labels
manager:npm package.json files (npm/yarn/pnpm) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)

Comments

@ghost
Copy link

ghost commented Jan 3, 2018

I noticed that when Renovate was unable to successfully query an npm registry proxy for package metadata, it caused Renovate to assume that a given preset was invalid. That led to Renovate raising configuration error issues with multiple repositories.

Here's what I saw in the logs:

WARN: Unknown npm lookup error (repository=example-org/example-application)
       "err": {
         "name": "RequestError",
         "host": "artifactory.example.com",
         "hostname": "artifactory.example.com",
         "method": "GET",
         "path": "/artifactory/api/npm/npm/renovate-config-config",
         "protocol": "http:",
         "url": "http://artifactory.example.com/artifactory/api/npm/npm/renovate-config-config"
       }
 INFO: Throwing preset error (repository=example-org/example-application)
 INFO: Repository has invalid config (repository=example-org/example-application)
       "error": {"validationError": "Cannot find preset's package (config:js-app)"}
DEBUG: raiseConfigWarningIssue() (repository=example-org/example-application)

The configuration errors weren't really errors.

Could npm access issues simply cause renovate to skip over renovating that repo?

@rarkins
Copy link
Collaborator

rarkins commented Jan 3, 2018

What if they are truly errors though - i.e. configuration errors - and we just keep skipping the repository forever. Can we distinguish between real not found errors and temporary/server errors and take different action?

@ghost
Copy link
Author

ghost commented Jan 3, 2018

Is a RequestError a 400 error?

Does that have a different meaning than a 404?

A preset package with a slight typo in the name:

$ curl https://registry.npmjs.org/renovate-config-defaultt --verbose
...
< HTTP/2 404

@ghost
Copy link
Author

ghost commented Jan 4, 2018

I believe the reason I'm getting request errors is because of my earlier attempt to work around the issue in got - sindresorhus/got#427 (comment)

Setting the agent to an empty string throws the request error.

Setting an empty agent string using Curl seems to work correctly:

$ curl -A '' https://registry.npmjs.org/renovate-config-default --verbose

So it seems I'm not understanding what agent is doing in got.

@ghost
Copy link
Author

ghost commented Jan 5, 2018

So I think it could be useful to detect 5xx errors and skip those (so not throw configuration error issues).

After that, 4xx level errors can be handled on a per-case basis.

For example, 451 which is related to legal issues, while 429 would just be to many requests.

@rarkins
Copy link
Collaborator

rarkins commented Jan 5, 2018

So maybe any 5xx from the registry would have a registry-error return value, and we can also loop/retry like is done after automerging?

@ghost
Copy link
Author

ghost commented Jan 17, 2018

Looks like 5xx errors have been addressed as part of #1388.

@ghost ghost changed the title Npm Registry Access Error Raises Invalid Configuration Error Npm Registry Access Error (4xx) Raises Invalid Configuration Error Jan 17, 2018
@ghost ghost changed the title Npm Registry Access Error (4xx) Raises Invalid Configuration Error Npm Registry Access Error (non-5xx) Raises Invalid Configuration Error Jan 17, 2018
@rarkins rarkins added type:feature Feature (new functionality) in progress priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Jan 19, 2018
@rarkins
Copy link
Collaborator

rarkins commented Jan 19, 2018

@destroyerofbuilds sorry I forgot to update this issue as it progressed. You are right, I now attempt to abort Renovation when presented with 5xx errors from the npm registry.

As I have some doubts that we've ever seen 451 or 429 in practice, I'm keeping this at a pri3 priority.

@ghost
Copy link
Author

ghost commented Jan 19, 2018

Recently we encountered an issue where our npm registry proxy went down, preventing Renovate from fetching packages. That led to quite a few erroneously auto-closed PRs. We added additional logging to always print the full error object out so that we can get a better sense of what error is thrown in various failure scenarios.

@rarkins rarkins added ready manager:npm package.json files (npm/yarn/pnpm) and removed in progress labels Jan 20, 2018
@rarkins
Copy link
Collaborator

rarkins commented Jan 21, 2018

@destroyerofbuilds I just searched my logs for messages of Unknown npm lookup error and saw some unexpected results. Here's an example:

{
    "name": "@types/node",
    "level": 40,
    "repository": "ikatyang/tslint-plugin-ikatyang",
    "packageFile": "package.json",
    "depType": "devDependencies",
    "err": {
        "name": "ParseError",
        "host": "registry.npmjs.org",
        "hostname": "registry.npmjs.org",
        "method": "GET",
        "path": "/@types%2Fnode",
        "protocol": "https:",
        "url": "https://registry.npmjs.org/@types%2Fnode",
        "statusCode": 200,
        "statusMessage": "OK"
    },
    "msg": "Unknown npm lookup error",
    "time": "2018-01-14T22:11:55.106Z"
}

It's thrown if got can't parse the JSON in the response.

@rarkins rarkins removed the ready label Jan 21, 2018
@ghost
Copy link
Author

ghost commented Jan 22, 2018

We did a quick test, though low fidelity, in which we provided an invalid registry endpoint, and the error object returned by got did not contain a statusCode. Though I'm not sure whether that truly replicates the environment where our proxy went down, at least it demonstrates how low level network errors do not return back status codes.

It's likely d774a14 will cover our particular case.

Thank you @rarkins.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:npm package.json files (npm/yarn/pnpm) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)
Projects
None yet
Development

No branches or pull requests

1 participant