Opened 5 years ago

Closed 5 years ago

#14588 closed (fixed)

Changed behaviour of middlewares after [14393]

Reported by: Petr Marhoun <petr.marhoun@…> Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords:
Cc: kmike Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Documentation says:

Unlike the process_request() and process_view() methods, the process_response() method is always called, even if the process_request() and process_view() methods of the same middleware class were skipped because an earlier middleware method returned an HttpResponse.

But after [14393] response middleware are called only if there is no response from process_request or process_view or process_exception or Http404 or PermissionDenied.

It is problematic with transaction middleware - it is necessary to call process_response or process_exception to close open transaction. But if process_response returns response, transaction is still opened and stacked. And if there is something outside transaction in the following request in the same thread, it is not committed - for example session middleware could be outside, it would be impossible to login.

I also found one bug in my code - if process_exception returns response, following exception middlewares are skipped and transaction could be closed in ThreadMiddleware.process_response. So there was commit, not rollback (before [14393]). It was logical consequence of the documentation, but maybe there could be a note about it.

Change History (3)

comment:1 Changed 5 years ago by kmike

  • Cc kmike added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Yes, this is a change in behavior that wasn't picked up by our test suite. I've got a fix in progress, along with a bunch of tests to validate the intended behavior of the request/response cycle.

comment:3 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [14398]) Fixed #14588 -- Corrected r14393 to ensure that response middlewares are are always applied. Includes a battery of tests to validate the expected behavior of the middleware cycle. Thanks to Petr Marhoun for the report.

Note: See TracTickets for help on using tickets.
Back to Top