aiohttp 4.0.0a1 compatibility issue

[jonathanslenders]

Hi there, first of all thanks for creating this library!

The newest aiohttp alpha has a change that expects redirects to be raised as exceptions.

raise web.HTTPFound(...)

This is not supported by the session middleware, because HTTPFound is not an instance of StreamResponse. It seems also more complex then adding a simple isinstance() check in __init__.py, because after doing this, the session is not created correctly at a 302 Found page. This is common when using oauth authentication though.

(The reason for using the latest aiohttp alpha is because I need the context variable support.)

Thanks!

[asvetlov]

Thanks for the report.
I'll try to find time for the fix soon but don't provide any estimation, sorry.

[jonathanslenders]

Thank you!

[asvetlov]

I'm wondering do you need context vars support or something also?
aiohttp 4.0 release is delayed, unfortunately.
4.0 is a chance to break backward compatibility; that's why I have a long list of awaiting changes.

Context vars support can be backported to 3.x line and released as a part of aiohttp 3.7. If it solves your needs I'll happy to backport. Seems like not only you need the feature.

[jonathanslenders]

Hi @asvetlov,

Yes, we do need context vars support, that's why we're using the 4.0 alpha.
I haven't read the implementation, but was wondering whether we could backport it with a simple middleware (run the handler in the middleware in copy_context().run(...)).

Anyway, I created my own session middleware, compatible with aiohttp 4.0 for now. This seems to work fine. In case anyone is interested, this is what we have:

import uuid
from contextvars import ContextVar
from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict

from aiohttp.web import Request, StreamResponse
from aiohttp.web_exceptions import HTTPMove

from our_database_backend import Database, SessionNotFoundError  # custom.


__all__ = ["SessionMiddleware", "session"]

# The session context variable that's populated by the `SessionMiddleware`.
session: ContextVar[Dict[str, Any]] = ContextVar("session", default={})

if TYPE_CHECKING:
    # Middleware type, see:
    # https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_app.py
    _Handler = Callable[[Request], Awaitable[StreamResponse]]


class SessionMiddleware:
    def __init__(self, database: Database, cookie_name="candela_session") -> None:
        self.database = database
        self.cookie_name = cookie_name

    async def __call__(self, request: Request, handler: "_Handler") -> StreamResponse:
        session_key = request.cookies.get(self.cookie_name)

        # Load session from database, if we have a session key.
        if session_key is None:
            new_session = True
        else:
            try:
                session_data = await self.database.load_session(session_key)
            except SessionNotFoundError:
                new_session = True
            else:
                new_session = False

        if new_session:
            session_key = str(uuid.uuid4())
            session_data = {}

        session.set(session_data)
        cookie_header = f"{self.cookie_name}={session_key}"

        try:
            response = await handler(request)

            # Add session cookie to response.
            # response.set_cookie(self._cookie_name, session_key)
            if new_session:
                response.headers["Set-Cookie"] = cookie_header

            return response
        except HTTPMove as move:
            # Add header to redirect response.
            move.headers["Set-Cookie"] = cookie_header
            raise
        finally:
            # Store new session in DB.
            await self.database.store_session(session_key, session.get())

Database in there refers to a custom storage back-end, whatever it is.

[jonathanslenders]

@asvetlov,

So, one thing I missed in aiohttp 4.0, was a set_cookie method for HTTPMove like we have with StreamResponse.

Thanks again for aiohttp, it's absolutely great.

[asvetlov]

My pleasure!

set_cookie for HTTPException (and descendant HTTPMove) is a brilliant idea.
The current aiohttp-session design has a spike: it's middleware converts HTTPExcetion into web.Response, it may be a source of confusion. Exception re-raising sounds better.
set_cookie (plus del_cookie) can solve this problem, will be no need to convert exception into response anymore.

[asvetlov]

aio-libs/aiohttp#4271 provides backport of contextvar related changes back to 3.x line.

[mistotebe]

As seen in #506 this is also broken with aiohttp 3.7.2

[Dreamsorcerer]

I think the isinstance() changes in #512 and the more recent addition of cookies on HTTPExceptions should fix this issue.

[hjacobs]

FYI: By using return instead of raise I could workaround this issue (aiohttp 3.7.3 and aiohttp-session 2.9.0): https://codeberg.org/hjacobs/kube-web-view/commit/507d9d632a6c837fd393d6c9e3ddeb5f9d7529ca

[Dreamsorcerer]

Right, though that behaviour is deprecated, so you should remember to switch it back once 3.8 is released.

[Dreamsorcerer]

I think this issue is fixed now (pending releases).