-
-
Notifications
You must be signed in to change notification settings - Fork 15
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: support async iter as rpc result #20
base: main
Are you sure you want to change the base?
Conversation
post(serialize(<Response>{ t: 's', i: msg.i, r: iterRes, d: false, e: error }), ...extra) | ||
} | ||
if (msg.i) | ||
return post(serialize(<Response>{ t: 's', i: msg.i, r: undefined, d: true, e: error }), ...extra) |
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.
I'd say we probably only return the d
flag when it's not finished to save some bytes for regular calls. Maybe u
for unfished
or s
for stream
?
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.
Currently, d
flag would be only posted when fn return result is iterable. In case of regular calls, it would be undefined
. Is this u expect?
That's a very interesting idea, thanks for bringing this up! I will do a proper review a bit later. From a quick scan, my main concern right now would be how we handle the timeout or disconnection in middle of the iterations. |
For long-running tasks, whether iterable or not, it may need to implement a heartbeat mechanism to handle timeout or disconnection cases. I would prefer to do it in business code due to its complexity, keep birpc stay simple 👀. |
Description
Currently, birpc can only handle result of rpc calling once, that make it hard to handle streaming multiple results (e.g. ReadableStream, Async Generator).
This pr check if fn result has
Symbol.asyncIterator
, if ture, post the result sequencially and consume it on the other side.Linked Issues
Additional context