From 0393c989ae11377b5b52449499aee736658d0cfd Mon Sep 17 00:00:00 2001
From: Cedric Brandily <zzelle@gmail.com>
Date: Fri, 26 Sep 2014 15:05:21 +0200
Subject: [PATCH] Handle correctly http error raise in run_http_exc

Currently http errors are raised inside a try bloc and transformed in
a more generic exception. This change raises http errors outside the
try bloc.

Change-Id: Iece270ac84925625e848f3049072f39e19e068cc
---
 git-review.1                  |  4 ++++
 git_review/cmd.py             | 13 ++++++++----
 git_review/tests/test_unit.py | 38 +++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/git-review.1 b/git-review.1
index 99416940..195577f9 100644
--- a/git-review.1
+++ b/git-review.1
@@ -321,6 +321,10 @@ or code.  This exit status will also be returned if the patchset is already
 applied to the current branch.
 .It 70
 Cannot determine top level Git directory or .git subdirectory path.
+.It 101
+Unauthorized (401) http request done by git-review.
+.It 104
+Not Found (404) http request done by git-review.
 .El
 .Pp
 Exit status larger than 31 indicates problem with
diff --git a/git_review/cmd.py b/git_review/cmd.py
index c942c337..8a17b4c4 100755
--- a/git_review/cmd.py
+++ b/git_review/cmd.py
@@ -154,6 +154,11 @@ def run_command_exc(klazz, *argv, **env):
     return output
 
 
+def http_code_2_return_code(code):
+    """Tranform http status code to system return code."""
+    return (code - 301) % 255 + 1
+
+
 def run_http_exc(klazz, url, **env):
     """Run http GET request url, on failure raise klazz
 
@@ -168,12 +173,12 @@ def run_http_exc(klazz, url, **env):
 
     try:
         res = requests.get(url, **env)
-        if not 200 <= res.status_code < 300:
-            code = (res.status_code - 301) % 255 + 1
-            raise klazz(code, res.text, ('GET', url), env)
-        return res
     except Exception as err:
         raise klazz(255, str(err), ('GET', url), env)
+    if not 200 <= res.status_code < 300:
+        raise klazz(http_code_2_return_code(res.status_code),
+                    res.text, ('GET', url), env)
+    return res
 
 
 def get_version():
diff --git a/git_review/tests/test_unit.py b/git_review/tests/test_unit.py
index 2fb8066d..6e41aa14 100644
--- a/git_review/tests/test_unit.py
+++ b/git_review/tests/test_unit.py
@@ -152,3 +152,41 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
         self.assertTrue(cmd.check_use_color_output(),
                         "Failed to use fallback to color.ui when "
                         "color.review not present")
+
+
+class FakeResponse(object):
+
+    def __init__(self, code, text=""):
+        self.status_code = code
+        self.text = text
+
+
+class FakeException(Exception):
+
+    def __init__(self, code, *args, **kwargs):
+        super(FakeException, self).__init__(*args, **kwargs)
+        self.code = code
+
+
+class GitReviewUnitTest(testtools.TestCase):
+    """Class for misc unit tests."""
+
+    @mock.patch('requests.get', return_value=FakeResponse(404))
+    def test_run_http_exc_raise_http_error(self, mock_get):
+        url = 'http://gerrit.example.com'
+        try:
+            cmd.run_http_exc(FakeException, url)
+            self.fails('Exception expected')
+        except FakeException as err:
+            self.assertEqual(cmd.http_code_2_return_code(404), err.code)
+            mock_get.assert_called_once_with(url)
+
+    @mock.patch('requests.get', side_effect=Exception())
+    def test_run_http_exc_raise_unknown_error(self, mock_get):
+        url = 'http://gerrit.example.com'
+        try:
+            cmd.run_http_exc(FakeException, url)
+            self.fails('Exception expected')
+        except FakeException as err:
+            self.assertEqual(255, err.code)
+            mock_get.assert_called_once_with(url)