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

prevent trying to get a commit that can't exist #5981

Merged
merged 2 commits into from
Oct 27, 2022

Conversation

jakecoffman
Copy link
Member

@jakecoffman jakecoffman commented Oct 26, 2022

The change in #5937 caused Dependabot to begin raising an exception inside of the rescue for repositories that had a branch that didn't exist inside of their dependabot.yml.

What is happening is:

  • The clone raises a HelperSubprocessFailed
  • It gets turned into RepoNotFound
  • The file_fetcher_job's rescue tries to call mark_job_as_processed
  • But first it tries to get the base_commit_sha which... doesn't exist! So it also raises.

This fixes two issues:

  • By setting @base_commit_sha to "unknown" when we know the clone failed, we can avoid the second raise.
  • By checking the error message for "remote brach X not found" we can raise the correct error so the UI is more helpful

@jakecoffman jakecoffman requested a review from a team as a code owner October 26, 2022 20:51
Copy link
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me

Definitely an improvement over the error cascades we had earlier - reminds me of throw null in Java

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@jakecoffman jakecoffman merged commit 6a9c745 into main Oct 27, 2022
@jakecoffman jakecoffman deleted the jakecoffman/fix-branch-not-found-user-error branch October 27, 2022 13:45
@pavera pavera mentioned this pull request Oct 31, 2022
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.

None yet

3 participants