From d888bf33772327f2a7974b1671d65f32b80f392e Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Mon, 11 Jan 2010 17:33:43 -0800
Subject: [PATCH] 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.
---
 src/allmydata/immutable/checker.py |  4 ++--
 src/allmydata/mutable/servermap.py |  4 ++--
 src/allmydata/test/test_upload.py  |  2 +-
 src/allmydata/test/test_util.py    | 19 +++++++++++++++++++
 src/allmydata/util/log.py          |  6 +++---
 5 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/allmydata/immutable/checker.py b/src/allmydata/immutable/checker.py
index cbb8637d..2672a409 100644
--- 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 ccc586b6..5798a448 100644
--- 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 c3b65eca..c098c197 100644
--- 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 1052a05e..6607d458 100644
--- 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 fab22f6b..e1487deb 100644
--- 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() """
-- 
2.45.2