Merge lp:~fwierzbicki/txaws/dont-log-stacktraces-for-4xx-errors into lp:txaws

[Frank Wierzbicki]

This branch logs only a message and not an error/stacktrace for 4xx errors. This fixes a bug in the downstream project clouddeck. See linked bug.

[Duncan McGreggor (oubiwann)]

+1 from me; even without the downstream impact, this is a good change.

review: Approve

=== modified file 'txaws/server/resource.py'
--- txaws/server/resource.py	2011-09-23 08:20:46 +0000
+++ txaws/server/resource.py	2011-10-05 21:18:23 +0000
@@ -98,13 +98,20 @@
             return response
 
         def write_error(failure):
-            log.err(failure)
             if failure.check(APIError):
                 status = failure.value.status
+
+                # Don't log the stack traces for 4xx responses.
+                if status < 400 or status >= 500:
+                    log.err(failure)
+                else:
+                    log.msg("status: %s message: %s" % (status, failure.value))
+
                 bytes = failure.value.response
                 if bytes is None:
                     bytes = self.dump_error(failure.value, request)
             else:
+                log.err(failure)
                 bytes = str(failure.value)
                 status = 500
             request.setResponseCode(status)

=== modified file 'txaws/server/tests/test_resource.py'
--- txaws/server/tests/test_resource.py	2011-09-23 08:20:46 +0000
+++ txaws/server/tests/test_resource.py	2011-10-05 21:18:23 +0000
@@ -240,7 +240,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("InvalidSignature - SignatureVersion '2' "
                              "not supported", request.response)
             self.assertEqual(403, request.code)
@@ -262,7 +263,8 @@
         self.api.execute = lambda call: 1 / 0
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(1, len(errors))
             self.assertTrue(request.finished)
             self.assertEqual("integer division or modulo by zero",
                              request.response)
@@ -284,7 +286,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("MissingParameter - The request must contain "
                              "the parameter Action", request.response)
             self.assertEqual(400, request.code)
@@ -300,7 +303,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("InvalidAction - The action FooBar is not valid"
                              " for this web service.", request.response)
             self.assertEqual(400, request.code)
@@ -323,7 +327,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("InvalidAction - The action CantDoIt is not "
                              "valid for this web service.", request.response)
             self.assertEqual(400, request.code)
@@ -344,7 +349,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("InvalidAction - The action FooBar is not valid"
                              " for this web service.", request.response)
             self.assertEqual(400, request.code)
@@ -363,7 +369,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("AuthFailure - No user with access key 'access'",
                              request.response)
             self.assertEqual(401, request.code)
@@ -383,7 +390,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual("SignatureDoesNotMatch - The request signature "
                              "we calculated does not match the signature you "
                              "provided. Check your key and signing method.",
@@ -407,7 +415,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual(
                 "InvalidParameterCombination - The parameter Timestamp"
                 " cannot be used with the parameter Expires",
@@ -450,7 +459,8 @@
         request = FakeRequest(query.params, endpoint)
 
         def check(ignored):
-            self.flushLoggedErrors()
+            errors = self.flushLoggedErrors()
+            self.assertEquals(0, len(errors))
             self.assertEqual(
                 "RequestExpired - Request has expired. Expires date is"
                 " 2010-01-01T12:00:00Z", request.response)