close session if HTTP response is not 200 or event is auth_revoked #10
Conversation
aiosseclient.py
Outdated
| yield Event.parse(''.join(lines)) | ||
| current_event = Event.parse(''.join(lines)) | ||
| yield current_event | ||
| if current_event.event == "auth_revoked": |
There was a problem hiding this comment.
First of all thanks for the filing the issue and the patch, doesn't comparing with "auth_revoked" feels to be too specific? Maybe it's something that a client should do or maybe not. I'll apply the patch upon you response as I trust you given your expertise on the field. Thanks 😊
aiosseclient.py
Outdated
| @@ -20,6 +20,9 @@ async def aiosseclient(url, last_id=None, **kwargs): | |||
| sock_connect=None, sock_read=None) | |||
| async with aiohttp.ClientSession(timeout=timeout) as session: | |||
| response = await session.get(url, **kwargs) | |||
| if response.status != 200: | |||
There was a problem hiding this comment.
Maybe something other than 200 also can be OK? Perhaps all the statuses starting 2 and maybe handling 3 also?
|
I wouldn't accept the PR as is. Based on the specification: https://html.spec.whatwg.org/multipage/server-sent-events.html,
What I do know is that a 401 should result in the session closing (and probably true for 4XX, 5XX and possibly others).
I'll have to think about how this could be modularized so that it would be easier to handle these cases for specific SSE implementations. For now I wanted to document that I had found a way to make this work for my case. |
|
Fantastic, thank you very much. I've invited you to the repository as a collaborator. You can merge this PR whenever you like or let me merge, or just leave it open as I understand the situation. Thanks 😊 |
parameterize valid http response codes and exit worthy events. This should not require existing users to change their code
|
I updated this to be parameterized with sensible defaults. Let me know what you think |
|
Just made a release containing the changes https://pypi.org/manage/project/aiosseclient/releases/ using GitHub Actions CI, feel free to test and let me know if anything missing. It's even fine if you want to introduce a breaking API change to the just added API so feel free to test and tweak whatever you like or not. Thanks 😊 |
|
Also thanks to your work I've discovered https://github.com/rtfol/aiohttp-sse-client and https://github.com/JelleZijlstra/aiohttp-sse-client2 which started with inspiration from this project and I believe we can learn from their experiences now. |
These changes make it possible to respond to auth failures and auth expirations