handle Deferred from render

src/allmydata/test/web/test_grid.py

@@ -18,6 +18,10 @@ from allmydata.storage.shares import get_share_file
 from allmydata.scripts.debug import CorruptShareOptions, corrupt_share
 from allmydata.immutable import upload
 from allmydata.mutable import publish
+
+from ...web.common import (
+    render_exception,
+)
 from .. import common_util as testutil
 from ..common import WebErrorMixin, ShouldFailMixin
 from ..no_network import GridTestMixin
@@ -34,6 +38,7 @@ class CompletelyUnhandledError(Exception):
     pass
 
 class ErrorBoom(object, resource.Resource):
+    @render_exception
     def render(self, req):
         raise CompletelyUnhandledError("whoops")
 

src/allmydata/web/common.py

@@ -3,15 +3,43 @@ import time
 import json
 from functools import wraps
 
+from hyperlink import (
+    DecodedURL,
+)
+
 from twisted.web import (
     http,
     resource,
     server,
     template,
 )
+from twisted.web.template import (
+    tags,
+)
+from twisted.web.util import (
+    FailureElement,
+    redirectTo,
+)
+from twisted.python.reflect import (
+    fullyQualifiedName,
+)
+from twisted.python.urlpath import (
+    URLPath,
+)
 from twisted.python import log
+from twisted.python.failure import (
+    Failure,
+)
+from twisted.internet.defer import (
+    Deferred,
+    maybeDeferred,
+)
+from twisted.web.resource import (
+    IResource,
+)
 from nevow import appserver
 from nevow.inevow import IRequest
+
 from allmydata import blacklist
 from allmydata.interfaces import (
     EmptyPathnameComponentError,
@@ -320,48 +348,10 @@ def humanize_failure(f):
 
 
 class MyExceptionHandler(appserver.DefaultExceptionHandler, object):
-    def simple(self, ctx, text, code=http.BAD_REQUEST):
-        req = IRequest(ctx)
-        req.setResponseCode(code)
-        #req.responseHeaders.setRawHeaders("content-encoding", [])
-        #req.responseHeaders.setRawHeaders("content-disposition", [])
-        req.setHeader("content-type", "text/plain;charset=utf-8")
-        if isinstance(text, unicode):
-            text = text.encode("utf-8")
-        req.setHeader("content-length", b"%d" % len(text))
-        req.write(text)
-        # TODO: consider putting the requested URL here
-        req.finishRequest(False)
-
     def renderHTTP_exception(self, ctx, 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):
-            # twisted.web.server.Request.render() has support for transforming
-            # this into an appropriate 501 NOT_IMPLEMENTED or 405 NOT_ALLOWED
-            # return code, but nevow does not.
-            req = IRequest(ctx)
-            method = req.method
-            return self.simple(ctx,
-                               "I don't know how to treat a %s request." % method,
-                               http.NOT_IMPLEMENTED)
         req = IRequest(ctx)
-        accept = req.getHeader("accept")
+        req.write(_renderHTTP_exception(req, f))
-        if not accept:
+        req.finishRequest(False)
-            accept = "*/*"
-        if "*/*" in accept or "text/*" in accept or "text/html" in accept:
-            super = appserver.DefaultExceptionHandler
-            return super.renderHTTP_exception(self, ctx, f)
-        # use plain text
-        traceback = f.getTraceback()
-        return self.simple(ctx, traceback, http.INTERNAL_SERVER_ERROR)
 
 
 class NeedOperationHandleError(WebError):
@@ -481,16 +471,130 @@ def exception_to_child(f):
     return g
 
 
-def render_exception(f):
+def render_exception(render):
     """
     Decorate a ``render_*`` method with exception handling behavior to render
     an error page reflecting the exception.
     """
-    @wraps(f)
+    @wraps(render)
     def g(self, request):
-        try:
+        bound_render = render.__get__(self, type(self))
-            return f(self, request)
+        result = maybeDeferred(bound_render, request)
-        except Exception as e:
+        if isinstance(result, Deferred):
-            description, status = humanize_exception(e)
+            result.addBoth(_finish, bound_render, request)
-            return resource.ErrorPage(status, "Error", description).render(request)
+            return server.NOT_DONE_YET
+        elif isinstance(result, bytes):
+            return result
+        elif result == server.NOT_DONE_YET:
+            return server.NOT_DONE_YET
+        else:
+            raise Exception("{!r} returned unusable {!r}".format(
+                fullyQualifiedName(bound_render),
+                result,
+            ))
+
     return g
+
+
+def _finish(result, render, request):
+    if isinstance(result, Failure):
+        _finish(
+            _renderHTTP_exception(request, result),
+            render,
+            request,
+        )
+    elif IResource.providedBy(result):
+        # If result is also using @render_exception then we don't want to
+        # double-apply the logic.  This leads to an attempt to double-finish
+        # the request.  If it isn't using @render_exception then you should
+        # fix it so it is.
+        result.render(request)
+    elif isinstance(result, bytes):
+        request.write(result)
+        request.finish()
+    elif isinstance(result, URLPath):
+        if result.netloc == b"":
+            root = URLPath.fromRequest(request)
+            result.scheme = root.scheme
+            result.netloc = root.netloc
+        _finish(redirectTo(str(result), request), render, request)
+    elif isinstance(result, DecodedURL):
+        _finish(redirectTo(str(result), request), render, request)
+    elif result is None:
+        request.finish()
+    elif result == server.NOT_DONE_YET:
+        pass
+    else:
+        log.err("Request for {!r} handled by {!r} returned unusable {!r}".format(
+            request.uri,
+            fullyQualifiedName(render),
+            result,
+        ))
+        request.setResponseCode(http.INTERNAL_SERVER_ERROR)
+        _finish(b"", render, request)
+
+
+def _renderHTTP_exception(request, failure):
+    try:
+        text, code = humanize_failure(failure)
+    except:
+        log.msg("exception in humanize_failure")
+        log.msg("argument was %s" % (failure,))
+        log.err()
+        text = str(failure)
+        code = None
+
+    if code is not None:
+        return _renderHTTP_exception_simple(request, text, code)
+
+    if failure.check(server.UnsupportedMethod):
+        # twisted.web.server.Request.render() has support for transforming
+        # this into an appropriate 501 NOT_IMPLEMENTED or 405 NOT_ALLOWED
+        # return code, but nevow does not.
+        method = request.method
+        return _renderHTTP_exception_simple(
+            request,
+            "I don't know how to treat a %s request." % (method,),
+            http.NOT_IMPLEMENTED,
+        )
+
+    accept = request.getHeader("accept")
+    if not accept:
+        accept = "*/*"
+    if "*/*" in accept or "text/*" in accept or "text/html" in accept:
+        request.setResponseCode(http.INTERNAL_SERVER_ERROR)
+        return template.renderElement(
+            request,
+            tags.html(
+                tags.head(
+                    tags.title(u"Exception"),
+                ),
+                tags.body(
+                    FailureElement(failure),
+                ),
+            ),
+        )
+
+    # use plain text
+    traceback = failure.getTraceback()
+    return _renderHTTP_exception_simple(
+        request,
+        traceback,
+        http.INTERNAL_SERVER_ERROR,
+    )
+
+
+def _renderHTTP_exception_simple(request, text, code):
+    request.setResponseCode(code)
+    request.setHeader("content-type", "text/plain;charset=utf-8")
+    if isinstance(text, unicode):
+        text = text.encode("utf-8")
+    request.setHeader("content-length", b"%d" % len(text))
+    return text
+
+
+def handle_when_done(req, d):
+    when_done = get_arg(req, "when_done", None)
+    if when_done:
+        d.addCallback(lambda res: DecodedURL.from_text(when_done.decode("utf-8")))
+    return d

src/allmydata/web/directory.py

@@ -58,6 +58,7 @@ from allmydata.web.common import (
     SlotsSequenceElement,
     exception_to_child,
     render_exception,
+    handle_when_done,
 )
 from allmydata.web.filenode import ReplaceMeMixin, \
      FileNodeHandler, PlaceHolderNodeHandler
@@ -206,6 +207,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object):
                 )
         return make_handler_for(node, self.client, self.node, name)
 
+    @render_exception
     def render_DELETE(self, req):
         assert self.parentnode and self.name
         d = self.parentnode.delete(self.name)
@@ -310,13 +312,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object):
         else:
             raise WebError("POST to a directory with bad t=%s" % t)
 
-        when_done = get_arg(req, "when_done", None)
+        return handle_when_done(req, d)
-        if when_done:
-            def done(res):
-                req.redirect(when_done)
-                return res
-            d.addCallback(done)
-        return d
 
     def _POST_mkdir(self, req):
         name = get_arg(req, "name", "")

src/allmydata/web/filenode.py

@@ -1,6 +1,10 @@
 
 import json
 
+from hyperlink import (
+    DecodedURL,
+)
+
 from twisted.web import http, static
 from twisted.internet import defer
 from twisted.web.resource import (
@@ -8,8 +12,6 @@ from twisted.web.resource import (
     ErrorPage,
 )
 
-from nevow import url
-
 from allmydata.interfaces import ExistingChildError
 from allmydata.monitor import Monitor
 from allmydata.immutable.upload import FileHandle
@@ -34,8 +36,8 @@ from allmydata.web.common import (
     render_exception,
     should_create_intermediate_directories,
     text_plain,
-    MyExceptionHandler,
     WebError,
+    handle_when_done,
 )
 from allmydata.web.check_results import (
     CheckResultsRenderer,
@@ -150,10 +152,7 @@ class PlaceHolderNodeHandler(Resource, ReplaceMeMixin):
             # placeholder.
             raise WebError("POST to a file: bad t=%s" % t)
 
-        when_done = get_arg(req, "when_done", None)
+        return handle_when_done(req, d)
-        if when_done:
-            d.addCallback(lambda res: when_done)
-        return d
 
 
 class FileNodeHandler(Resource, ReplaceMeMixin, object):
@@ -315,10 +314,7 @@ class FileNodeHandler(Resource, ReplaceMeMixin, object):
         else:
             raise WebError("POST to file: bad t=%s" % t)
 
-        when_done = get_arg(req, "when_done", None)
+        return handle_when_done(req, d)
-        if when_done:
-            d.addCallback(lambda res: url.URL.fromString(when_done))
-        return d
 
     def _maybe_literal(self, res, Results_Class):
         if res:
@@ -485,23 +481,11 @@ class FileDownloader(Resource, object):
         if req.method == "HEAD":
             return ""
 
-        finished = []
-        def _request_finished(ign):
-            finished.append(True)
-        req.notifyFinish().addBoth(_request_finished)
-
         d = self.filenode.read(req, first, size)
 
-        def _finished(ign):
-            if not finished:
-                req.finish()
         def _error(f):
             lp = log.msg("error during GET", facility="tahoe.webish", failure=f,
                          level=log.UNUSUAL, umid="xSiF3w")
-            if finished:
-                log.msg("but it's too late to tell them", parent=lp,
-                        level=log.UNUSUAL, umid="j1xIbw")
-                return
             req._tahoe_request_had_error = f # for HTTP-style logging
             if req.startedWriting:
                 # The content-type is already set, and the response code has
@@ -513,15 +497,16 @@ class FileDownloader(Resource, object):
                 # error response be shorter than the intended results.
                 #
                 # We don't have a lot of options, unfortunately.
-                req.write("problem during download\n")
+                return b"problem during download\n"
-                req.finish()
             else:
                 # We haven't written anything yet, so we can provide a
                 # sensible error message.
-                eh = MyExceptionHandler()
+                return f
-                eh.renderHTTP_exception(req, f)
+        d.addCallbacks(
-        d.addCallbacks(_finished, _error)
+            lambda ignored: None,
-        return req.deferred
+            _error,
+        )
+        return d
 
 
 def _file_json_metadata(req, filenode, edge_metadata):

src/allmydata/web/operations.py

@@ -1,10 +1,15 @@
 
 import time
-from nevow import url
+from hyperlink import (
+    DecodedURL,
+)
 from twisted.web.template import (
     renderer,
     tags as T,
 )
+from twisted.python.urlpath import (
+    URLPath,
+)
 from twisted.python.failure import Failure
 from twisted.internet import reactor, defer
 from twisted.web import resource
@@ -84,17 +89,14 @@ class OphandleTable(resource.Resource, service.Service):
         """
         :param allmydata.webish.MyRequest req:
         """
-        ophandle = get_arg(req, "ophandle")
+        ophandle = get_arg(req, "ophandle").decode("utf-8")
         assert ophandle
-        target = get_root(req) + "/operations/" + ophandle
+        root = DecodedURL.from_text(unicode(URLPath.fromRequest(req)))
+        target = root.click(u"/").child(u"operations", ophandle)
         output = get_arg(req, "output")
         if output:
-            target = target + "?output=%s" % output
+            target = target.add(u"output", output.decode("utf-8"))
-
+        return target
-        # XXX: We have to use nevow.url here because nevow.appserver
-        # is unhappy with anything else; so this gets its own ticket.
-        # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3314
-        return url.URL.fromString(target)
 
     @exception_to_child
     def getChild(self, name, req):

src/allmydata/web/unlinked.py

@@ -2,6 +2,9 @@
 import urllib
 from twisted.web import http
 from twisted.internet import defer
+from twisted.python.urlpath import (
+    URLPath,
+)
 from twisted.python.filepath import FilePath
 from twisted.web.resource import Resource
 from twisted.web.template import (
@@ -66,7 +69,7 @@ def POSTUnlinkedCHK(req, client):
         def _done(upload_results, redir_to):
             if "%(uri)s" in redir_to:
                 redir_to = redir_to.replace("%(uri)s", urllib.quote(upload_results.get_uri()))
-            return url.URL.fromString(redir_to)
+            return URLPath.fromString(redir_to)
         d.addCallback(_done, when_done)
     else:
         # return the Upload Results page, which includes the URI