From 72abfb0572cc2d45ff364003ca9e817a807f02e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= <pawelredzynski@gmail.com>
Date: Thu, 14 Nov 2019 15:33:40 +0100
Subject: [PATCH] remote: http: raise exception when download response with
 error status code

---
 dvc/exceptions.py              |  8 ++++++++
 dvc/remote/http.py             |  9 +++++++--
 tests/func/test_repro.py       |  3 ++-
 tests/unit/remote/test_http.py | 24 ++++++++++++++++++++++++
 tests/utils/httpd.py           |  4 ++--
 5 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/dvc/exceptions.py b/dvc/exceptions.py
index 1a167117a..3778be592 100644
--- a/dvc/exceptions.py
+++ b/dvc/exceptions.py
@@ -348,3 +348,11 @@ class NoOutputInExternalRepoError(DvcException):
                 relpath(path, external_repo_path), external_repo_url
             )
         )
+
+
+class HTTPErrorStatusCodeException(DvcException):
+    def __init__(self, code, reason):
+        super(HTTPErrorStatusCodeException, self).__init__(
+            "Server responded with error status code: '{}' and message: "
+            "'{}'".format(code, reason)
+        )
diff --git a/dvc/remote/http.py b/dvc/remote/http.py
index c22416625..d4ff1cd04 100644
--- a/dvc/remote/http.py
+++ b/dvc/remote/http.py
@@ -8,6 +8,7 @@ from funcy import cached_property, wrap_prop
 from dvc.config import Config
 from dvc.config import ConfigError
 from dvc.exceptions import DvcException
+from dvc.exceptions import HTTPErrorStatusCodeException
 from dvc.progress import Tqdm
 from dvc.remote.base import RemoteBASE
 from dvc.scheme import Schemes
@@ -37,7 +38,11 @@ class RemoteHTTP(RemoteBASE):
             )
 
     def _download(self, from_info, to_file, name=None, no_progress_bar=False):
-        request = self._request("GET", from_info.url, stream=True)
+        response = self._request("GET", from_info.url, stream=True)
+        if response.status_code != 200:
+            raise HTTPErrorStatusCodeException(
+                response.status_code, response.reason
+            )
         with Tqdm(
             total=None if no_progress_bar else self._content_length(from_info),
             leave=False,
@@ -46,7 +51,7 @@ class RemoteHTTP(RemoteBASE):
             disable=no_progress_bar,
         ) as pbar:
             with open(to_file, "wb") as fd:
-                for chunk in request.iter_content(chunk_size=self.CHUNK_SIZE):
+                for chunk in response.iter_content(chunk_size=self.CHUNK_SIZE):
                     fd.write(chunk)
                     fd.flush()
                     pbar.update(len(chunk))
diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py
index ae0727c14..f6e9a23a9 100644
--- a/tests/func/test_repro.py
+++ b/tests/func/test_repro.py
@@ -43,6 +43,7 @@ from tests.func.test_data_cloud import _should_test_ssh
 from tests.func.test_data_cloud import get_ssh_url
 from tests.func.test_data_cloud import TEST_AWS_REPO_BUCKET
 from tests.func.test_data_cloud import TEST_GCP_REPO_BUCKET
+from tests.utils.httpd import ContentMD5Handler
 from tests.utils.httpd import StaticFileServer
 
 
@@ -1176,7 +1177,7 @@ class TestReproExternalHTTP(TestReproExternalBase):
 
         self.dvc.remove("imported_file.dvc")
 
-        with StaticFileServer(handler="Content-MD5") as httpd:
+        with StaticFileServer(handler_class=ContentMD5Handler) as httpd:
             import_url = urljoin(self.get_remote(httpd.server_port), self.FOO)
             import_output = "imported_file"
             import_stage = self.dvc.imp_url(import_url, import_output)
diff --git a/tests/unit/remote/test_http.py b/tests/unit/remote/test_http.py
index 2b1ecdb07..d1c2a5efa 100644
--- a/tests/unit/remote/test_http.py
+++ b/tests/unit/remote/test_http.py
@@ -1,7 +1,11 @@
 import pytest
+from BaseHTTPServer import BaseHTTPRequestHandler
 
 from dvc.config import ConfigError
+from dvc.exceptions import HTTPErrorStatusCodeException
+from dvc.path_info import URLInfo
 from dvc.remote.http import RemoteHTTP
+from tests.utils.httpd import StaticFileServer
 
 
 def test_no_traverse_compatibility(dvc_repo):
@@ -13,3 +17,23 @@ def test_no_traverse_compatibility(dvc_repo):
 
     with pytest.raises(ConfigError):
         RemoteHTTP(dvc_repo, config)
+
+
+@pytest.mark.parametrize("response_code", [404, 403, 500])
+def test_download_fails_on_error_code(response_code, dvc_repo):
+    class ErrorStatusRequestHandler(BaseHTTPRequestHandler):
+        def do_GET(self):
+            self.send_response(response_code, message="Error message")
+            self.end_headers()
+
+    with StaticFileServer(ErrorStatusRequestHandler) as httpd:
+        url = "http://localhost:{}/".format(httpd.server_port)
+        config = {"url": url}
+
+        remote = RemoteHTTP(dvc_repo, config)
+        import os
+
+        with pytest.raises(HTTPErrorStatusCodeException):
+            remote._download(
+                URLInfo(os.path.join(url, "file.txt")), "file.txt"
+            )
diff --git a/tests/utils/httpd.py b/tests/utils/httpd.py
index ff902e71a..5883db7b7 100644
--- a/tests/utils/httpd.py
+++ b/tests/utils/httpd.py
@@ -45,9 +45,9 @@ class ContentMD5Handler(TestRequestHandler):
 class StaticFileServer:
     _lock = threading.Lock()
 
-    def __init__(self, handler="etag"):
+    def __init__(self, handler_class=ETagHandler):
         self._lock.acquire()
-        handler_class = ETagHandler if handler == "etag" else ContentMD5Handler
+        self.response_handler = handler_class
         self._httpd = HTTPServer(("localhost", 0), handler_class)
         self._thread = None