search

concourse / semver-resource

automated semantic version bumping
Apache License 2.0
97 stars 105 forks source link

Why siliently discard git push failure? #124

Closed evanchaoli closed 1 year ago

evanchaoli commented 3 years ago

We are on Concourse 6.7.3, corresponding semver-resource version is 1.2.1.

One of our users reported an issue where git push failed with error [rejected], but the put step still passed.

I checked the code:

https://github.com/concourse/semver-resource/blob/c9c668a92ef426a68f0e7a8ad89e161c2e3c0f76/driver/git.go#L375-L385

Looks like it intentionally eat the error. So I want to ask why it was designed that way? Is it better if it tries to rebase and push again?

laurenty commented 3 years ago

I am wondering the same thing. The user I was using originally did not have write permissions to the repo and the put step was failing even though it appeared to be successful in Concourse:

ERROR: Permission to <org/repo.git> denied to <user>.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

image

laurenty commented 3 years ago

After taking a closer look at the code, it looks like in my case the error shouldn't match any "expected" errors in which case the function should return false along with the error. I went looking for the tests for the git driver, but couldn't find any. Any ideas?

jgcrespo-git commented 1 year ago

We get the same issue when multiple concourse pipelines are running at the same time, and as the git push silently fails after three attempts, it's very dangerous because no error is raised in the pipeline. Shouldn't RetriesOnErrorWriteVersion be a configurable parameter and an error be raised after the max number of attempts is reached?