dirnode lookup: use distinct NoSuchChildError instead of the generic KeyError when a child can't be found

src/allmydata/dirnode.py

@@ -8,7 +8,7 @@ from allmydata.mutable.common import NotMutableError
 from allmydata.mutable.node import MutableFileNode
 from allmydata.interfaces import IMutableFileNode, IDirectoryNode,\
      IURI, IFileNode, IMutableFileURI, IFilesystemNode, \
-     ExistingChildError, ICheckable, IDeepCheckable
+     ExistingChildError, NoSuchChildError, ICheckable, IDeepCheckable
 from allmydata.checker_results import DeepCheckResults, \
      DeepCheckAndRepairResults
 from allmydata.monitor import Monitor
@@ -28,7 +28,7 @@ class Deleter:
         children = self.node._unpack_contents(old_contents)
         if self.name not in children:
             if self.must_exist:
-                raise KeyError(self.name)
+                raise NoSuchChildError(self.name)
             self.old_child = None
             return None
         self.old_child, metadata = children[self.name]
@@ -44,6 +44,8 @@ class MetadataSetter:
 
     def modify(self, old_contents):
         children = self.node._unpack_contents(old_contents)
+        if self.name not in children:
+            raise NoSuchChildError(self.name)
         children[self.name] = (children[self.name][0], self.metadata)
         new_contents = self.node._pack_contents(children)
         return new_contents
@@ -246,13 +248,13 @@ class NewDirectoryNode:
     def _get(self, children, name):
         child = children.get(name)
         if child is None:
-            raise KeyError(name)
+            raise NoSuchChildError(name)
         return child[0]
 
     def _get_with_metadata(self, children, name):
         child = children.get(name)
         if child is None:
-            raise KeyError(name)
+            raise NoSuchChildError(name)
         return child
 
     def get(self, name):

src/allmydata/ftpd.py

@@ -9,7 +9,8 @@ from twisted.protocols import ftp
 from twisted.cred import error, portal, checkers, credentials
 from twisted.web.client import getPage
 
-from allmydata.interfaces import IDirectoryNode, ExistingChildError
+from allmydata.interfaces import IDirectoryNode, ExistingChildError, \
+     NoSuchChildError
 from allmydata.immutable.download import ConsumerAdapter
 from allmydata.immutable.upload import FileHandle
 from allmydata.util import base32
@@ -86,7 +87,7 @@ class Handler:
             return defer.succeed(node)
         d = node.get(path[0])
         def _maybe_create(f):
-            f.trap(KeyError)
+            f.trap(NoSuchChildError)
             return node.create_empty_directory(path[0])
         d.addErrback(_maybe_create)
         d.addCallback(self._get_or_create_directories, path[1:])
@@ -159,7 +160,7 @@ class Handler:
         return d
 
     def _convert_error(self, f):
-        if f.check(KeyError):
+        if f.check(NoSuchChildError):
             childname = f.value.args[0].encode("utf-8")
             msg = "'%s' doesn't exist" % childname
             raise ftp.FileNotFoundError(msg)

src/allmydata/interfaces.py

@@ -653,6 +653,9 @@ class ExistingChildError(Exception):
     """A directory node was asked to add or replace a child that already
     exists, and overwrite= was set to False."""
 
+class NoSuchChildError(Exception):
+    """A directory node was asked to fetch a child which does not exist."""
+
 class IDirectoryNode(IMutableFilesystemNode):
     """I represent a name-to-child mapping, holding the tahoe equivalent of a
     directory. All child names are unicode strings, and all children are some
@@ -691,13 +694,15 @@ class IDirectoryNode(IMutableFilesystemNode):
     def get(name):
         """I return a Deferred that fires with a specific named child node,
         either an IFileNode or an IDirectoryNode. The child name must be a
-        unicode string."""
+        unicode string. I raise NoSuchChildError if I do not have a child by
+        that name."""
 
     def get_metadata_for(name):
         """I return a Deferred that fires with the metadata dictionary for a
         specific named child node. This metadata is stored in the *edge*, not
         in the child, so it is attached to the parent dirnode rather than the
-        child dir-or-file-node. The child name must be a unicode string."""
+        child dir-or-file-node. The child name must be a unicode string. I
+        raise NoSuchChildError if I do not have a child by that name."""
 
     def set_metadata_for(name, metadata):
         """I replace any existing metadata for the named child with the new
@@ -705,14 +710,15 @@ class IDirectoryNode(IMutableFilesystemNode):
         stored in the *edge*, not in the child, so it is attached to the
         parent dirnode rather than the child dir-or-file-node. I return a
         Deferred (that fires with this dirnode) when the operation is
-        complete."""
+        complete. I raise NoSuchChildError if I do not have a child by that
+        name."""
 
     def get_child_at_path(path):
         """Transform a child path into an IDirectoryNode or IFileNode.
 
         I perform a recursive series of 'get' operations to find the named
         descendant node. I return a Deferred that fires with the node, or
-        errbacks with IndexError if the node could not be found.
+        errbacks with NoSuchChildError if the node could not be found.
 
         The path can be either a single string (slash-separated) or a list of
         path-name elements. All elements must be unicode strings.
@@ -792,7 +798,8 @@ class IDirectoryNode(IMutableFilesystemNode):
     def delete(name):
         """I remove the child at the specific name. I return a Deferred that
         fires when the operation finishes. The child name must be a unicode
-        string."""
+        string. I raise NoSuchChildError if I do not have a child by that
+        name."""
 
     def create_empty_directory(name, overwrite=True):
         """I create and attach an empty directory at the given name. The
@@ -805,7 +812,8 @@ class IDirectoryNode(IMutableFilesystemNode):
         is referenced by name. On the new parent, the child will live under
         'new_child_name', which defaults to 'current_child_name'. TODO: what
         should we do about metadata? I return a Deferred that fires when the
-        operation finishes. The child name must be a unicode string."""
+        operation finishes. The child name must be a unicode string. I raise
+        NoSuchChildError if I do not have a child by that name."""
 
     def build_manifest():
         """Return a Monitor. The Monitor's results will be a list of (path,

src/allmydata/test/test_dirnode.py

@@ -7,7 +7,8 @@ from allmydata import uri, dirnode
 from allmydata.immutable import upload
 from allmydata.interfaces import IURI, IClient, IMutableFileNode, \
      INewDirectoryURI, IReadonlyNewDirectoryURI, IFileNode, \
-     ExistingChildError, IDeepCheckResults, IDeepCheckAndRepairResults
+     ExistingChildError, NoSuchChildError, \
+     IDeepCheckResults, IDeepCheckAndRepairResults
 from allmydata.util import hashutil, testutil
 from allmydata.monitor import Monitor
 from allmydata.test.common import make_chk_file_uri, make_mutable_file_uri, \
@@ -104,7 +105,7 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
                 self.failUnless(u"child" in children)
             d.addCallback(_check1)
             d.addCallback(lambda res:
-                          self.shouldFail(KeyError, "get bogus", None,
+                          self.shouldFail(NoSuchChildError, "get bogus", None,
                                           dn.get, u"bogus"))
             def _corrupt(res):
                 filenode = dn._node
@@ -381,8 +382,8 @@ class Dirnode(unittest.TestCase, testutil.ShouldFailMixin, testutil.StallMixin):
                                                ["ctime", "mtime"]))
 
             d.addCallback(lambda res:
-                          self.shouldFail(KeyError, "gcamap-no",
+                          self.shouldFail(NoSuchChildError, "gcamap-no",
-                                          "'nope'",
+                                          "nope",
                                           n.get_child_and_metadata_at_path,
                                           u"subdir/nope"))
             d.addCallback(lambda res:

src/allmydata/test/test_system.py

@@ -15,7 +15,7 @@ from allmydata.util import log, base32
 from allmydata.scripts import runner
 from allmydata.interfaces import IDirectoryNode, IFileNode, IFileURI, \
      ICheckerResults, ICheckAndRepairResults, IDeepCheckResults, \
-     IDeepCheckAndRepairResults
+     IDeepCheckAndRepairResults, NoSuchChildError
 from allmydata.monitor import Monitor, OperationCancelledError
 from allmydata.mutable.common import NotMutableError
 from allmydata.mutable import layout as mutable_layout
@@ -900,7 +900,7 @@ class SystemTest(SystemTestMixin, unittest.TestCase):
 
             d1.addCallback(lambda res: self.shouldFail2(NotMutableError, "set_uri(nope)", None, dirnode.set_uri, u"hopeless", self.uri))
 
-            d1.addCallback(lambda res: self.shouldFail2(KeyError, "get(missing)", "'missing'", dirnode.get, u"missing"))
+            d1.addCallback(lambda res: self.shouldFail2(NoSuchChildError, "get(missing)", "missing", dirnode.get, u"missing"))
 
             personal = self._personal_node
             d1.addCallback(lambda res: self.shouldFail2(NotMutableError, "mv from readonly", None, dirnode.move_child_to, u"mydata992", personal, u"nope"))

src/allmydata/web/directory.py

@@ -14,7 +14,7 @@ from foolscap.eventual import fireEventually
 from allmydata.util import base32
 from allmydata.uri import from_string_dirnode
 from allmydata.interfaces import IDirectoryNode, IFileNode, IMutableFileNode, \
-     ExistingChildError
+     ExistingChildError, NoSuchChildError
 from allmydata.monitor import Monitor
 from allmydata.web.common import text_plain, WebError, \
      IClient, IOpHandleTable, NeedOperationHandleError, \
@@ -72,7 +72,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
         t = get_arg(req, "t", "").strip()
         if isinstance(node_or_failure, Failure):
             f = node_or_failure
-            f.trap(KeyError)
+            f.trap(NoSuchChildError)
             # No child by this name. What should we do about it?
             if DEBUG: print "no child", name
             if DEBUG: print "postpath", req.postpath
@@ -235,7 +235,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
             return defer.succeed(node)
         d = node.get(path[0])
         def _maybe_create(f):
-            f.trap(KeyError)
+            f.trap(NoSuchChildError)
             return node.create_empty_directory(path[0])
         d.addErrback(_maybe_create)
         d.addCallback(self._get_or_create_directories, path[1:])
@@ -268,7 +268,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
         def _maybe_got_node(node_or_failure):
             if isinstance(node_or_failure, Failure):
                 f = node_or_failure
-                f.trap(KeyError)
+                f.trap(NoSuchChildError)
                 # create a placeholder which will see POST t=upload
                 return PlaceHolderNodeHandler(self.node, name)
             else: