t.web.wsgi.WSGIResource - WSGI application error handling

[osuchw]

Currently if there is an unhandled error in a WSGI application the WSGIResource will log the traceback but the request does not finish leaving the client spinning its wheels forever.

According to  PEP-333 it is the application responsibility to handle all the errors but it would be nice if WSGIResource returned 500 in such a case.

I did a quick check and it looks like web2.WSGIResource, mod_wsgi and cherrypy do return 500, making the twisted.web version the odd man out.

To demonstrate here are runnable examples. The t.web version

from twisted.web import server
from twisted.web.wsgi import WSGIResource
from twisted.python.threadpool import ThreadPool
from twisted.internet import reactor
from twisted.application import service, strports

wsgiThreadPool = ThreadPool()
wsgiThreadPool.start()

reactor.addSystemEventTrigger('after', 'shutdown', wsgiThreadPool.stop)

def application(environ, start_response):
    1/0
    start_response('200 OK', [('Content-type','text/plain')])
    return ['Hello World!']

wsgiAppAsResource = WSGIResource(reactor, wsgiThreadPool, application)

application = service.Application('Twisted.web.wsgi Hello World Example')
server = strports.service('tcp:8080', server.Site(wsgiAppAsResource))
server.setServiceParent(application)

and t.web2 version

from twisted.web2 import server, channel
from twisted.web2.wsgi import WSGIResource
from twisted.application import service, strports

def application(environ, start_response):
    1/0
    start_response('200 OK', [('Content-type','text/plain')])
    return ['Hello World!']

wsgiAppAsResource = WSGIResource(application)

application = service.Application('Twisted.web2.wsgi Hello World Example')
server = strports.service('tcp:8080', channel.HTTPFactory(server.Site(wsgiAppAsResource)))
server.setServiceParent(application)

[exarkun]

Thanks. Adding error handling to properly finish requests in this case would be great. Do you think you can put together a patch?

[osuchw]

Well, here is an attempt. No tests though. I would not know where to start with them.

[osuchw]

Updated

[osuchw]

updated to work with current trunk

[exarkun]

It looks like this belongs in review. But! a cursory glance at the patch reveals no unit tests, so how about I make some suggestions there.

All of the existing tests for this code are in twisted/web/test/test_wsgi.py. ApplicationTests is probably a good case to extent. You can test the various error handling being added by defining various application objects which each exhibit one of the error cases. Then use the lowLevelRender function to issue a request and in a callback on the rendering Deferred, assert that the right things happened (ie, exceptions were logged, the server returned a 500 response, whatever is appropriate).

Thanks!

[exarkun]

(In [27522]) Branching to 'wsgi-error-handling-4019'

[therve]

Looks good. As an additional feature it would be nice if an error page was displayed, I think, like ErrorPage is doing. It could probably be done in another branch though. Please merge!

[exarkun]

(In [27539]) Merge wsgi-error-handling-4019

Author: exarkun Reviewer: therve Fixes: #4019

In the Twisted Web WSGI container, handle exceptions raised by the application object by turning them into 500 responses where possible or by closing the connection when it is too late to set the response code.

[osuchw]

Thanks for working on this. Now I know how it should have been done. Live and learn :)