Skip to content
Snippets Groups Projects
Commit 66a4803f authored by Georges Racinet's avatar Georges Racinet
Browse files

Repository loading: returning proper error if not found

We already had good enough error responses for the case when
the *storage* is not found, but it was missing for the case
when the repository is not found in an existing storage.

This is important, because it can actually break the landing page
of some users: the Rails application does not seem to expect an
unspecified Gitaly error here, but it should recover from
NOT_FOUND.

I've been impaired by this on dev setup after
with some repository removals, but don't have anything of that
kind on hand any more, which would be more reassuring.

We could have a better separation in concerns, as the
subsequent change in `RepositoryExists()` indicates, but that's
not a blocker for the imminent release.
parent e4074022
No related branches found
No related tags found
1 merge request!11Repository loading: returning proper error if not found
Pipeline #12613 passed
......@@ -11,7 +11,6 @@
from mercurial import (
archival,
scmutil,
error,
)
# TODO move that to the heptapod package
......@@ -64,5 +63,5 @@
try:
self.load_repo(request.repository, context)
exists = True
except error.RepoError:
except KeyError:
exists = False
......@@ -68,4 +67,10 @@
exists = False
# TODO would be better to have a two-layer helper
# in servicer: load_repo() for proper gRPC error handling and
# load_repo_raw_exceptions() (name to be improved) to get the
# raw exceptions
context.set_code(StatusCode.OK)
context.set_details('')
return RepositoryExistsResponse(exists=exists)
......
......@@ -9,6 +9,7 @@
from grpc import StatusCode
from mercurial import (
error,
hg,
ui as uimod,
)
......@@ -69,4 +70,9 @@
logger.info("loading repo at %r", repo_path)
# ensure caller gets private copy of ui
return hg.repository(self.ui.copy(), repo_path)
try:
return hg.repository(self.ui.copy(), repo_path)
except error.RepoError as exc:
context.set_code(StatusCode.NOT_FOUND)
context.set_details(repr(exc.args))
raise KeyError('repo', repo_path)
......@@ -57,7 +57,7 @@
assert context.code == grpc.StatusCode.NOT_FOUND
def test_not_found_propagation(grpc_channel):
def test_not_found_propagation(grpc_channel, server_repos_root):
# Taking a random RPC to check that the client receives the
# proper error response
repo_stub = RepositoryServiceStub(grpc_channel)
......@@ -69,3 +69,12 @@
assert exc.code() == grpc.StatusCode.NOT_FOUND
assert 'dream' in exc.details()
with pytest.raises(grpc.RpcError) as exc_info:
repo_stub.HasLocalBranches(HasLocalBranchesRequest(
repository=Repository(storage_name='default',
relative_path='not_here')))
exc = exc_info.value
assert exc.code() == grpc.StatusCode.NOT_FOUND
assert 'not_here' in exc.details()
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment