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

Add retry-all-errors curl parameter #95

Merged
merged 1 commit into from
May 25, 2023
Merged

Add retry-all-errors curl parameter #95

merged 1 commit into from
May 25, 2023

Conversation

pera
Copy link
Contributor

@pera pera commented May 12, 2023

Lately we have seen Exited with code exit status 56 quite often during the download Snyk CLI step. The error is transient and usually goes away with one manual rerun.

This PR adds an extra option to make curl retry even on network layer errors.

Related previous PR: #63

@pera pera requested a review from a team as a code owner May 12, 2023 14:05
@CLAassistant
Copy link

CLAassistant commented May 12, 2023

CLA assistant check
All committers have signed the CLA.

@j-luong
Copy link
Contributor

j-luong commented May 17, 2023

Hi,

Transient errors should be handled by the --retry flag, unfortunately adding --retry-all-errors would be too broad, and is not recommended.

Error 56 would suggest a network error, which could be for many different reasons, it's really not possible to say without more information. I would suggest doing some investigation on your network to identify the cause, things like proxies, antivirus, firewalls, etc could all be causes.

@j-luong j-luong closed this May 17, 2023
@pera
Copy link
Contributor Author

pera commented May 17, 2023

@j-luong the warning in the documentation is not relevant in this case as in the script curl is only used to download Snyk CLI and the checksum. Also the error happens between CircleCI and static.snyk.io, not our network.

Could you please reconsider merging this PR? This issue is unfortunately quite common.

@bastiandoetsch
Copy link
Contributor

We just discussed internally, and decided that in this case, we're ok with going against the advice.

@bastiandoetsch
Copy link
Contributor

But before it can be merged, the commit must be signed. So please sign the commit and force-push to this branch again. Also, please change the commit message to "chore: add retry-all-errors curl parameter" for semantic release to work properly.

@pera
Copy link
Contributor Author

pera commented May 25, 2023

done, thanks @bastiandoetsch

@bastiandoetsch bastiandoetsch merged commit 4a6d54a into snyk:develop May 25, 2023
bastiandoetsch added a commit that referenced this pull request May 25, 2023
Co-authored-by: Brian Gomes Bascoy <338659+pera@users.noreply.github.com>
@bastiandoetsch
Copy link
Contributor

hey @pera , should be available in snyk-orb@1.5.1

@pera
Copy link
Contributor Author

pera commented May 25, 2023

@bastiandoetsch sorry but this PR should be reverted ASAP:

Downloading Snyk CLI version 1.1166.0
curl: option --retry-all-errors: is unknown
curl: try 'curl --help' or 'curl --manual' for more information

This option was added in curl-7.71.0 but this orb is using Ubuntu 20.04 (cimg/base:stable-20.04) that comes with curl-7.68.0

bastiandoetsch added a commit that referenced this pull request May 25, 2023
bastiandoetsch added a commit that referenced this pull request May 25, 2023
fix: revert retry-all-errors curl parameter (#95)
@bastiandoetsch
Copy link
Contributor

Yeah, the orb is reverted for now. We'll see if we can update the base image without consequences - I doubt it's gonna be a problem to update to 22.04.

@pera
Copy link
Contributor Author

pera commented May 26, 2023

Thanks, this problem is affecting a few teams here. I guess another option (but probably harder) would be to investigate what is causing intermittent connectivity issues between CircleCI and static.snyk.io.

@bastiandoetsch
Copy link
Contributor

Please submit that request to CircleCI, as apparently the connection failures occurs even before the request reaches Snyk. We'll be happy to support CircleCI to find a solution with our CDN provider (currently Akamai).

bastiandoetsch added a commit that referenced this pull request May 26, 2023
@bastiandoetsch
Copy link
Contributor

Version 1.6.0 of the orb will update the base image and include your fix again. If you want to, you can already test it (#99) with the snyk orb using version dev:develop

@pera
Copy link
Contributor Author

pera commented May 29, 2023

Thank you @bastiandoetsch, we just tried the dev version and it looks it's working fine now :)

@bastiandoetsch
Copy link
Contributor

That's great, @pera , glad to hear it!

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 this pull request may close these issues.

4 participants