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

Consider retries in SymbolsValidationRule due to intermittent HTTP errors #539

Closed
atifaziz opened this issue Jun 27, 2023 · 2 comments · Fixed by #541
Closed

Consider retries in SymbolsValidationRule due to intermittent HTTP errors #539

atifaziz opened this issue Jun 27, 2023 · 2 comments · Fixed by #541

Comments

@atifaziz
Copy link
Contributor

atifaziz commented Jun 27, 2023

I'm using your excellent package validation tool (thanks!) in the CI builds for MoreLINQ. Recently, I've noticed the CI builds failing during package validation with the error code 119:

An example of the output from the tool is:

{
  "Packages": {
    "/home/appveyor/projects/morelinq/dist/morelinq.4.0.0-ci-20230627T0635.nupkg": {
      "Errors": [
        {
          "ErrorCode": 119,
          "Message": "Source file \u0027https://raw.githubusercontent.com/morelinq/MoreLINQ/a9437d48e3d30cf05035853d2e22226f630a2468/MoreLinq/ZipImpl.cs\u0027 is not accessible",
          "FileName": "lib/net6.0/MoreLinq.dll"
        }
      ]
    }
  }
}

The error seems to be intermittent and fails on different URLs and images of the build each time. When I try to access the identified URL, it works. When I re-run the CI build, it usually passes. Would you consider adding a retry? If so, I'd be glad to send a PR with an implementation.

@meziantou
Copy link
Owner

meziantou commented Jun 27, 2023

There is already a retry policy (SharedHttpClient), but it may not be sufficient as GitHub can limit the concurrency. I think it could be improved by handling correctly the status code 429 (I would accept a PR).

For GitHub, I think the best fix is to allow specifying an authentication token. GitHub is less restrictive when a request is authenticated.

In the case of GitHub Actions, you can use the job token:

meziantou.validate-nuget-package sample.nupkg --github-token=${{ secrets.GITHUB_TOKEN }}

The new package will be published by https://github.com/meziantou/Meziantou.Framework/actions/runs/5392720755

Note: You can also exclude the rule 119 if it's too flaky using --excluded-rule-ids 119

@atifaziz
Copy link
Contributor Author

atifaziz commented Jun 27, 2023

There is already a retry policy (SharedHttpClient), …

Ah, cool. I dug into the rule code, but not the SharedHttpClient implementation. Perhaps it's just to aggressive (if I am reading it right, just milliseconds apart with a max of 0.5 seconds) in terms of its back-off strategy by default?

but it may not be sufficient as GitHub can limit the concurrency.

Might be (almost always) better as a default to use something more exponential?

I think it could be improved by handling correctly the status code 429 (I would accept a PR).

Since the status code is never revealed in the error, I'm not sure it's a 429 although I'm guessing it's probably the case.

For GitHub, I think the best fix is to allow specifying an authentication token. GitHub is less restrictive when a request is authenticated.

Thanks for the tip and cheers for the quick turnaround with PR #540!

Note: You can also exclude the rule 119 if it's too flaky using --excluded-rule-ids 119

Right, though I'd rather not as it could be, while unlikely, a true negative.

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 a pull request may close this issue.

2 participants