Added websocket client handler, and websocket-client module

[antoinetran]

#31

Tested, seems to work fine in both websockets and websocket!

Will try to add doc later.

[antoinetran]

Oups, I just saw that exception handling is not clean. Will look into it later.

[orweis]

Hi @antoinetran , from a quick skim looks like a great direction.
Will try and review early next week

[antoinetran]

Ok:

• doc added
• exception handled per Websocket Client Handler (exceptions during read and during connect)
• comments added
• both handlers tested with and without proxy

[antoinetran]

@orweis I think I am done! Please give me a feedback when you can, and hopefully release a tag if possible :)

[orweis]

Hi @antoinetran - this is greta - kudos!
I did an initial review - and there's some good stuff to start here

[roekatz]

Other than the double logging of exceptions in connect - Looks great to me :)

[antoinetran]

Hello! Please wait for me to do final tests. I found some minor bugs.

[antoinetran]

Ok now I tested in my small app, both in server and client code (before I only tested on client). I fixed an error on server-side (endpoint) in WebSocketSimplifier, because it implements SimpleWebSocket too, It works fine! Normally this is it.

Can you accept the test workflow here? https://github.com/permitio/fastapi_websocket_rpc/actions/runs/7400381788 There was an error because it was missing an os import (fixed).

Then, I don't know what is the merge process. Do you need me to squash the history, or do you merge as-it?

[antoinetran]

@orweis I have the impression https://github.com/permitio/fastapi_websocket_rpc/actions/runs/7400381788/job/20134732113#step:6:80 failed because of incorrect rootdir:

Then, the ModuleNotFoundError: No module named 'fastapi_websocket_rpc.rpc_methods'error would disappear, because it was already in fastapi_websocket_rpc directory. But I don't know what is causing the wrong rootdir. Looking into it...

[orweis]

Maybe try to fix the import, try relative import with . Synatx , absolute with adding the parent folder to sys path

[antoinetran]

Maybe try to fix the import, try relative import with . Synatx , absolute with adding the parent folder to sys path

Ok I read lots of documentation. Can you try running https://github.com/permitio/fastapi_websocket_rpc/actions/runs/7401903856? It seems pytest version has updated, which caused a regression. I don't have experience with pytest, but I tried a few solution and this commit seems to fix it: antoinetran@91d8d80

[antoinetran]

Ok there were some issues regarding Unit Tests:

• somehow, the module cannot be found anymore, I think this is because of pytest update? I fixed it by writing to pytest.ini the pythonpath = ., and removed some PATH hack in the test files. This seems to work now.
• the unit test were running indefinitely (6 hours), nothing is shown. It took me a while to figure out, by adding verbose + capture stdout that one Unit Test implements the SimpleWebSocket too and thus was missing the new method.

Still working on it.

[orweis]

No worries, take your time. And thank you

[antoinetran]

@orweis I fixed the Unit Tests :) See https://github.com/antoinetran/fastapi_websocket_rpc/actions/runs/7409756440 , all green.

Can you run https://github.com/permitio/fastapi_websocket_rpc/actions/runs/7409769095? Thanks!

[orweis]

This looks good, @roekatz your final review here is requested.

Completed