From: Brian Warner Date: Wed, 4 Mar 2009 04:56:30 +0000 (-0700) Subject: web: full patch for HTML-vs-plaintext traceback renderings, improve test coverage... X-Git-Url: https://git.rkrishnan.org/pf/content/en/service/FOOURL?a=commitdiff_plain;h=f42e3bb107179b15eb648be4f96c9136e26ab9a0;p=tahoe-lafs%2Ftahoe-lafs.git web: full patch for HTML-vs-plaintext traceback renderings, improve test coverage of exception rendering --- diff --git a/docs/frontends/webapi.txt b/docs/frontends/webapi.txt index 5d7f46d9..55d79e65 100644 --- a/docs/frontends/webapi.txt +++ b/docs/frontends/webapi.txt @@ -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 diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index a7f882f3..e83e2f6c 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -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("" 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("" 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("" 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("" 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") diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index c012be8d..5e070f39 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -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): diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 4a9c8ddb..2a54c250 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -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): diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 78e235b6..727d9b68 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -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