-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add callback
api to WebSocketProxy
#41
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #41 +/- ##
==========================================
+ Coverage 96.74% 97.13% +0.39%
==========================================
Files 9 9
Lines 461 524 +63
Branches 67 74 +7
==========================================
+ Hits 446 509 +63
Misses 9 9
Partials 6 6 ☔ View full report in Codecov by Sentry. |
callback
api to WebSocket
callback
api to WebSocketProxy
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I just found a bug. The current implementation only supports a strict async def callback(ctx: CallbackPipeContextType[str]) -> None:
with ctx as (sender, receiver):
# multiple receives and one send, dead lock!
await receiver.receive()
await receiver.receive()
await sender.send("foo")
async def callback(ctx: CallbackPipeContextType[str]) -> None:
with ctx as (sender, receiver):
# one receive and multiple sends, dead lock!
async for message in receiver:
await sender.send("foo")
await sender.send("bar")
async def callback(ctx: CallbackPipeContextType[str]) -> None:
with ctx as (sender, receiver):
# sending before receiving, dead lock!
await sender.send("foo")
async for message in receiver:
await sender.send(message) Unfortunately, we can't resolve this issue until the underlying logic is rewritten using There is already a PR for Edit:I just created an issue to track this bug. #42 |
- bump `typing_extensions` from `4.5.0` to `4.12.0` - explicitly add `anyio` as a dependency - fix docs `host` typo
- test message modification using `callback` under normal conditions - test WebSocket closing when `callback` encounters an exception
- fix broken `httpx` authentication docs link
## Modify WebSocket message | ||
|
||
In some cases, you might want to modify the content of the messages that the WebSocket proxy receives and sends to the client and target server. | ||
|
||
In version `0.2.0` of `fastapi-proxy-lib`, we introduced a [`callback API`][fastapi_proxy_lib.core.websocket.ReverseWebSocketProxy.proxy] for `WebSocketProxy` to allow you to do this. | ||
|
||
See example: [ReverseWebSocketProxy#with-callback][fastapi_proxy_lib.core.websocket.ReverseWebSocketProxy--with-callback] | ||
|
||
Also: | ||
|
||
- RFC: [#40](https://github.com/WSH032/fastapi-proxy-lib/issues/40) | ||
- PR: [#41](https://github.com/WSH032/fastapi-proxy-lib/pull/41) | ||
|
||
!!!example | ||
The current implementation still has some defects. Read the [callback-implementation][fastapi_proxy_lib.core.websocket.BaseWebSocketProxy.send_request_to_target--callback-implementation] section, or you might accidentally shoot yourself in the foot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IvesAwadi What do you think of this description? Can you provide some specific use cases? This way, I can add them to the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IvesAwadi What do you think of this description? Can you provide some specific use cases? This way, I can add them to the documentation.
The description is good and makes sense, took a bit to respond to this (had some issues with my email.)
@IvesAwadi. This PR is completed. Do you think there are any areas that need improvement? If not, I will merge this PR tomorrow. As a non-native English speaker, I’m unsure about the quality of the API documentation (including inline docstrings). I would greatly appreciate any suggestions for improvement. If you want to preview the documentation in advance, you can follow the instructions in the CONTRIBUTING. |
typing_extensions
from4.5.0
to4.12.0
anyio
as a dependencyhost
typohttpx
authentication docs linktool_4_test_fixture
to factory to support testing different ws appsSummary
Close: #40
example
proxy server
client
Checklist
CONTRIBUTING.md
.