I was investigating another issue and noticed the remote mirror update logic appears flawed. ~In the `UpdateRemoteMirrorService` we have a [comment that blocked URLs result in a hard fail](https://gitlab.com/gitlab-org/gitlab/-/blob/07224b53adf008d2f9a927d8b70ae952cc8b4e55/app/services/projects/update_remote_mirror_service.rb#L12) but the logic [tries to retry](https://gitlab.com/gitlab-org/gitlab/-/blob/07224b53adf008d2f9a927d8b70ae952cc8b4e55/app/services/projects/update_remote_mirror_service.rb#L21)~ ~I believe we should change this to simply~ ```ruby remote_mirror.hard_fail!(_('The remote mirror URL is invalid.')) ``` Edit: the previous section appears to be intentional from https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57392#note_540149493 so we should ideally update the [comment](https://gitlab.com/gitlab-org/gitlab/-/blob/07224b53adf008d2f9a927d8b70ae952cc8b4e55/app/services/projects/update_remote_mirror_service.rb#L12) to clarify. I also noticed that we're then returning `remote_mirror.last_error` which in this case should be the error we just specified `_('The remove mirror URL is invalid.')`. If that's the case it's probably easier to read and understand if we just return that error e.g. ```ruby message = _('The remote mirror URL is invalid.') remote_mirror.hard_fail!(message) return error(message) ``` I'm also wondering if we should also update this error to say "The remote mirror URL is blocked" so it's clear to the end user why this failed.