Hold on to your butts!

This is a pretty significant restructuring of ZPublisher to
allow a new module hook for handling errors.  There are two goals
in this:

  - To allow Zope to retry a request when ZODB raises a ConflictError, and

  - To move reformatting of exception using standard_error_message
    out to the publisher level so that:

    - Errors don't get obfuscated by error handling done by
      sub-documents and external methods, and

    - Errors raised by the publisher (e.g. for input errors,
      not-found errors, system exit, etc) get prettified.

lib/python/ZPublisher/BaseRequest.py

@@ -82,7 +82,7 @@
 # attributions are listed in the accompanying credits file.
 # 
 ##############################################################################
-__version__='$Revision: 1.12 $'[11:-2]
+__version__='$Revision: 1.13 $'[11:-2]
 
 from string import join, split, find, rfind, lower, upper
 from urllib import quote
@@ -246,8 +246,10 @@ class BaseRequest:
     
         parents=request['PARENTS']
         object=parents[-1]
-        del parents[-1]
-    
+        del parents[:]
+        try:
+            # We build parents in the wrong order, so we
+            # need to make sure we reverse it when we're doe.
             if hasattr(object,'__roles__'): roles=object.__roles__
             else:                           roles=UNSPECIFIED_ROLES
         
@@ -278,7 +280,7 @@ class BaseRequest:
                 # Try to bind the top-level object to the request
                 object=object.__of__(RequestContainer(REQUEST=request))
         
-        steps=[]
+            steps=self.steps
             path.reverse()
             while path:
                 entry_name=path[-1]
@@ -303,13 +305,17 @@ class BaseRequest:
                             object, subobject = subobject
                     else:
                         try:
-                        # Note - this is necessary to support things like DAV.
-                        # We have to make sure that the target object is not
-                        # acquired if the request_method is other than GET or
-                        # POST. Otherwise, you could never use PUT to add a new
-                        # object named 'test' if an object 'test' existed above
-                        # it in the heirarchy -- you'd always get the existing
-                        # object :(
+                            
+                            # Note - this is necessary to support
+                            # things like DAV.  We have to make sure
+                            # that the target object is not acquired
+                            # if the request_method is other than GET
+                            # or POST. Otherwise, you could never use
+                            # PUT to add a new object named 'test' if
+                            # an object 'test' existed above it in the
+                            # heirarchy -- you'd always get the
+                            # existing object :(
+                            
                             if baseflag and hasattr(object, 'aq_base'):
                                 if hasattr(object.aq_base, entry_name):
                                     subobject=getattr(object, entry_name)
@@ -331,8 +337,8 @@ class BaseRequest:
                         if not doc: raise AttributeError, entry_name
                     except:
                         if debug_mode:
-                        return response.debugError("Missing doc string at: %s"
-                                                   % URL)
+                            return response.debugError(
+                                "Missing doc string at: %s" % URL)
                         else: return response.notFoundError("%s" % (URL))
     
                     if hasattr(subobject,'__roles__'):
@@ -366,15 +372,8 @@ class BaseRequest:
                                 i=rfind(URL,'/')
                                 if i > 0: response.setBase(URL[:i])
         
-    
-        # THIS LOOKS WRONG!
-#        if entry_name != method and method != 'index_html':
-#            if debug_mode:
-#                response.debugError("Method %s not found at: %s"
-#                                    % (method,URL))
-#            else: response.notFoundError(method)
-    
-        request.steps=steps
+        finally:
+            # We need to MAKE SURE this happens due to new error handling
             parents.reverse()
     
         # Do authorization checks
@@ -448,6 +447,16 @@ class BaseRequest:
     
         return object
 
+    retry_count=0
+    def supports_retry(self): return 0
+
+    _held=()
+    def _hold(self, object):
+        """Hold a reference to an object to delay it's destruction until mine
+        """
+        self._held=self._held+(object,)
+
+    
 def old_validation(groups, request, auth,
                    roles=UNSPECIFIED_ROLES):
 

lib/python/ZPublisher/BaseResponse.py

@@ -84,8 +84,8 @@
 ##############################################################################
 '''CGI Response Output formatter
 
-$Id: BaseResponse.py,v 1.3 1999/03/18 22:34:53 jim Exp $'''
-__version__='$Revision: 1.3 $'[11:-2]
+$Id: BaseResponse.py,v 1.4 1999/08/04 18:05:26 jim Exp $'''
+__version__='$Revision: 1.4 $'[11:-2]
 
 import string, types, sys, regex
 from string import find, rfind, lower, upper, strip, split, join, translate
@@ -98,6 +98,7 @@ class BaseResponse:
     """
     debug_mode=None
     _auth=None
+    _error_format='text/plain'
 
     def __init__(self, stdout, stderr,
                  body='', headers=None, status=None, cookies=None):

lib/python/ZPublisher/HTTPRequest.py

@@ -83,7 +83,7 @@
 # 
 ##############################################################################
 
-__version__='$Revision: 1.19 $'[11:-2]
+__version__='$Revision: 1.20 $'[11:-2]
 
 import regex, sys, os, string
 from string import lower, atoi, rfind, split, strip, join, upper, find
@@ -170,8 +170,19 @@ class HTTPRequest(BaseRequest):
     _hacked_path=None
     args=()
 
-    def __init__(self, stdin, environ, response, clean=0):
+    retry_max_count=3
+    def supports_retry(self): return self.retry_count < self.retry_max_coun
+
+    def retry(self):
+        r=self.__class__(stdin=self.stdin,
+                         environ=self._orig_env,
+                         response=self.response.retry()
+                         )
+        r._held=self._held
+        return r
 
+    def __init__(self, stdin, environ, response, clean=0):
+        self._orig_env=environ
         # Avoid the overhead of scrubbing the environment in the
         # case of request cloning for traversal purposes. If the
         # clean flag is set, we know we can use the passed in
@@ -180,8 +191,64 @@ class HTTPRequest(BaseRequest):
         
         self.stdin=stdin
         self.environ=environ
+        have_env=environ.has_key
+        get_env=environ.get
         self.response=response
-        other=self.other={}
+        other=self.other={'RESPONSE': response}
+        self.form={}
+        self.steps=[]
+
+        ################################################################
+        # Get base info first. This isn't likely to cause
+        # errors and might be useful to error handlers.
+        b=script=strip(get_env('SCRIPT_NAME',''))
+        while b and b[-1]=='/': b=b[:-1]
+        p = rfind(b,'/')
+        if p >= 0: b=b[:p+1]
+        else: b=''
+        while b and b[0]=='/': b=b[1:]
+
+        server_url=get_env('SERVER_URL',None)
+        if server_url is not None:
+             server_url=strip(server_url)
+        else:
+             if have_env('HTTPS') and (
+                 environ['HTTPS'] == "on" or environ['HTTPS'] == "ON"):
+                 server_url='https://'
+             elif (have_env('SERVER_PORT_SECURE') and 
+                   environ['SERVER_PORT_SECURE'] == "1"):
+                 server_url='https://'
+             else: server_url='http://'
+
+             if have_env('HTTP_HOST'):
+                 server_url=server_url+strip(environ['HTTP_HOST'])
+             else:
+                 server_url=server_url+strip(environ['SERVER_NAME'])
+                 server_port=environ['SERVER_PORT']
+                 if server_port!='80': server_url=server_url+':'+server_port
+             other['SERVER_URL']=server_url
+             
+        if server_url[-1:]=='/': server_url=server_url[:-1]
+                        
+        if b: self.base="%s/%s" % (server_url,b)
+        else: self.base=server_url
+        while script[:1]=='/': script=script[1:]
+        if script: script="%s/%s" % (server_url,script)
+        else:      script=server_url
+        other['URL']=self.script=script
+
+        ################################################################
+        # Cookie values should *not* be appended to existing form
+        # vars with the same name - they are more like default values
+        # for names not otherwise specified in the form.
+        cookies={}
+        k=get_env('HTTP_COOKIE','')
+        if k:
+            parse_cookie(k, cookies)
+            for k,item in cookies.items():
+                if not other.has_key(k):
+                    other[k]=item
+        self.cookies=cookies
     
     def processInputs(
         self,
@@ -201,8 +268,8 @@ class HTTPRequest(BaseRequest):
         ):
         """Process request inputs
 
-        We need to delay input parsing so that it is done under publisher control for
-        error handling prposes.
+        We need to delay input parsing so that it is done under
+        publisher control for error handling purposes.
         """
         response=self.response
         environ=self.environ
@@ -216,8 +283,9 @@ class HTTPRequest(BaseRequest):
             response._auth=1
             del environ['HTTP_AUTHORIZATION']
 
-        form={}
+        form=self.form
         other=self.other
+
         meth=None
         fs=FieldStorage(fp=fp,environ=environ,keep_blank_values=1)
         if not hasattr(fs,'list') or fs.list is None:
@@ -229,7 +297,8 @@ class HTTPRequest(BaseRequest):
                 global xmlrpc
                 if xmlrpc is None: import xmlrpc
                 meth, self.args = xmlrpc.parse_input(fs.value)
-                response=self.response=xmlrpc.response(response)
+                response=xmlrpc.response(response)
+                other['RESPONSE']=self.response=response
                 other['REQUEST_METHOD']='' # We don't want index_html!
             else:
                 self._file=fs.file
@@ -461,12 +530,20 @@ class HTTPRequest(BaseRequest):
                                    if getattr(x, '__class__',0) is record:
                                        # if the x is a record
                                        for k, v in x.__dict__.items():
-                                           # loop through each attribute and value in
+                                           
+                                           # loop through each
+                                           # attribute and value in
                                            # the record
+                                           
                                            for y in l:
-                                               # loop through each record in the form
-                                               # list if it doesn't have the attributes
-                                               # in the default dictionary, set them
+                                               
+                                               # loop through each
+                                               # record in the form
+                                               # list if it doesn't
+                                               # have the attributes
+                                               # in the default
+                                               # dictionary, set them
+                                               
                                                if not hasattr(y, k):
                                                    setattr(y, k, v)
                                    else:
@@ -529,57 +606,6 @@ class HTTPRequest(BaseRequest):
             other['PATH_INFO']=path="%s/%s" % (path,meth)
             self._hacked_path=1
 
-        # Cookie values should *not* be appended to existing form
-        # vars with the same name - they are more like default values
-        # for names not otherwise specified in the form.
-        cookies={}
-        if environ.has_key('HTTP_COOKIE'):
-            parse_cookie(environ['HTTP_COOKIE'],cookies)
-            for k,item in cookies.items():
-                if not other.has_key(k):
-                    other[k]=item
-
-        self.form=form
-        self.cookies=cookies
-        other['RESPONSE']=response
-
-        have_env=environ.has_key
-
-        b=script=strip(environ['SCRIPT_NAME'])
-        while b and b[-1]=='/': b=b[:-1]
-        p = rfind(b,'/')
-        if p >= 0: b=b[:p+1]
-        else: b=''
-        while b and b[0]=='/': b=b[1:]
-        
-        if have_env('SERVER_URL'):
-             server_url=strip(environ['SERVER_URL'])
-        else:
-             if have_env('HTTPS') and (
-                 environ['HTTPS'] == "on" or environ['HTTPS'] == "ON"):
-                 server_url='https://'
-             elif (have_env('SERVER_PORT_SECURE') and 
-                   environ['SERVER_PORT_SECURE'] == "1"):
-                 server_url='https://'
-             else: server_url='http://'
-
-             if have_env('HTTP_HOST'):
-                 server_url=server_url+strip(environ['HTTP_HOST'])
-             else:
-                 server_url=server_url+strip(environ['SERVER_NAME'])
-                 server_port=environ['SERVER_PORT']
-                 if server_port!='80': server_url=server_url+':'+server_port
-             other['SERVER_URL']=server_url
-             
-        if server_url[-1:]=='/': server_url=server_url[:-1]
-                        
-        if b: self.base="%s/%s" % (server_url,b)
-        else: self.base=server_url
-        while script[:1]=='/': script=script[1:]
-        if script: script="%s/%s" % (server_url,script)
-        else:      script=server_url
-        other['URL']=self.script=script
-
     def resolve_url(self, url):
         # Attempt to resolve a url into an object in the Zope
         # namespace. The url must be a fully-qualified url. The

lib/python/ZPublisher/HTTPResponse.py

@@ -84,8 +84,8 @@
 ##############################################################################
 '''CGI Response Output formatter
 
-$Id: HTTPResponse.py,v 1.16 1999/06/30 14:02:15 jim Exp $'''
-__version__='$Revision: 1.16 $'[11:-2]
+$Id: HTTPResponse.py,v 1.17 1999/08/04 18:05:27 jim Exp $'''
+__version__='$Revision: 1.17 $'[11:-2]
 
 import string, types, sys, regex
 from string import find, rfind, lower, upper, strip, split, join, translate
@@ -186,6 +186,7 @@ class HTTPResponse(BaseResponse):
     accumulated_headers=''
     body=''
     realm='Zope'
+    _error_format='text/html'
 
     def __init__(self,body='',status=200,headers=None,
                  stdout=sys.stdout, stderr=sys.stderr,):
@@ -208,6 +209,16 @@ class HTTPResponse(BaseResponse):
         self.stdout=stdout
         self.stderr=stderr
 
+    def retry(self):
+        """Return a response object to be used in a retry attempt
+        """
+        
+        # This implementation is a bit lame, because it assumes that
+        # only stdout stderr were passed to the constructor. OTOH, I
+        # think that that's all that is ever passed.
+        
+        return self.__class__(stdout=self.stdout, stderr=self.stderr)
+    
     def setStatus(self, status, reason=None):
         '''\
         Sets the HTTP status code of the response; the argument may

lib/python/ZPublisher/Publish.py

@@ -84,8 +84,8 @@
 ##############################################################################
 __doc__="""Python Object Publisher -- Publish Python objects on web servers
 
-$Id: Publish.py,v 1.134 1999/08/04 12:01:20 jim Exp $"""
-__version__='$Revision: 1.134 $'[11:-2]
+$Id: Publish.py,v 1.135 1999/08/04 18:05:28 jim Exp $"""
+__version__='$Revision: 1.135 $'[11:-2]
 
 import sys, os
 from string import lower, atoi, rfind, strip
@@ -94,6 +94,10 @@ from Request import Request
 from maybe_lock import allocate_lock
 from mapply import mapply
 
+class Retry(Exception):
+    """Raise this to retry a request
+    """
+
 def call_object(object, args, request):
     result=apply(object,args) # Type s<cr> to step into published object.
     return result
@@ -113,6 +117,14 @@ def publish(request, module_name, after_list, debug=0,
             mapply=mapply,
             ):
 
+    (bobo_before, bobo_after, object, realm, debug_mode, err_hook,
+     have_transactions)= get_module_info(module_name)
+
+    parents=None
+
+    try:
+        request.processInputs()
+
         request_get=request.get
         response=request.response
     
@@ -122,9 +134,6 @@ def publish(request, module_name, after_list, debug=0,
             cancel=request_get('CANCEL_ACTION','')
             if cancel: raise 'Redirect', cancel
     
-    bobo_before, bobo_after, object, realm, debug_mode = get_module_info(
-        module_name)
-
         after_list[0]=bobo_after
         if debug_mode: response.debug_mode=debug_mode
         if realm and not request.get('REMOTE_USER',None):
@@ -137,15 +146,12 @@ def publish(request, module_name, after_list, debug=0,
     
         request['PARENTS']=parents=[object]
         
-    # Attempt to start a transaction:
-    try: transaction=get_transaction()
-    except: transaction=None
-    if transaction is not None: transaction.begin()
+        if have_transactions: get_transaction().begin()
     
         object=request.traverse(path)
     
         # Record transaction meta-data
-    if transaction is not None:
+        if have_transactions:
             get_transaction().note(request_get('PATH_INFO'))
             auth_user=request_get('AUTHENTICATED_USER',None)
             if auth_user is not None:
@@ -160,10 +166,77 @@ def publish(request, module_name, after_list, debug=0,
     
         if result is not response: response.setBody(result)
     
-    if transaction: get_transaction().commit()
+        if have_transactions: get_transaction().commit()
 
         return response
 
+    except:
+        if err_hook is not None:
+            if parents: parents=parents[-1]
+            try:
+                return err_hook(parents, request,
+                                sys.exc_info()[0],
+                                sys.exc_info()[1],
+                                sys.exc_info()[2],
+                                )
+            except Retry:
+                # We need to try again....
+                if not request.supports_retry():
+                    return err_hook(parents, request,
+                                    sys.exc_info()[0],
+                                    sys.exc_info()[1],
+                                    sys.exc_info()[2],
+                                    )
+                return publish(request.retry(), module_name, after_list, debug)
+        else: raise
+            
+
+def publish_module(module_name,
+                   stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr,
+                   environ=os.environ, debug=0, request=None, response=None):
+    must_die=0
+    status=200
+    after_list=[None]
+    try:
+        try:
+            if response is None:
+                response=Response(stdout=stdout, stderr=stderr)
+            else:
+                stdout=response.stdout
+
+            if request is None:
+                request=Request(stdin, environ, response)
+
+            response = publish(request, module_name, after_list, debug=debug)
+        except SystemExit, v:
+            if hasattr(sys, 'exc_info'): must_die=sys.exc_info()
+            else: must_die = SystemExit, v, sys.exc_traceback
+            response.exception(must_die)
+        except ImportError, v:
+            if type(v) is type(()) and len(v)==3: must_die=v
+            elif hasattr(sys, 'exc_info'): must_die=sys.exc_info()
+            else: must_die = SystemExit, v, sys.exc_traceback
+            response.exception(1, v)
+        except:
+            response.exception()
+            status=response.getStatus()
+
+        if response:
+            response=str(response)
+            if response:
+                stdout.write(response)
+
+        # The module defined a post-access function, call it
+        if after_list[0] is not None: after_list[0]()
+
+    finally:
+        if request is not None: request.other={}
+
+    if must_die: raise must_die[0], must_die[1], must_die[2]
+    sys.exc_type, sys.exc_value, sys.exc_traceback = None, None, None
+    return status
+
+
 _l=allocate_lock()
 def get_module_info(module_name, modules={},
                     acquire=_l.acquire,
@@ -220,7 +293,14 @@ def get_module_info(module_name, modules={},
             object=module.web_objects
         else: object=module
 
-        info= (bobo_before, bobo_after, object, realm, debug_mode)
+        error_hook=getattr(module,'zpublisher_exception_hook', None)
+
+        try: get_transaction()
+        except: have_transactions=0
+        else: have_transactions=1
+    
+        info= (bobo_before, bobo_after, object, realm, debug_mode,
+               error_hook, have_transactions)
     
         modules[module_name]=modules[module_name+'.cgi']=info
 
@@ -232,47 +312,3 @@ def get_module_info(module_name, modules={},
     finally:
         tb=None
         release()
-
-def publish_module(module_name,
-                   stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr,
-                   environ=os.environ, debug=0, request=None, response=None):
-    must_die=0
-    status=200
-    after_list=[None]
-    try:
-        try:
-            if response is None:
-                response=Response(stdout=stdout, stderr=stderr)
-            else:
-                stdout=response.stdout
-            if request is None:
-                request=Request(stdin, environ, response)
-            request.processInputs()
-            response=request.response # could have changed!
-            response = publish(request, module_name, after_list, debug=debug)
-        except SystemExit, v:
-            if hasattr(sys, 'exc_info'): must_die=sys.exc_info()
-            else: must_die = SystemExit, v, sys.exc_traceback
-            response.exception(must_die)
-        except ImportError, v:
-            if type(v) is type(()) and len(v)==3: must_die=v
-            elif hasattr(sys, 'exc_info'): must_die=sys.exc_info()
-            else: must_die = SystemExit, v, sys.exc_traceback
-            response.exception(1, v)
-        except:
-            response.exception()
-            status=response.getStatus()
-        if response:
-            response=str(response)
-        if response: stdout.write(response)
-
-        # The module defined a post-access function, call it
-        if after_list[0] is not None: after_list[0]()
-
-    finally:
-        if request is not None: request.other={}
-
-    if must_die: raise must_die[0], must_die[1], must_die[2]
-    sys.exc_type, sys.exc_value, sys.exc_traceback = None, None, None
-    return status
-

lib/python/ZPublisher/Test.py

@@ -162,9 +162,9 @@ Examples
             s
 
 
-$Id: Test.py,v 1.32 1999/08/04 12:01:20 jim Exp $
+$Id: Test.py,v 1.33 1999/08/04 18:05:29 jim Exp $
 '''
-__version__='$Revision: 1.32 $'[11:-2]
+__version__='$Revision: 1.33 $'[11:-2]
 
 import sys, traceback, profile, os, getopt, string
 from time import clock
@@ -256,8 +256,6 @@ def publish_module(module_name,
                 stdout=response.stdout
             if request is None:
                 request=Request(stdin, environ, response)
-            request.processInputs()
-            response=request.response # could have changed!
             for k, v in extra.items(): request[k]=v
             response = publish(request, module_name, after_list, debug=debug)
         except SystemExit, v:

lib/python/ZPublisher/__init__.py

@@ -82,9 +82,9 @@
 # attributions are listed in the accompanying credits file.
 # 
 ##############################################################################
-__version__='$Revision: 1.7 $'[11:-2]
+__version__='$Revision: 1.8 $'[11:-2]
 
-from Publish import publish_module
+from Publish import publish_module, Retry
 
 def test(*args, **kw):
     global test