From bda7b7fbd6ace503470b12e572a1cc001a7b0e82 Mon Sep 17 00:00:00 2001 From: pje Date: Wed, 6 Oct 2004 18:27:11 +0000 Subject: Added "sensible default error handling", auto-calc of Content-Length, and various documentation enhancements. Handlers will now log exceptions to 'wsgi.errors' by default, and generate a trivial client error message (that can easily be changed in subclasses). Added tests for "error before headers sent" and "error after headers sent" scenarios. git-svn-id: svn://svn.eby-sarna.com/svnroot/wsgiref@251 571e12c6-e1fa-0310-aee7-ff1267fa46bd --- src/wsgiref/handlers.py | 198 ++++++++++++++++++++++++++----------- src/wsgiref/tests/test_handlers.py | 82 +++++++++++++-- 2 files changed, 213 insertions(+), 67 deletions(-) diff --git a/src/wsgiref/handlers.py b/src/wsgiref/handlers.py index d49b687..287e3bb 100644 --- a/src/wsgiref/handlers.py +++ b/src/wsgiref/handlers.py @@ -1,3 +1,5 @@ +"""Base classes for server/gateway implementations""" + from types import StringType from util import FileWrapper, guess_scheme from headers import Headers @@ -37,39 +39,37 @@ except NameError: - - class BaseHandler: """Manage the invocation of a WSGI application""" + # Configuration parameters; can override per-subclass or per-instance wsgi_version = (1,0) wsgi_multithread = True wsgi_multiprocess = True wsgi_run_once = False - # os_environ may be overridden at class or instance level, if desired - os_environ = dict(os.environ.items()) + # os_environ is used to supply configuration from the OS environment: + # by default it's a copy of 'os.environ' as of import time, but you can + # override this in e.g. your __init__ method. + os_environ = dict(os.environ.items()) # Collaborator classes wsgi_file_wrapper = FileWrapper # set to None to disable headers_class = Headers # must be a Headers-like class - # State variables + # Error handling (also per-subclass or per-instance) + traceback_limit = None # Print entire traceback to self.get_stderr() + error_status = "500 Dude, this is whack!" + error_headers = [('Content-Type','text/plain')] + error_body = "A server error occurred. Please contact the administrator." + + # State variables (don't mess with these) status = result = None headers_sent = False headers = None bytes_sent = 0 - def run(self, application): - """Invoke the application""" - try: - self.setup_environ() - self.result = application(self.environ, self.start_response) - self.finish_response() - except: - self.handle_error() - self.close() @@ -80,6 +80,26 @@ class BaseHandler: + def run(self, application): + """Invoke the application""" + # Note to self: don't move the close()! Asynchronous servers shouldn't + # call close() from finish_response(), so if you close() anywhere but + # the double-error branch here, you'll break asynchronous servers by + # prematurely closing. Async servers must return from 'run()' without + # closing if there might still be output to iterate over. + try: + self.setup_environ() + self.result = application(self.environ, self.start_response) + self.finish_response() + except: + try: + self.handle_error() + except: + # If we get an error handling an error, just give up already! + self.close() + raise # ...and let the actual server figure it out. + + def setup_environ(self): """Set up the environment for one request""" @@ -89,7 +109,7 @@ class BaseHandler: env['wsgi.input'] = self.get_stdin() env['wsgi.errors'] = self.get_stderr() env['wsgi.version'] = self.wsgi_version - env['wsgi.run_once'] = self.wsgi_run_once + env['wsgi.run_once'] = self.wsgi_run_once env['wsgi.url_scheme'] = self.get_scheme() env['wsgi.multithread'] = self.wsgi_multithread env['wsgi.multiprocess'] = self.wsgi_multiprocess @@ -98,27 +118,22 @@ class BaseHandler: env['wsgi.file_wrapper'] = self.wsgi_file_wrapper + + + def finish_response(self): """Send any iterable data, then close self and the iterable Subclasses intended for use in asynchronous servers will - probably want to redefine this method, such that it sets up - callbacks to iterate over the data, and to call 'self.close()' - when finished. + want to redefine this method, such that it sets up callbacks + in the event loop to iterate over the data, and to call + 'self.close()' once the response is finished. """ - try: - try: - if not self.result_is_file() and not self.sendfile(): - for data in self.result: - self.write(data) - self.finish_content() - except: - self.handle_error() - finally: - self.close() - - - + if not self.result_is_file() and not self.sendfile(): + for data in self.result: + self.write(data) + self.finish_content() + self.close() def get_scheme(self): @@ -126,9 +141,25 @@ class BaseHandler: return guess_scheme(self.environ) + def set_content_length(self): + """Compute Content-Length or switch to chunked encoding if possible""" + try: + blocks = len(self.result) + except (TypeError,AttributeError,NotImplementedError): + pass + else: + if blocks==1: + self.headers['Content-Length'] = str(self.bytes_sent) + return + # XXX Try for chunked encoding if enabled + + def cleanup_headers(self): """Make any necessary header changes or defaults""" - # XXX set up Content-Length, chunked encoding, if possible/needed + if not self.headers.has_key('Content-Length'): + self.set_content_length() + + # XXX set up Date, Server headers def start_response(self, status, headers,exc_info=None): @@ -162,6 +193,16 @@ class BaseHandler: + + + + + + + + + + def write(self, data): """'write()' callable as specified by PEP 333""" @@ -171,10 +212,12 @@ class BaseHandler: raise AssertionError("write() before start_response()") elif not self.headers_sent: - # Before the first output, send the stored headers - self.send_headers() + # Before the first output, send the stored headers + self.bytes_sent = len(data) # make sure we know content-length + self.send_headers() + else: + self.bytes_sent += len(data) - self.bytes_sent += len(data) # XXX check Content-Length and truncate if too many bytes written? self._write(data) self._flush() @@ -188,21 +231,19 @@ class BaseHandler: return iterable ('self.result') is an instance of 'self.wsgi_file_wrapper'. - This method should return a true value if it is able to + This method should return a true value if it was able to actually transmit the wrapped file-like object using a platform-specific approach. It should return a false value if normal iteration should be used instead. An exception can be raised to indicate that transmission was attempted, but failed. - NOTE: this method should call 'self.send_headers()' if it is - going to attempt direct transmission, and 'self.headers_sent' - is false. + NOTE: this method should call 'self.send_headers()' if + 'self.headers_sent' is false and it is going to attempt direct + transmission of the file1. """ return False # No platform-specific transmission by default - - def finish_content(self): """Ensure headers and content have both been sent""" if not self.headers_sent: @@ -212,15 +253,16 @@ class BaseHandler: pass # XXX check if content-length was too short? def close(self): - """Close the iterable, if needed, and reset all instance vars + """Close the iterable (if needed) and reset all instance vars Subclasses may want to also drop the client connection. """ - if hasattr(self.result,'close'): - self.result.close() - self.result = self.headers = self.status = self.environ = None - self.bytes_sent = 0 - self.headers_sent = False + try: + if hasattr(self.result,'close'): + self.result.close() + finally: + self.result = self.headers = self.status = self.environ = None + self.bytes_sent = 0; self.headers_sent = False def send_status(self): @@ -229,7 +271,6 @@ class BaseHandler: (BaseCGIHandler overrides this to use a "Status:" prefix.)""" self._write('%s\r\n' % status) - def send_headers(self): """Transmit headers to the client, via self._write()""" self.cleanup_headers() @@ -244,19 +285,64 @@ class BaseHandler: return wrapper is not None and isinstance(self.result,wrapper) - # Pure abstract methods; *must* be overridden in subclasses + def log_exception(self,exc_info): + """Log the 'exc_info' tuple in the server log + + Subclasses may override to retarget the output or change its format. + """ + try: + from traceback import print_exception + print_exception( + exc_info[0], exc_info[1], exc_info[2], + self.traceback_limit, self.get_stderr() + ) + finally: + exc_info = None + def handle_error(self): - """Override in subclass to handle error recovery and logging""" - # XXX this really should do something sensible by default - raise NotImplementedError + """Log current error, and send error output to client if possible""" + self.log_exception(sys.exc_info()) + if not self.headers_sent: + self.result = self.error_output(self.environ, self.start_response) + self.finish_response() + # XXX else: attempt advanced recovery techniques for HTML or text? + + def error_output(self, environ, start_response): + """WSGI mini-app to create error output + + By default, this just uses the 'error_status', 'error_headers', + and 'error_body' attributes to generate an output page. It can + be overridden in a subclass to dynamically generate diagnostics, + choose an appropriate message for the user's preferred language, etc. + + Note, however, that it's not recommended from a security perspective to + spit out diagnostics to any old user; ideally, you should have to do + something special to enable diagnostic output, which is why we don't + include any here! + """ + start_response(self.error_status, self.error_headers[:]) + return [self.error_body] + + + # Pure abstract methods; *must* be overridden in subclasses + def _write(self,data): - """Override in subclass to buffer data for send to client""" + """Override in subclass to buffer data for send to client + + It's okay if this method actually transmits the data; BaseHandler + just separates write and flush operations for greater efficiency + when the underlying system actually has such a distinction. + """ raise NotImplementedError def _flush(self): - """Override in subclass to force sending of recent '_write()' calls""" + """Override in subclass to force sending of recent '_write()' calls + + It's okay if this method is a no-op (i.e., if '_write()' actually + sends the data. + """ raise NotImplementedError def get_stdin(self): @@ -281,10 +367,6 @@ class BaseHandler: - - - - class BaseCGIHandler(BaseHandler): """CGI-like systems using input/output/error streams and environ mapping diff --git a/src/wsgiref/tests/test_handlers.py b/src/wsgiref/tests/test_handlers.py index 69b86d9..7d0863f 100644 --- a/src/wsgiref/tests/test_handlers.py +++ b/src/wsgiref/tests/test_handlers.py @@ -1,11 +1,12 @@ from __future__ import nested_scopes # Backward compat for 2.1 from unittest import TestCase, TestSuite, makeSuite from wsgiref.util import setup_testing_defaults +from wsgiref.headers import Headers from wsgiref.handlers import BaseHandler, BaseCGIHandler from StringIO import StringIO -class TestHandler(BaseCGIHandler): +class ErrorHandler(BaseCGIHandler): """Simple handler subclass for testing BaseHandler""" def __init__(self,**kw): @@ -15,6 +16,9 @@ class TestHandler(BaseCGIHandler): multithread=True, multiprocess=True ) +class TestHandler(ErrorHandler): + """Simple handler subclass for testing BaseHandler, w/error passthru""" + def handle_error(self): raise # for testing, we want to see what's happening @@ -33,10 +37,6 @@ class TestHandler(BaseCGIHandler): - - - - class HandlerTests(TestCase): @@ -83,21 +83,85 @@ class HandlerTests(TestCase): def testAbstractMethods(self): h = BaseHandler() for name in [ - 'handle_error','_flush','get_stdin','get_stderr','add_cgi_vars' + '_flush','get_stdin','get_stderr','add_cgi_vars' ]: self.assertRaises(NotImplementedError, getattr(h,name)) self.assertRaises(NotImplementedError, h._write, "test") - def testSimpleRun(self): + def testContentLength(self): + # Demo one reason iteration is better than write()... ;) + + def trivial_app1(e,s): + s('200 OK',[]) + return [e['wsgi.url_scheme']] + + def trivial_app2(e,s): + s('200 OK',[])(e['wsgi.url_scheme']) + return [] + h = TestHandler() - h.run(lambda e,s: [(s('200 OK',[]) or 1) and e['wsgi.url_scheme']]) - self.assertEqual(h.stdout.getvalue(),"Status: 200 OK\r\n\r\nhttp") + h.run(trivial_app1) + self.assertEqual(h.stdout.getvalue(), + "Status: 200 OK\r\n" + "Content-Length: 4\r\n" + "\r\n" + "http") + h = TestHandler() + h.run(trivial_app2) + self.assertEqual(h.stdout.getvalue(), + "Status: 200 OK\r\n" + "\r\n" + "http") + + + + def testBasicErrorOutput(self): + + def non_error_app(e,s): + s('200 OK',[]) + return [] + + def error_app(e,s): + raise AssertionError("This should be caught by handler") + + h = ErrorHandler() + h.run(non_error_app) + self.assertEqual(h.stdout.getvalue(), + "Status: 200 OK\r\n" + "Content-Length: 0\r\n" + "\r\n") + self.assertEqual(h.stderr.getvalue(),"") + + h = ErrorHandler() + h.run(error_app) + self.assertEqual(h.stdout.getvalue(), + "Status: %s\r\n" + "Content-Type: text/plain\r\n" + "Content-Length: %d\r\n" + "\r\n%s" % (h.error_status,len(h.error_body),h.error_body)) + + self.failUnless(h.stderr.getvalue().find("AssertionError")<>-1) + + def testErrorAfterOutput(self): + MSG = "Some output has been sent" + def error_app(e,s): + s("200 OK",[])(MSG) + raise AssertionError("This should be caught by handler") + + h = ErrorHandler() + h.run(error_app) + self.assertEqual(h.stdout.getvalue(), + "Status: 200 OK\r\n" + "\r\n"+MSG) + self.failUnless(h.stderr.getvalue().find("AssertionError")<>-1) + + TestClasses = ( HandlerTests, -- cgit v1.2.1