Opened 2 years ago

Closed 2 years ago

#34173 closed Cleanup/optimization (fixed)

SessionMiddleware support 503 status code

Reported by: SessionIssue Owned by: Abhinav Yadav
Component: contrib.sessions Version: 4.1
Severity: Normal Keywords:
Cc: Florian Apolloner, Anssi Kääriäinen Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by SessionIssue)

Hi guys,

I have the following situation. In one of my applications I'm having an issue with returning the right status code.
For example I had this situation where I wanted to list 1000 results, this normally takes a couple of seconds, but during this request, my DB went offline or got stuck for some reason. Currently, this resulted in a 500 status code.
In the API client that interfaces with this code we want to return a 503 because of an external source that only retries tasks on specific status codes (like 503), The current SessionMiddleware hijacks the statuscode and makes it impossible to raise a Service Unavailable (503).

if response.status_code != 500:
                    try:
                        request.session.save()
                    except UpdateError:
                        raise SessionInterrupted(
                            "The request's session was deleted before the "
                            "request completed. The user may have logged "
                            "out in a concurrent request, for example."
                        )
                    response.set_cookie(
                        settings.SESSION_COOKIE_NAME,
                        request.session.session_key, max_age=max_age,
                        expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
                        path=settings.SESSION_COOKIE_PATH,
                        secure=settings.SESSION_COOKIE_SECURE or None,
                        httponly=settings.SESSION_COOKIE_HTTPONLY or None,
                        samesite=settings.SESSION_COOKIE_SAMESITE,
                    )

As my DB is offline, this results in a 400 error, as the session can't be saved. But this is incorrect, as the base request isn't a bad request.
I rewrote this small piece in a custom middleware that inherits the SessionMiddleware, but this is not a futureproof solution:

**if response.status_code not in [500, 503]:**
                    try:
                        request.session.save()
                    except UpdateError:
                        raise SessionInterrupted(
                            "The request's session was deleted before the "
                            "request completed. The user may have logged "
                            "out in a concurrent request, for example."
                        )
                    response.set_cookie(
                        settings.SESSION_COOKIE_NAME,
                        request.session.session_key, max_age=max_age,
                        expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
                        path=settings.SESSION_COOKIE_PATH,
                        secure=settings.SESSION_COOKIE_SECURE or None,
                        httponly=settings.SESSION_COOKIE_HTTPONLY or None,
                        samesite=settings.SESSION_COOKIE_SAMESITE,
                    )


It's a small change, but it will make it hard for us to keep track of all the Django updates.

Do you have a solution for this issue?

Thanks in advance.

Change History (11)

comment:1 by Mariusz Felisiak, 2 years ago

Resolution: invalid
Status: newclosed

Thanks for this ticket, however it is a support question and Trac is not a support channel.

comment:2 by SessionIssue, 2 years ago

Description: modified (diff)
Resolution: invalid
Status: closednew
Summary: SessionMiddleware only returns 400 or 500 error in case of DB issues.SessionMiddleware support 503 status code
Type: BugNew feature

comment:3 by Mariusz Felisiak, 2 years ago

Cc: Florian Apolloner Anssi Kääriäinen added

I'm not sure why we should treat 503 differently. IMO, we could reconsider avoiding session saving when status code > 500 🤔

PS. Do not reopen tickets without further explanation.

in reply to:  3 comment:4 by SessionIssue, 2 years ago

Replying to Mariusz Felisiak:

I'm not sure why we should treat 503 differently. IMO, we could reconsider avoiding session saving when status code > 500 🤔

PS. Do not reopen tickets without further explanation.

That would fix the problem and seems like a convenient solution to me.
Thanks a lot! 😁

Version 0, edited 2 years ago by SessionIssue (next)

comment:5 by Carlton Gibson, 2 years ago

Triage Stage: UnreviewedAccepted
Type: New featureCleanup/optimization

Happy to look at a patch (with tests) adjusting the check to if response.status_code >= 500:

comment:6 by Carlton Gibson, 2 years ago

Easy pickings: set

comment:7 by Abhinav Yadav, 2 years ago

Owner: changed from nobody to Abhinav Yadav
Status: newassigned

in reply to:  5 comment:8 by SessionIssue, 2 years ago

Replying to Carlton Gibson:

Happy to look at a patch (with tests) adjusting the check to if response.status_code >= 500:

Is this a patch we can expect to see in 3.2 (LTS), or will this only be patched in the upcoming 4.2 LTS?
Thanks in advance.

comment:9 by Abhinav Yadav, 2 years ago

Has patch: set

comment:10 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 17472c3:

Fixed #34173 -- Skipped saving sessions on 5xx responses.

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