Skip to content

Commit

Permalink
Cherry pick fix for request close on retry from 98fbc88.
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Howitz committed Dec 10, 2018
1 parent f1e5fc4 commit a50f1a9
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 12 deletions.
7 changes: 7 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
For the change log of the alpha versions see
https://github.com/zopefoundation/Zope/blob/4.0a6/CHANGES.rst

4.0b7.post1 (unreleased)
------------------------

- Fix closing newly created request before processing it after a retryable
error has occurred.
(`#413 <https://github.com/zopefoundation/Zope/issues/413>`_)

4.0b7 (2018-10-30)
------------------

Expand Down
30 changes: 18 additions & 12 deletions src/ZPublisher/WSGIPublisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,23 @@ def publish_module(environ, start_response,

environ['PATH_INFO'] = path_info
with closing(BytesIO()) as stdout, closing(BytesIO()) as stderr:
response = (_response if _response is not None else
_response_factory(stdout=stdout, stderr=stderr))
response._http_version = environ['SERVER_PROTOCOL'].split('/')[1]
response._server_version = environ.get('SERVER_SOFTWARE')

request = (_request if _request is not None else
_request_factory(environ['wsgi.input'], environ, response))

for i in range(getattr(request, 'retry_max_count', 3) + 1):
new_response = (
_response
if _response is not None
else _response_factory(stdout=stdout, stderr=stderr))
new_response._http_version = environ['SERVER_PROTOCOL'].split('/')[1]
new_response._server_version = environ.get('SERVER_SOFTWARE')

new_request = (
_request
if _request is not None
else _request_factory(environ['wsgi.input'],
environ,
new_response))

for i in range(getattr(new_request, 'retry_max_count', 3) + 1):
request = new_request
response = new_response
setRequest(request)
try:
with load_app(module_info) as new_mod_info:
Expand All @@ -270,9 +278,7 @@ def publish_module(environ, start_response,
except (ConflictError, TransientError) as exc:
if request.supports_retry():
new_request = request.retry()
request.close()
request = new_request
response = new_request.response
new_response = new_request.response
else:
raise
finally:
Expand Down
37 changes: 37 additions & 0 deletions src/ZPublisher/tests/test_WSGIPublisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import unittest

import transaction
from ZODB.POSException import ConflictError
from zope.interface.common.interfaces import IException
from zope.publisher.interfaces import INotFound
from zope.security.interfaces import IUnauthorized
Expand Down Expand Up @@ -448,6 +449,27 @@ def _request_factory(stdin, environ, response):
_request_factory=_request_factory)
self.assertTrue(_request._closed)

def test_handle_ConflictError(self):
environ = self._makeEnviron()
start_response = DummyCallable()
def _publish(request, module_info):
if request.retry_count < 1:
raise ConflictError
response = DummyResponse()
response.setBody(request.other.get('method'))
return response

try:
from ZPublisher.HTTPRequest import HTTPRequest
original_retry_max_count = HTTPRequest.retry_max_count
HTTPRequest.retry_max_count = 1
# After the retry the request has a filled `other` dict, thus the
# new request is not closed before processing it:
self.assertEqual(
self._callFUT(environ, start_response, _publish), (b'', 'GET'))
finally:
HTTPRequest.retry_max_count = original_retry_max_count

def testCustomExceptionViewUnauthorized(self):
from AccessControl import Unauthorized
registerExceptionView(IUnauthorized)
Expand Down Expand Up @@ -507,6 +529,7 @@ def testCustomExceptionViewBadRequest(self):
body = b''.join(app_iter)
self.assertEqual(start_response._called_with[0][0], '400 Bad Request')
self.assertTrue(b'Exception View: BadRequest' in body)
unregisterExceptionView(IException)

def testCustomExceptionViewInternalError(self):
from zExceptions import InternalError
Expand All @@ -520,6 +543,7 @@ def testCustomExceptionViewInternalError(self):
self.assertEqual(
start_response._called_with[0][0], '500 Internal Server Error')
self.assertTrue(b'Exception View: InternalError' in body)
unregisterExceptionView(IException)

def testRedirectExceptionView(self):
from zExceptions import Redirect
Expand All @@ -535,6 +559,7 @@ def testRedirectExceptionView(self):
self.assertTrue(b'Exception View: Redirect' in body)
headers = dict(headers)
self.assertEqual(headers['Location'], 'http://localhost:9/')
unregisterExceptionView(IException)

def testHandleErrorsFalseBypassesExceptionResponse(self):
from AccessControl import Unauthorized
Expand Down Expand Up @@ -648,6 +673,18 @@ def registerExceptionView(for_):
name=u'index.html',
)

def unregisterExceptionView(for_):
from zope.interface import Interface
from zope.component import getGlobalSiteManager
from zope.publisher.interfaces.browser import IDefaultBrowserLayer
gsm = getGlobalSiteManager()
gsm.unregisterAdapter(
CustomExceptionView,
required=(for_, IDefaultBrowserLayer),
provided=Interface,
name=u'index.html',
)


class DummyRequest(dict):
_processedInputs = False
Expand Down

0 comments on commit a50f1a9

Please sign in to comment.