# HG changeset patch
# User Marcin Kuzminski <marcin@rhodecode.com>
# Date 2020-03-25 14:23:14
# Node ID d6e71ccf44662809c22108b7bc13c5f112df9a53
# Parent  63f4c33e85c36924e206c2c7f54f29bcdafd6aa9

exceptions: don't report lookup errors as exceptions stored in the exception store.

- those are regular not found problems that don't indicate any exceptional case
- also make the errors report nicer, not as KeyError, or generic Exception

diff --git a/vcsserver/exceptions.py b/vcsserver/exceptions.py
--- a/vcsserver/exceptions.py
+++ b/vcsserver/exceptions.py
@@ -115,3 +115,7 @@ class HTTPRepoLocked(HTTPLocked):
 class HTTPRepoBranchProtected(HTTPForbidden):
     def __init__(self, *args, **kwargs):
         super(HTTPForbidden, self).__init__(*args, **kwargs)
+
+
+class RefNotFoundException(KeyError):
+    pass
diff --git a/vcsserver/git.py b/vcsserver/git.py
--- a/vcsserver/git.py
+++ b/vcsserver/git.py
@@ -697,7 +697,12 @@ class GitRemote(RemoteBase):
                 missing_commit_err = 'Commit {} does not exist for `{}`'.format(sha, wire['path'])
                 try:
                     commit = repo.revparse_single(sha)
-                except (KeyError, ValueError) as e:
+                except KeyError:
+                    # NOTE(marcink): KeyError doesn't give us any meaningful information
+                    # here, we instead give something more explicit
+                    e = exceptions.RefNotFoundException('SHA: %s not found', sha)
+                    raise exceptions.LookupException(e)(missing_commit_err)
+                except ValueError as e:
                     raise exceptions.LookupException(e)(missing_commit_err)
 
                 is_tag = False
@@ -719,7 +724,10 @@ class GitRemote(RemoteBase):
                         if branch:
                             break
                     else:
-                        raise exceptions.LookupException(None)(missing_commit_err)
+                        # NOTE(marcink): Empty error doesn't give us any meaningful information
+                        # here, we instead give something more explicit
+                        e = exceptions.RefNotFoundException('SHA: %s not found in branches', sha)
+                        raise exceptions.LookupException(e)(missing_commit_err)
 
                 commit_id = commit.hex
                 type_id = commit.type
diff --git a/vcsserver/http_main.py b/vcsserver/http_main.py
--- a/vcsserver/http_main.py
+++ b/vcsserver/http_main.py
@@ -404,7 +404,20 @@ class HTTPApplication(object):
                 exc_info[0] = org_exc.__class__
                 exc_info[1] = org_exc
 
-            store_exception(id(exc_info), exc_info)
+            should_store_exc = True
+            if org_exc:
+                def get_exc_fqn(_exc_obj):
+                    module_name = getattr(org_exc.__class__, '__module__', 'UNKNOWN')
+                    return module_name + '.' + org_exc_name
+
+                exc_fqn = get_exc_fqn(org_exc)
+
+                if exc_fqn in ['mercurial.error.RepoLookupError',
+                               'vcsserver.exceptions.RefNotFoundException']:
+                    should_store_exc = False
+
+            if should_store_exc:
+                store_exception(id(exc_info), exc_info)
 
             tb_info = ''.join(
                 traceback.format_exception(exc_type, exc_value, exc_traceback))