mirror of
https://github.com/tahoe-lafs/tahoe-lafs.git
synced 2025-01-31 08:25:35 +00:00
web: full patch for HTML-vs-plaintext traceback renderings, improve test coverage of exception rendering
This commit is contained in:
parent
90226f335f
commit
f42e3bb107
@ -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
|
a series of HTML pages with links to download files, and forms that use POST
|
||||||
actions to upload, rename, and delete files.
|
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 ==
|
== URLs ==
|
||||||
|
|
||||||
Tahoe uses a variety of read- and write- caps to identify files and
|
Tahoe uses a variety of read- and write- caps to identify files and
|
||||||
|
@ -6,6 +6,7 @@ from twisted.trial import unittest
|
|||||||
from twisted.internet import defer, reactor
|
from twisted.internet import defer, reactor
|
||||||
from twisted.web import client, error, http
|
from twisted.web import client, error, http
|
||||||
from twisted.python import failure, log
|
from twisted.python import failure, log
|
||||||
|
from nevow import rend
|
||||||
from allmydata import interfaces, uri, webish
|
from allmydata import interfaces, uri, webish
|
||||||
from allmydata.storage.mutable import MutableShareFile
|
from allmydata.storage.mutable import MutableShareFile
|
||||||
from allmydata.storage.immutable import ShareFile
|
from allmydata.storage.immutable import ShareFile
|
||||||
@ -3171,3 +3172,113 @@ class Grid(GridTestMixin, WebErrorMixin, unittest.TestCase, ShouldFailMixin):
|
|||||||
|
|
||||||
d.addErrback(self.explain_web_error)
|
d.addErrback(self.explain_web_error)
|
||||||
return d
|
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")
|
||||||
|
@ -140,7 +140,22 @@ class MyExceptionHandler(appserver.DefaultExceptionHandler):
|
|||||||
"No such child: %s" % name.encode("utf-8"),
|
"No such child: %s" % name.encode("utf-8"),
|
||||||
http.NOT_FOUND)
|
http.NOT_FOUND)
|
||||||
elif f.check(NotEnoughSharesError):
|
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):
|
elif f.check(WebError):
|
||||||
return self.simple(ctx, f.value.text, f.value.code)
|
return self.simple(ctx, f.value.text, f.value.code)
|
||||||
elif f.check(FileTooLargeError):
|
elif f.check(FileTooLargeError):
|
||||||
|
@ -111,7 +111,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
|
|||||||
return PlaceHolderNodeHandler(self.client, self.node, name)
|
return PlaceHolderNodeHandler(self.client, self.node, name)
|
||||||
if DEBUG: print " 404"
|
if DEBUG: print " 404"
|
||||||
# otherwise, we just return a no-such-child error
|
# otherwise, we just return a no-such-child error
|
||||||
return rend.FourOhFour()
|
return f
|
||||||
|
|
||||||
node = node_or_failure
|
node = node_or_failure
|
||||||
if nonterminal and should_create_intermediate_directories(req):
|
if nonterminal and should_create_intermediate_directories(req):
|
||||||
|
@ -13,7 +13,8 @@ from allmydata.immutable.filenode import LiteralFileNode
|
|||||||
from allmydata.util import log, base32
|
from allmydata.util import log, base32
|
||||||
|
|
||||||
from allmydata.web.common import text_plain, WebError, RenderMixin, \
|
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, \
|
from allmydata.web.check_results import CheckResults, \
|
||||||
CheckAndRepairResults, LiteralCheckResults
|
CheckAndRepairResults, LiteralCheckResults
|
||||||
from allmydata.web.info import MoreInfo
|
from allmydata.web.info import MoreInfo
|
||||||
@ -398,19 +399,13 @@ class FileDownloader(rend.Page):
|
|||||||
#
|
#
|
||||||
# We don't have a lot of options, unfortunately.
|
# We don't have a lot of options, unfortunately.
|
||||||
req.write("problem during download\n")
|
req.write("problem during download\n")
|
||||||
|
req.finish()
|
||||||
else:
|
else:
|
||||||
# We haven't written anything yet, so we can provide a
|
# We haven't written anything yet, so we can provide a
|
||||||
# sensible error message.
|
# sensible error message.
|
||||||
msg = str(f.type)
|
eh = MyExceptionHandler()
|
||||||
msg.replace("\n", "|")
|
eh.renderHTTP_exception(ctx, f)
|
||||||
req.setResponseCode(http.GONE, msg)
|
d.addCallbacks(lambda ign: req.finish(), _error)
|
||||||
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())
|
|
||||||
return req.deferred
|
return req.deferred
|
||||||
|
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user