← Back to team overview

dulwich-users team mailing list archive

[PATCH 2/8] web: Distinguish between missing files and read errors.

 

From: Dave Borowitz <dborowitz@xxxxxxxxxx>

Missing files now return a 404, whereas read errors return a 500.

We now do not need to log 404s and 403s, since they will typically be
logged by the WSGI container and the root causes are fairly unambiguous.

Change-Id: Ibf3728af53e56756512ed92dd0260ceb9e029539
---
 NEWS                      |    7 +++++++
 dulwich/tests/test_web.py |    3 ++-
 dulwich/web.py            |   12 ++++++++++--
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 89bb03d..22b67a9 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,13 @@
 
   * New public function dulwich.pack.write_pack_header. (Dave Borowitz)
 
+  * Distinguish between missing files and read errors in HTTP server.
+    (Dave Borowitz)
+
+ TESTS
+
+  * Use GitFile when modifying packed-refs in tests. (Dave Borowitz)
+
  API CHANGES
 
   * ObjectStore.iter_tree_contents now walks contents in depth-first, sorted
diff --git a/dulwich/tests/test_web.py b/dulwich/tests/test_web.py
index 9e5010a..1018f28 100644
--- a/dulwich/tests/test_web.py
+++ b/dulwich/tests/test_web.py
@@ -31,6 +31,7 @@ from dulwich.web import (
     HTTP_OK,
     HTTP_NOT_FOUND,
     HTTP_FORBIDDEN,
+    HTTP_ERROR,
     send_file,
     get_info_refs,
     handle_service_request,
@@ -98,7 +99,7 @@ class DumbHandlersTestCase(WebTestCase):
 
         f = TestFile()
         list(send_file(self._req, f, 'text/plain'))
-        self.assertEquals(HTTP_NOT_FOUND, self._status)
+        self.assertEquals(HTTP_ERROR, self._status)
         self.assertTrue(f.closed)
 
     def test_get_info_refs(self):
diff --git a/dulwich/web.py b/dulwich/web.py
index d42bd6a..76c1dac 100644
--- a/dulwich/web.py
+++ b/dulwich/web.py
@@ -48,6 +48,7 @@ logger = log_utils.getLogger(__name__)
 HTTP_OK = '200 OK'
 HTTP_NOT_FOUND = '404 Not Found'
 HTTP_FORBIDDEN = '403 Forbidden'
+HTTP_ERROR = '500 Internal Server Error'
 
 
 def date_time_string(timestamp=None):
@@ -100,7 +101,7 @@ def send_file(req, f, content_type):
         f.close()
     except IOError:
         f.close()
-        yield req.not_found('Error reading file')
+        yield req.error('Error reading file')
     except:
         f.close()
         raise
@@ -128,7 +129,7 @@ def get_loose_object(req, backend, mat):
     try:
         data = object_store[sha].as_legacy_object()
     except IOError:
-        yield req.not_found('Error reading object')
+        yield req.error('Error reading object')
     req.cache_forever()
     req.respond(HTTP_OK, 'application/x-git-loose-object')
     yield data
@@ -285,6 +286,13 @@ class HTTPGitRequest(object):
         self.respond(HTTP_FORBIDDEN, 'text/plain')
         return message
 
+    def error(self, message):
+        """Begin a HTTP 500 response and return the text of a message."""
+        self._cache_headers = []
+        logger.error('Error: %s', message)
+        self.respond(HTTP_ERROR, 'text/plain')
+        return message
+
     def nocache(self):
         """Set the response to never be cached by the client."""
         self._cache_headers = [
-- 
1.7.1




References