Issue #288 resolved

HTTP Error Handling simplifications

Anonymous created an issue

HTTP Error Handling simplifications.

  • rename HTTPStatusError HTTPError
  • Merge _cpOnHTTPError with _cpOnError
  • get rid of configurable template + page style
  • do not wrap custom messages in a html body

Reported by mikerobi

Comments (10)

  1. Robert Brewer

    I still think you should move the test for HTTPError out of _cpOnError--there's no reason to log a traceback for an HTTPError, for example. It can be quite confusing, because an "HTTP error" is not really a CherryPy or Python (unexpected) error, but we use Exceptions to handle them! The three concepts are quite distinct, however, and should be separated carefully.

    So where to handle it instead? If an HTTPError should invoke the error filters (an open question), then HTTPError should instead be special-cased in handleError like this:

        if not isinstance(exc, cherrypy.HTTPError):
            # _cpOnError will probably change cherrypy.response.body.
            # They may also change the headerMap, etc.
            getSpecialAttribute('_cpOnError')()
    

    If the filters should ''not'' be invoked, then HTTPError should be special-cased in Request.run (with another except: block like the one for HTTPRedirect):

        def run(self):
            """Process the Request."""
            try:
                try:
                    applyFilters('onStartResource')
                    
                    try:
                        self.processRequestHeaders()
                        
                        applyFilters('beforeRequestBody')
                        if cherrypy.request.processRequestBody:
                            self.processRequestBody()
                        
                        applyFilters('beforeMain')
                        if cherrypy.response.body is None:
                            main()
                        
                        applyFilters('beforeFinalize')
                        finalize()
                    except cherrypy.RequestHandled:
                        pass
                    except cherrypy.HTTPRedirect, inst:
                        # For an HTTPRedirect, we don't go through the regular
                        # mechanism: we return the redirect immediately
                        inst.set_response()
                        applyFilters('beforeFinalize')
                        finalize()
                    except HTTPError:
                        # This includes NotFound
                        applyFilters('beforeFinalize')
                        finalize()
                finally:
                    applyFilters('onEndResource')
            except:
                handleError(sys.exc_info())
    

    If you choose the latter, then HTTPError and HTTPRedirect should probably have the same interface; that is, they should either both use set_response() or neither. Heck, they should both be the same regardless of where you handle HTTPError.

  2. Anonymous

    Currently CP filters are not applied (fumanchu's first suggestion), but is this the best way.

    Currently the error pages can only be customized by using replacing the built in HTTPError class. I think it is worthwile to add a simple lookup such as cherrypy.config.get('error.404') to the built in error handler. This would not be as powerfull as a custom HTTPError, but it would be much simpler for most users.

    Thankfully these are all trivial changes.

  3. Robert Brewer

    Currently CP filters are not applied

    Looks to me like they are ;) Just a couple of data points: there are two builtin filters that currently use errorResponse filters, sessionfilter and xmlrpcfilter. The sessionfilter uses it to clean up session data, but it seems to me that should happen when the HTTPError page is finalized anyway. The xmlrpcfilter uses it to send an xmlrpclib.Fault, which... completely overrides the work of HTTPError (I think?). If someone could explain this, that would be good.

  4. Anonymous

    When an error happens during:

    cherrypy.response.body = [xmlrpclib.dumps(
                (cherrypy.response.body[0],),
                methodresponse=1,
                encoding=encoding,
                allow_none=1)]
    

    xmlrpclib raises Fault

    Then in beforeErrorResponse, the filter does format the error message which had been stored in cherrypy.response.body using the Fault() object and then the dumps() function format it into an XML-RPC response which can be sent onto the wire to the client.

    It could easily be changed into something like:

    def beforeFinalize(self):
            """ Called before finalizing output """
            if (not cherrypy.threadData.xmlRpcFilterOn
                or not cherrypy.request.isRPC):
                return
    
            encoding = cherrypy.config.get('xmlRpcFilter.encoding', 'utf-8')
    
            if not isinstance(cherrypy.response.body, list):
                cherrypy.response.body = [cherrypy.response.body]
            
            cherrypy.response.headerMap['Content-Type'] = 'text/xml'
            
            try:
                cherrypy.response.body = [xmlrpclib.dumps(
                    (cherrypy.response.body[0],),
                    methodresponse=1,
                    encoding=encoding,
                    allow_none=1)]
            except:
                body = ''.join([chunk for chunk in cherrypy.response.body])
                cherrypy.response.body = [xmlrpclib.dumps(xmlrpclib.Fault(1, body))]
                # do whatever with HTTPError
            
            cherrypy.response.headerMap['Content-Length'] = `len(cherrypy.response.body[0])`
    

    But is important to keep the body has being an XML-RPC message so that it can be understood by the client.

  5. Log in to comment