Clean up log.err calls, for one of the issues in #889.

allmydata.util.log.err() either takes a Failure as the first positional
argument, or takes no positional arguments and must be invoked in an
exception handler. Fixed its signature to match both foolscap.logging.log.err
and twisted.python.log.err . Included a brief unit test.

diff --git a/src/allmydata/immutable/checker.py b/src/allmydata/immutable/checker.py
index cbb8637d1102c30923e787a3bd53d27a1f25ab57..2672a409f6649f785b1117c1c619d44ea4721d21 100644 (file)
--- a/src/allmydata/immutable/checker.py
+++ b/src/allmydata/immutable/checker.py
@@ -118,10 +118,10 @@ class Checker(log.PrefixingLogMixin):
                      level=log.WEIRD, umid="atbAxw")
             return
         # local errors are cause for alarm
-        log.err(format="local error in add_lease to [%(peerid)s]: %(f_value)s",
+        log.err(f,
+                format="local error in add_lease to [%(peerid)s]: %(f_value)s",
                 peerid=idlib.shortnodeid_b2a(peerid),
                 f_value=str(f.value),
-                failure=f,
                 level=log.WEIRD, umid="hEGuQg")
 
 

diff --git a/src/allmydata/mutable/servermap.py b/src/allmydata/mutable/servermap.py
index ccc586b6f6a756c8ec6b5fc936c29f1b316e4e79..5798a448fb32a052edb891a834e942e8c83628f7 100644 (file)
--- a/src/allmydata/mutable/servermap.py
+++ b/src/allmydata/mutable/servermap.py
@@ -770,10 +770,10 @@ class ServermapUpdater:
                      level=log.WEIRD, umid="iqg3mw")
             return
         # local errors are cause for alarm
-        log.err(format="local error in add_lease to [%(peerid)s]: %(f_value)s",
+        log.err(f,
+                format="local error in add_lease to [%(peerid)s]: %(f_value)s",
                 peerid=idlib.shortnodeid_b2a(peerid),
                 f_value=str(f.value),
-                failure=f,
                 level=log.WEIRD, umid="ZWh6HA")
 
     def _query_failed(self, f, peerid):

diff --git a/src/allmydata/test/test_upload.py b/src/allmydata/test/test_upload.py
index c3b65eca9e4ce80d047bf62a7e3ffc3a8972c7ee..c098c197035c129608f6afeafe04c5c551f9949d 100644 (file)
--- a/src/allmydata/test/test_upload.py
+++ b/src/allmydata/test/test_upload.py
@@ -162,7 +162,7 @@ class FakeBucketWriter:
         self.closed = True
 
     def remote_abort(self):
-        log.err("uh oh, I was asked to abort")
+        log.err(RuntimeError("uh oh, I was asked to abort"))
 
 class FakeClient:
     DEFAULT_ENCODING_PARAMETERS = {"k":25,

diff --git a/src/allmydata/test/test_util.py b/src/allmydata/test/test_util.py
index 1052a05e5402a4d4175cf94f0b29a484ed666d68..6607d458a2a1fd54f4644f17db669b41e9008abd 100644 (file)
--- a/src/allmydata/test/test_util.py
+++ b/src/allmydata/test/test_util.py
@@ -12,6 +12,7 @@ from allmydata.util import base32, idlib, humanreadable, mathutil, hashutil
 from allmydata.util import assertutil, fileutil, deferredutil, abbreviate
 from allmydata.util import limiter, time_format, pollmixin, cachedir
 from allmydata.util import statistics, dictutil, pipeline
+from allmydata.util import log as tahoe_log
 
 class Base32(unittest.TestCase):
     def test_b2a_matches_Pythons(self):
@@ -1492,3 +1493,21 @@ class Pipeline(unittest.TestCase):
 
         self.calls[1][0].callback("two-result")
         self.calls[2][0].errback(ValueError("three-error"))
+
+class SampleError(Exception):
+    pass
+
+class Log(unittest.TestCase):
+    def test_err(self):
+        if not hasattr(self, "flushLoggedErrors"):
+            # without flushLoggedErrors, we can't get rid of the
+            # twisted.log.err that tahoe_log records, so we can't keep this
+            # test from [ERROR]ing
+            raise unittest.SkipTest("needs flushLoggedErrors from Twisted-2.5.0")
+        try:
+            raise SampleError("simple sample")
+        except:
+            f = Failure()
+        tahoe_log.err(format="intentional sample error",
+                      failure=f, level=tahoe_log.OPERATIONAL, umid="wO9UoQ")
+        self.flushLoggedErrors(SampleError)

diff --git a/src/allmydata/util/log.py b/src/allmydata/util/log.py
index fab22f6bce3108a0680b532dea7c32c9e79aba16..e1487debb47f6ef3287dcb5e01f641d8360bd600 100644 (file)
--- a/src/allmydata/util/log.py
+++ b/src/allmydata/util/log.py
@@ -20,11 +20,11 @@ msg = log.msg
 # thing happens that is nevertheless handled, use log.msg(failure=f,
 # level=WEIRD) instead.
 
-def err(*args, **kwargs):
-    tw_log.err(*args, **kwargs)
+def err(failure=None, _why=None, **kwargs):
+    tw_log.err(failure, _why, **kwargs)
     if 'level' not in kwargs:
         kwargs['level'] = log.UNUSUAL
-    return log.err(*args, **kwargs)
+    return log.err(failure, _why, **kwargs)
 
 class LogMixin(object):
     """ I remember a msg id and a facility and pass them to log.msg() """