Download Install Tutorial Docs FAQ Tools WikiLicense Team IRC Planet Involvement Shop Book

Ticket #288 (defect)

Opened 3 years ago

Last modified 3 years ago

HTTP Error Handling simplifications

Status: closed (fixed)

Reported by: mikerobi Assigned to: mikerobi
Priority: normal Milestone: 2.1-rc1
Component: CherryPy code Keywords:
Cc:

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

Change History

09/08/05 13:46:04: Modified by mikerobi

  • status changed from new to assigned.

09/13/05 17:51:30: Modified by mikerobi

  • status changed from assigned to closed.
  • resolution set to fixed.

documentation coming soon.

09/13/05 18:41:41: Modified by fumanchu

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.

09/13/05 23:45:45: Modified by mikerobi

  • status changed from closed to reopened.
  • resolution deleted.

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.

09/14/05 02:33:07: Modified by fumanchu

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.

09/14/05 03:19:49: Modified by lawouach

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.

09/14/05 16:36:57: Modified by mikerobi

  • status changed from reopened to closed.
  • resolution set to fixed.

Hopefully this time it is fixed for good :)

Hosted by WebFaction

Log in as guest/cpguest to create tickets