Return a 404 not found rather than a 500 when a repo can't be fetched
Created by: mollylogue
…d in GetByName
Surfaced by the change in https://github.com/sourcegraph/sourcegraph/pull/31722/files#diff-83915dc8969b3cb3d5c201581bf569b7af057b6ae113bcbb4bcc574cd3ed0ecfR73
Case: a repo doesn't exist (i.e. has been deleted, or made private)
when GetByName
is called, the current behavior is to check IsRepoCloneable which first tries to fetch the repo as is (which results in a Not Found error) and then tries to append .git
to the end and fetch again (which in the case of a deleted gitlab project, results in an Access Denied error). This bubbles up to the frontend as a 500 internal server error. The new behavior in this PR is to return a 404 not found error in the case that the repo isn't found in IsRepoCloneable
. There aren't really tests around this currently to be able to tell if this is a safe change to make. (i.e., is it possible that the repo fetch yields a 404 but appending .git
to the repo name and trying again is successful)? Would like some input on that front.
Test plan
Unit tests, and manually ensuring that we can navigate to deleted/private repos in the cloud ui and get a 404 rather than a 500.