fix(WebSocket): handle `OSError` upon `send()` + fix `max_receive_queue == 0`

falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.

https://falcon.readthedocs.io/en/stable/

Apache License 2.0

9.53k stars 945 forks source link

fix(WebSocket): handle `OSError` upon `send()` + fix `max_receive_queue == 0` #2324

Closed vytas7 closed 2 months ago

vytas7 commented 2 months ago

[x] Revise and rewrite parts of Lost Connections.
[x] Add unit tests (with full coverage) for the max_receive_queue == 0 case.
[x] Revise the test client if it could better simulate both behaviours depending on the simulated spec version.
[x] Add Uvicorn tests in test_asgi_servers.py that exercise the max_receive_queue == 0 case, and catches an error from the webserver's OSError upon send().
[x] Change E2E tests not to use buffering (since we know that use Uvicorn there).

Closes #2292.

vytas7 commented 2 months ago

Note that I still need to revise the wording of our WebSocket docs, hence Draft (for now).

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (87e0454) to head (db2c3c4). Report is 1 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2324 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 63 63 Lines 7522 7539 +17 Branches 1275 1281 +6 ========================================= + Hits 7522 7539 +17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

CaselIT commented 2 months ago

I'm waiting for this to exit draft. let me know if I should review before then

vytas7 commented 2 months ago

Thanks @CaselIT , I haven't had much time to work on this, there are a couple of items remaining. Code-wise (not tests) I am not planning any more changes, so time permitting you could take a look at that part. Documentation is ready too I think.

I need to add more tests, and morph newsfragments.

vytas7 commented 2 months ago

@CaselIT maybe this PR is still not perfect (nothing is), but it is IMHO ready for review!