Split out NoSharesError, stop adding attributes to NotEnoughSharesError, change humanize_failure to include the original exception string, update tests, behave better if humanize_failure fails.

src/allmydata/immutable/download.py

@@ -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",
+            msg = "Failed to get enough shareholders: have %d, need %d" \
-                                       len(self._share_buckets),
+                  % (len(self._share_buckets), self._verifycap.needed_shares)
-                                       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",
+                msg = "Unable to activate enough shares: have %d, need %d" \
-                                           have, self._verifycap.needed_shares)
+                      % (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()

src/allmydata/immutable/encode.py

@@ -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
+            msg = "lost too many shareholders during upload (still have %d, want %d): %s" % \
-            raise NotEnoughSharesError(msg, len(self.landlords),
+                  (len(self.landlords), self.shares_of_happiness, why)
-                                       self.shares_of_happiness)
+            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)

src/allmydata/immutable/upload.py

@@ -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,
+                if placed_shares:
-                                           self.shares_of_happiness)
+                    raise NotEnoughSharesError(msg)
+                else:
+                    raise NoSharesError(msg)
             else:
                 # we placed enough to be happy, so we're done
                 if self._status:

src/allmydata/interfaces.py

@@ -774,11 +774,11 @@ class IMutableFileNode(IFileNode, IMutableFilesystemNode):
         """
 
 class NotEnoughSharesError(Exception):
-    def __init__(self, msg, got, needed):
+    """Download was unable to get enough shares, or upload was unable to
-        Exception.__init__(self, msg)
+    place 'shares_of_happiness' shares."""
-        self.got = got
+
-        self.needed = needed
+class NoSharesError(Exception):
-        self.servermap = None
+    """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

src/allmydata/mutable/retrieve.py

@@ -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),
+                                      (format % args, self._last_failure))
-                                       len(self.shares), k)
             if self._bad_shares:
                 self.log("We found some bad shares this pass. You should "
                          "update the servermap and try again to check "

src/allmydata/test/test_cli.py

@@ -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,
+            self.failUnlessIn("NotEnoughSharesError: ", err)
-                            err)
+            self.failUnlessIn("Failed to get enough shareholders: have 1, need 3", err)
         d.addCallback(_check1)
 
         return d

src/allmydata/test/test_system.py

@@ -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),
+                self.failUnless(res.check(NoSharesError),
-                                "expected NotEnoughSharesError, got %s" % res)
+                                "expected NoSharesError, 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.
             d1.addBoth(_baduri_should_fail)
             return d1
         d.addCallback(_download_nonexistent_uri)

src/allmydata/test/test_upload.py

@@ -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

src/allmydata/test/test_web.py

@@ -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 "
+            exp = ("NotEnoughSharesError: This indicates that some "
-                   "3 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.")
+                   "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)
 

src/allmydata/web/common.py

@@ -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
+        t = ("NotEnoughSharesError: This indicates that some "
-        needed = f.value.needed
+             "servers were unavailable, or that shares have been "
-        if got == 0:
+             "lost to server departure, hard drive failure, or disk "
-            t = ("NotEnoughSharesError: no shares could be found. "
+             "corruption. You should perform a filecheck on "
-                 "Zero shares usually indicates a corrupt URI, or that "
+             "this object to learn more.\n\nThe full error message is:\n"
-                 "no servers were connected, but it might also indicate "
+             "%s") % str(f.value)
-                 "severe corruption. You should perform a filecheck on "
+        return (t, http.GONE)
-                 "this object to learn more.")
+    if f.check(NoSharesError):
-        else:
+        t = ("NoSharesError: no shares could be found. "
-            t = ("NotEnoughSharesError: %d share%s found, but we need "
+             "Zero shares usually indicates a corrupt URI, or that "
-                 "%d to recover the file. This indicates that some "
+             "no servers were connected, but it might also indicate "
-                 "servers were unavailable, or that shares have been "
+             "severe corruption. You should perform a filecheck on "
-                 "lost to server departure, hard drive failure, or disk "
+             "this object to learn more.\n\nThe full error message is:\n"
-                 "corruption. You should perform a filecheck on "
+             "%s") % str(f.value)
-                 "this object to learn more.") % (got, plural(got), needed)
         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):