Split out NoSharesError, stop adding attributes to NotEnoughSharesError, change human...
authorBrian Warner <warner@lothar.com>
Thu, 25 Jun 2009 02:17:07 +0000 (19:17 -0700)
committerBrian Warner <warner@lothar.com>
Thu, 25 Jun 2009 02:17:07 +0000 (19:17 -0700)
src/allmydata/immutable/download.py
src/allmydata/immutable/encode.py
src/allmydata/immutable/upload.py
src/allmydata/interfaces.py
src/allmydata/mutable/retrieve.py
src/allmydata/test/test_cli.py
src/allmydata/test/test_system.py
src/allmydata/test/test_upload.py
src/allmydata/test/test_web.py
src/allmydata/web/common.py

index 9dfc0bb80d546f4645cf85bb6ab7e8e293470419..01ce4127ab953ed548147875fb62e8c75b013e29 100644 (file)
@@ -11,7 +11,7 @@ from allmydata import codec, hashtree, uri
 from allmydata.interfaces import IDownloadTarget, IDownloader, \
      IFileURI, IVerifierURI, \
      IDownloadStatus, IDownloadResults, IValidatedThingProxy, \
-     IStorageBroker, NotEnoughSharesError, NoServersError, \
+     IStorageBroker, NotEnoughSharesError, NoSharesError, NoServersError, \
      UnableToFetchCriticalDownloadDataError
 from allmydata.immutable import layout
 from allmydata.monitor import Monitor
@@ -818,9 +818,12 @@ class CiphertextDownloader(log.PrefixingLogMixin):
             self._results.timings["peer_selection"] = now - self._started
 
         if len(self._share_buckets) < self._verifycap.needed_shares:
-            raise NotEnoughSharesError("Failed to get enough shareholders",
-                                       len(self._share_buckets),
-                                       self._verifycap.needed_shares)
+            msg = "Failed to get enough shareholders: have %d, need %d" \
+                  % (len(self._share_buckets), self._verifycap.needed_shares)
+            if self._share_buckets:
+                raise NotEnoughSharesError(msg)
+            else:
+                raise NoSharesError(msg)
 
         #for s in self._share_vbuckets.values():
         #    for vb in s:
@@ -906,8 +909,12 @@ class CiphertextDownloader(log.PrefixingLogMixin):
             potential_shnums = list(available_shnums - handled_shnums)
             if len(potential_shnums) < (self._verifycap.needed_shares - len(self.active_buckets)):
                 have = len(potential_shnums) + len(self.active_buckets)
-                raise NotEnoughSharesError("Unable to activate enough shares",
-                                           have, self._verifycap.needed_shares)
+                msg = "Unable to activate enough shares: have %d, need %d" \
+                      % (have, self._verifycap.needed_shares)
+                if have:
+                    raise NotEnoughSharesError(msg)
+                else:
+                    raise NoSharesError(msg)
             # For the next share, choose a primary share if available, else a randomly chosen
             # secondary share.
             potential_shnums.sort()
index 5c1f18695a2612c216105c4cd460b852cc67de34..5629a8b855d24e359bc9c897b65eba996b875276 100644 (file)
@@ -11,7 +11,7 @@ from allmydata.util import mathutil, hashutil, base32, log
 from allmydata.util.assertutil import _assert, precondition
 from allmydata.codec import CRSEncoder
 from allmydata.interfaces import IEncoder, IStorageBucketWriter, \
-     IEncryptedUploadable, IUploadStatus, NotEnoughSharesError
+     IEncryptedUploadable, IUploadStatus, NotEnoughSharesError, NoSharesError
 
 """
 The goal of the encoder is to turn the original file into a series of
@@ -487,9 +487,12 @@ class Encoder(object):
             self.log("they weren't in our list of landlords", parent=ln,
                      level=log.WEIRD, umid="TQGFRw")
         if len(self.landlords) < self.shares_of_happiness:
-            msg = "lost too many shareholders during upload: %s" % why
-            raise NotEnoughSharesError(msg, len(self.landlords),
-                                       self.shares_of_happiness)
+            msg = "lost too many shareholders during upload (still have %d, want %d): %s" % \
+                  (len(self.landlords), self.shares_of_happiness, why)
+            if self.landlords:
+                raise NotEnoughSharesError(msg)
+            else:
+                raise NoSharesError(msg)
         self.log("but we can still continue with %s shares, we'll be happy "
                  "with at least %s" % (len(self.landlords),
                                        self.shares_of_happiness),
@@ -504,7 +507,7 @@ class Encoder(object):
             # otherwise be consumed. Allow non-NotEnoughSharesError exceptions
             # to pass through as an unhandled errback. We use this in lieu of
             # consumeErrors=True to allow coding errors to be logged.
-            f.trap(NotEnoughSharesError)
+            f.trap(NotEnoughSharesError, NoSharesError)
             return None
         for d0 in dl:
             d0.addErrback(_eatNotEnoughSharesError)
index 0dc541dbfada0ac8079b9aeffc5ddc6ec6832370..18f276773ec32420d27d03ae23d08c8fc7892f9f 100644 (file)
@@ -17,7 +17,8 @@ from allmydata.util.assertutil import precondition
 from allmydata.util.rrefutil import add_version_to_remote_reference
 from allmydata.interfaces import IUploadable, IUploader, IUploadResults, \
      IEncryptedUploadable, RIEncryptedUploadable, IUploadStatus, \
-     NotEnoughSharesError, InsufficientVersionError, NoServersError
+     NotEnoughSharesError, NoSharesError, NoServersError, \
+     InsufficientVersionError
 from allmydata.immutable import layout
 from pycryptopp.cipher.aes import AES
 
@@ -286,11 +287,13 @@ class Tahoe2PeerSelector:
             placed_shares = self.total_shares - len(self.homeless_shares)
             if placed_shares < self.shares_of_happiness:
                 msg = ("placed %d shares out of %d total (%d homeless), "
+                       "want to place %d, "
                        "sent %d queries to %d peers, "
                        "%d queries placed some shares, %d placed none, "
                        "got %d errors" %
                        (self.total_shares - len(self.homeless_shares),
                         self.total_shares, len(self.homeless_shares),
+                        self.shares_of_happiness,
                         self.query_count, self.num_peers_contacted,
                         self.good_query_count, self.bad_query_count,
                         self.error_count))
@@ -298,8 +301,10 @@ class Tahoe2PeerSelector:
                 if self.last_failure_msg:
                     msg += " (%s)" % (self.last_failure_msg,)
                 log.msg(msg, level=log.UNUSUAL, parent=self._log_parent)
-                raise NotEnoughSharesError(msg, placed_shares,
-                                           self.shares_of_happiness)
+                if placed_shares:
+                    raise NotEnoughSharesError(msg)
+                else:
+                    raise NoSharesError(msg)
             else:
                 # we placed enough to be happy, so we're done
                 if self._status:
index e130fa0f340adb2fb3ab9f8f7f198448d47593af..fc82018162fe9540c9dc4a506cd5d73052aadfb1 100644 (file)
@@ -774,11 +774,11 @@ class IMutableFileNode(IFileNode, IMutableFilesystemNode):
         """
 
 class NotEnoughSharesError(Exception):
-    def __init__(self, msg, got, needed):
-        Exception.__init__(self, msg)
-        self.got = got
-        self.needed = needed
-        self.servermap = None
+    """Download was unable to get enough shares, or upload was unable to
+    place 'shares_of_happiness' shares."""
+
+class NoSharesError(Exception):
+    """Upload or Download was unable to get any shares at all."""
 
 class UnableToFetchCriticalDownloadDataError(Exception):
     """I was unable to fetch some piece of critical data which is supposed to
index 935ec50fbcd70df5fe75228bfed0521d215be554..a72e1088b3c0e438b3b33ccbba4b49e49bff30b3 100644 (file)
@@ -465,8 +465,7 @@ class Retrieve:
             self.log(format=format,
                      level=log.WEIRD, umid="ezTfjw", **args)
             err = NotEnoughSharesError("%s, last failure: %s" %
-                                      (format % args, self._last_failure),
-                                       len(self.shares), k)
+                                      (format % args, self._last_failure))
             if self._bad_shares:
                 self.log("We found some bad shares this pass. You should "
                          "update the servermap and try again to check "
index 84d17334a5c98433f110dc7059b5845ed6dcb1a3..f990ebb3c4e3e03a96f28478234ea7e9ca11d494 100644 (file)
@@ -1425,8 +1425,8 @@ class Errors(GridTestMixin, CLITestMixin, unittest.TestCase):
         def _check1((rc, out, err)):
             self.failIfEqual(rc, 0)
             self.failUnless("410 Gone" in err, err)
-            self.failUnless("NotEnoughSharesError: 1 share found, but we need 3" in err,
-                            err)
+            self.failUnlessIn("NotEnoughSharesError: ", err)
+            self.failUnlessIn("Failed to get enough shareholders: have 1, need 3", err)
         d.addCallback(_check1)
 
         return d
index 9a7f469806e60fe15eb0ff16a5ffa5ea1c783b7f..b9a1b593bc55589d2d4bee1ea2069d738c280b42 100644 (file)
@@ -16,7 +16,7 @@ from allmydata.util import idlib, mathutil
 from allmydata.util import log, base32
 from allmydata.scripts import runner
 from allmydata.interfaces import IDirectoryNode, IFileNode, IFileURI, \
-     NoSuchChildError, NotEnoughSharesError
+     NoSuchChildError, NotEnoughSharesError, NoSharesError
 from allmydata.monitor import Monitor
 from allmydata.mutable.common import NotMutableError
 from allmydata.mutable import layout as mutable_layout
@@ -219,7 +219,7 @@ class SystemTest(SystemTestMixin, unittest.TestCase):
             bad_n = self.clients[1].create_node_from_uri(bad_u.to_string())
             # this should cause an error during download
 
-            d = self.shouldFail2(NotEnoughSharesError, "'download bad node'",
+            d = self.shouldFail2(NoSharesError, "'download bad node'",
                                  None,
                                  bad_n.read, MemoryConsumer(), offset=2)
             return d
@@ -234,12 +234,8 @@ class SystemTest(SystemTestMixin, unittest.TestCase):
                 log.msg("finished downloading non-existend URI",
                         level=log.UNUSUAL, facility="tahoe.tests")
                 self.failUnless(isinstance(res, Failure))
-                self.failUnless(res.check(NotEnoughSharesError),
-                                "expected NotEnoughSharesError, got %s" % res)
-                # TODO: files that have zero peers should get a special kind
-                # of NotEnoughSharesError, which can be used to suggest that
-                # the URI might be wrong or that they've never uploaded the
-                # file in the first place.
+                self.failUnless(res.check(NoSharesError),
+                                "expected NoSharesError, got %s" % res)
             d1.addBoth(_baduri_should_fail)
             return d1
         d.addCallback(_download_nonexistent_uri)
index ffeb4ee7fbc726e7c25e5faae4cfb5a039e26d7b..45c85d765399ba088c72ce8b5d1cad473f157b50 100644 (file)
@@ -10,7 +10,8 @@ from foolscap.api import fireEventually
 import allmydata # for __full_version__
 from allmydata import uri, monitor
 from allmydata.immutable import upload
-from allmydata.interfaces import IFileURI, FileTooLargeError, NotEnoughSharesError
+from allmydata.interfaces import IFileURI, FileTooLargeError, \
+     NotEnoughSharesError, NoSharesError
 from allmydata.util.assertutil import precondition
 from allmydata.util.deferredutil import DeferredListShouldSucceed
 from no_network import GridTestMixin
@@ -381,7 +382,7 @@ class FullServer(unittest.TestCase):
         self.u.parent = self.node
 
     def _should_fail(self, f):
-        self.failUnless(isinstance(f, Failure) and f.check(NotEnoughSharesError), f)
+        self.failUnless(isinstance(f, Failure) and f.check(NoSharesError), f)
 
     def test_data_large(self):
         data = DATA
index 49c4e29faf8632295a12d0e40a31c362bee0d838..c3bd38758c32d5a42cf5b66d6f1e05894ef4ab89 100644 (file)
@@ -3237,16 +3237,17 @@ class Grid(GridTestMixin, WebErrorMixin, unittest.TestCase, ShouldFailMixin):
 
         d.addCallback(lambda ignored:
                       self.shouldHTTPError("GET unrecoverable",
-                                           410, "Gone", "NotEnoughSharesError",
+                                           410, "Gone", "NoSharesError",
                                            self.GET, self.fileurls["0shares"]))
         def _check_zero_shares(body):
             self.failIf("<html>" in body, body)
             body = " ".join(body.strip().split())
-            exp = ("NotEnoughSharesError: no shares could be found. "
+            exp = ("NoSharesError: no shares could be found. "
                    "Zero shares usually indicates a corrupt URI, or that "
                    "no servers were connected, but it might also indicate "
                    "severe corruption. You should perform a filecheck on "
-                   "this object to learn more.")
+                   "this object to learn more. The full error message is: "
+                   "Failed to get enough shareholders: have 0, need 3")
             self.failUnlessEqual(exp, body)
         d.addCallback(_check_zero_shares)
 
@@ -3258,12 +3259,12 @@ class Grid(GridTestMixin, WebErrorMixin, unittest.TestCase, ShouldFailMixin):
         def _check_one_share(body):
             self.failIf("<html>" in body, body)
             body = " ".join(body.strip().split())
-            exp = ("NotEnoughSharesError: 1 share found, but we need "
-                   "3 to recover the file. This indicates that some "
+            exp = ("NotEnoughSharesError: This indicates that some "
                    "servers were unavailable, or that shares have been "
                    "lost to server departure, hard drive failure, or disk "
                    "corruption. You should perform a filecheck on "
-                   "this object to learn more.")
+                   "this object to learn more. The full error message is:"
+                   " Failed to get enough shareholders: have 1, need 3")
             self.failUnlessEqual(exp, body)
         d.addCallback(_check_one_share)
 
index 5c33758186f2e121e837e93fd9eee822bd1712ff..28aec62c50e879abaaa93da1d510e577f78cc7cd 100644 (file)
@@ -1,11 +1,12 @@
 
 from twisted.web import http, server
+from twisted.python import log
 from zope.interface import Interface
 from nevow import loaders, appserver
 from nevow.inevow import IRequest
 from nevow.util import resource_filename
 from allmydata.interfaces import ExistingChildError, NoSuchChildError, \
-     FileTooLargeError, NotEnoughSharesError
+     FileTooLargeError, NotEnoughSharesError, NoSharesError
 from allmydata.mutable.common import UnrecoverableFileError
 from allmydata.util import abbreviate # TODO: consolidate
 
@@ -124,21 +125,20 @@ def humanize_failure(f):
         name = f.value.args[0]
         return ("No such child: %s" % name.encode("utf-8"), http.NOT_FOUND)
     if f.check(NotEnoughSharesError):
-        got = f.value.got
-        needed = f.value.needed
-        if got == 0:
-            t = ("NotEnoughSharesError: no shares could be found. "
-                 "Zero shares usually indicates a corrupt URI, or that "
-                 "no servers were connected, but it might also indicate "
-                 "severe corruption. You should perform a filecheck on "
-                 "this object to learn more.")
-        else:
-            t = ("NotEnoughSharesError: %d share%s found, but we need "
-                 "%d to recover the file. This indicates that some "
-                 "servers were unavailable, or that shares have been "
-                 "lost to server departure, hard drive failure, or disk "
-                 "corruption. You should perform a filecheck on "
-                 "this object to learn more.") % (got, plural(got), needed)
+        t = ("NotEnoughSharesError: This indicates that some "
+             "servers were unavailable, or that shares have been "
+             "lost to server departure, hard drive failure, or disk "
+             "corruption. You should perform a filecheck on "
+             "this object to learn more.\n\nThe full error message is:\n"
+             "%s") % str(f.value)
+        return (t, http.GONE)
+    if f.check(NoSharesError):
+        t = ("NoSharesError: no shares could be found. "
+             "Zero shares usually indicates a corrupt URI, or that "
+             "no servers were connected, but it might also indicate "
+             "severe corruption. You should perform a filecheck on "
+             "this object to learn more.\n\nThe full error message is:\n"
+             "%s") % str(f.value)
         return (t, http.GONE)
     if f.check(UnrecoverableFileError):
         t = ("UnrecoverableFileError: the directory (or mutable file) could "
@@ -170,7 +170,13 @@ class MyExceptionHandler(appserver.DefaultExceptionHandler):
         req.finishRequest(False)
 
     def renderHTTP_exception(self, ctx, f):
-        text, code = humanize_failure(f)
+        try:
+            text, code = humanize_failure(f)
+        except:
+            log.msg("exception in humanize_failure")
+            log.msg("argument was %s" % (f,))
+            log.err()
+            text, code = str(f), None
         if code is not None:
             return self.simple(ctx, text, code)
         if f.check(server.UnsupportedMethod):