]> git.rkrishnan.org Git - tahoe-lafs/tahoe-lafs.git/commitdiff
web: full patch for HTML-vs-plaintext traceback renderings, improve test coverage...
authorBrian Warner <warner@lothar.com>
Wed, 4 Mar 2009 04:56:30 +0000 (21:56 -0700)
committerBrian Warner <warner@lothar.com>
Wed, 4 Mar 2009 04:56:30 +0000 (21:56 -0700)
docs/frontends/webapi.txt
src/allmydata/test/test_web.py
src/allmydata/web/common.py
src/allmydata/web/directory.py
src/allmydata/web/filenode.py

index 5d7f46d948c095c1c91cb28c5929769c22e14db9..55d79e65559bd22cfea96b2c504a6de2111182ad 100644 (file)
@@ -77,6 +77,19 @@ using a standard web browser to work with the filesystem. This user is given
 a series of HTML pages with links to download files, and forms that use POST
 actions to upload, rename, and delete files.
 
+When an error occurs, the HTTP response code will be set to an appropriate
+400-series code (like 404 for an unknown childname, or 400 Gone when a file
+is unrecoverable due to insufficient shares), and the HTTP response body will
+usually contain a few lines of explanation as to the cause of the error and
+possible responses. Unusual exceptions may result in a 500 Internal Server
+Error as a catch-all, with a default response body will contain a
+Nevow-generated HTML-ized representation of the Python exception stack trace
+that caused the problem. CLI programs which want to copy the response body to
+stderr should provide an "Accept: text/plain" header to their requests to get
+a plain text stack trace instead. If the Accept header contains */*, or
+text/*, or text/html (or if there is no Accept header), HTML tracebacks will
+be generated.
+
 == URLs ==
 
 Tahoe uses a variety of read- and write- caps to identify files and
index a7f882f33f3e19a3b45c29338073f42040c8ef4f..e83e2f6cdc956a7dbcca995970c5c8bd649c3151 100644 (file)
@@ -6,6 +6,7 @@ from twisted.trial import unittest
 from twisted.internet import defer, reactor
 from twisted.web import client, error, http
 from twisted.python import failure, log
+from nevow import rend
 from allmydata import interfaces, uri, webish
 from allmydata.storage.mutable import MutableShareFile
 from allmydata.storage.immutable import ShareFile
@@ -3171,3 +3172,113 @@ class Grid(GridTestMixin, WebErrorMixin, unittest.TestCase, ShouldFailMixin):
 
         d.addErrback(self.explain_web_error)
         return d
+
+
+    def test_exceptions(self):
+        self.basedir = "web/Grid/exceptions"
+        self.set_up_grid(num_clients=1, num_servers=2)
+        c0 = self.g.clients[0]
+        self.fileurls = {}
+        DATA = "data" * 100
+        d = c0.create_empty_dirnode()
+        def _stash_root(n):
+            self.fileurls["root"] = "uri/" + urllib.quote(n.get_uri()) + "/"
+            self.fileurls["imaginary"] = self.fileurls["root"] + "imaginary"
+            return n
+        d.addCallback(_stash_root)
+        d.addCallback(lambda ign: c0.upload(upload.Data(DATA, convergence="")))
+        def _stash_bad(ur):
+            self.fileurls["1share"] = "uri/" + urllib.quote(ur.uri)
+            self.delete_shares_numbered(ur.uri, range(1,10))
+
+            u = uri.from_string(ur.uri)
+            u.key = testutil.flip_bit(u.key, 0)
+            baduri = u.to_string()
+            self.fileurls["0shares"] = "uri/" + urllib.quote(baduri)
+        d.addCallback(_stash_bad)
+
+        # NotEnoughSharesError should be reported sensibly, with a
+        # text/plain explanation of the problem, and perhaps some
+        # information on which shares *could* be found.
+
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError("GET unrecoverable",
+                                           410, "Gone", "NotEnoughSharesError",
+                                           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. "
+                   "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.")
+            self.failUnlessEqual(exp, body)
+        d.addCallback(_check_zero_shares)
+
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError("GET 1share",
+                                           410, "Gone", "NotEnoughSharesError",
+                                           self.GET, self.fileurls["1share"]))
+        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 "
+                   "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.")
+            self.failUnlessEqual(exp, body)
+        d.addCallback(_check_one_share)
+
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError("GET imaginary",
+                                           404, "Not Found", None,
+                                           self.GET, self.fileurls["imaginary"]))
+        def _missing_child(body):
+            self.failUnless("No such child: imaginary" in body, body)
+        d.addCallback(_missing_child)
+
+        # attach a webapi child that throws a random error, to test how it
+        # gets rendered.
+        w = c0.getServiceNamed("webish")
+        w.root.putChild("ERRORBOOM", ErrorBoom())
+
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError("GET errorboom_html",
+                                           500, "Internal Server Error", None,
+                                           self.GET, "ERRORBOOM"))
+        def _internal_error_html(body):
+            # test that a weird exception during a webapi operation with
+            # Accept:*/* results in a text/html stack trace, while one
+            # without that Accept: line gets us a text/plain stack trace
+            self.failUnless("<html>" in body, "expected HTML, not '%s'" % body)
+        d.addCallback(_internal_error_html)
+
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError("GET errorboom_text",
+                                           500, "Internal Server Error", None,
+                                           self.GET, "ERRORBOOM",
+                                           headers={"accept": ["text/plain"]}))
+        def _internal_error_text(body):
+            # test that a weird exception during a webapi operation with
+            # Accept:*/* results in a text/html stack trace, while one
+            # without that Accept: line gets us a text/plain stack trace
+            self.failIf("<html>" in body, body)
+            self.failUnless(body.startswith("Traceback "), body)
+        d.addCallback(_internal_error_text)
+
+        def _flush_errors(res):
+            # Trial: please ignore the CompletelyUnhandledError in the logs
+            self.flushLoggedErrors(CompletelyUnhandledError)
+            return res
+        d.addBoth(_flush_errors)
+
+        return d
+
+class CompletelyUnhandledError(Exception):
+    pass
+class ErrorBoom(rend.Page):
+    def beforeRender(self, ctx):
+        raise CompletelyUnhandledError("whoops")
index c012be8ddd8b6bac3914060f5b07a8509e732b0e..5e070f395518e85a76935f75968cbabcbe619972 100644 (file)
@@ -140,7 +140,22 @@ class MyExceptionHandler(appserver.DefaultExceptionHandler):
                                "No such child: %s" % name.encode("utf-8"),
                                http.NOT_FOUND)
         elif f.check(NotEnoughSharesError):
-            return self.simple(ctx, str(f), http.GONE)
+            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)
+            return self.simple(ctx, t, http.GONE)
         elif f.check(WebError):
             return self.simple(ctx, f.value.text, f.value.code)
         elif f.check(FileTooLargeError):
index 4a9c8ddb2f603d13339f1d27e04d40f4714de11e..2a54c25063929051a29a309509141837033a8451 100644 (file)
@@ -111,7 +111,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
                     return PlaceHolderNodeHandler(self.client, self.node, name)
             if DEBUG: print " 404"
             # otherwise, we just return a no-such-child error
-            return rend.FourOhFour()
+            return f
 
         node = node_or_failure
         if nonterminal and should_create_intermediate_directories(req):
index 78e235b60cedb7e762b01cb3752dcb01621baee9..727d9b68b4f607fa46a9a5a2e0a3b100522d9392 100644 (file)
@@ -13,7 +13,8 @@ from allmydata.immutable.filenode import LiteralFileNode
 from allmydata.util import log, base32
 
 from allmydata.web.common import text_plain, WebError, RenderMixin, \
-     boolean_of_arg, get_arg, should_create_intermediate_directories
+     boolean_of_arg, get_arg, should_create_intermediate_directories, \
+     MyExceptionHandler
 from allmydata.web.check_results import CheckResults, \
      CheckAndRepairResults, LiteralCheckResults
 from allmydata.web.info import MoreInfo
@@ -398,19 +399,13 @@ class FileDownloader(rend.Page):
                 #
                 # We don't have a lot of options, unfortunately.
                 req.write("problem during download\n")
+                req.finish()
             else:
                 # We haven't written anything yet, so we can provide a
                 # sensible error message.
-                msg = str(f.type)
-                msg.replace("\n", "|")
-                req.setResponseCode(http.GONE, msg)
-                req.setHeader("content-type", "text/plain")
-                req.responseHeaders.setRawHeaders("content-encoding", [])
-                req.responseHeaders.setRawHeaders("content-disposition", [])
-                # TODO: HTML-formatted exception?
-                req.write(str(f))
-        d.addErrback(_error)
-        d.addBoth(lambda ign: req.finish())
+                eh = MyExceptionHandler()
+                eh.renderHTTP_exception(ctx, f)
+        d.addCallbacks(lambda ign: req.finish(), _error)
         return req.deferred