Skip to content

Conversation

@kongmoumou
Copy link

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

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)
Copy link
Member

@antfu antfu Sep 8, 2024

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?

Copy link
Author

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?

@antfu
Copy link
Member

antfu commented Sep 8, 2024

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.

@kongmoumou
Copy link
Author

kongmoumou commented Oct 22, 2024

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 👀.

@kongmoumou kongmoumou requested a review from antfu November 29, 2024 13:00
@kongmoumou
Copy link
Author

Looking forward to ur thoughts ~ @antfu

@antfu
Copy link
Member

antfu commented Jan 20, 2025

I wonder if you would consider implementing this as a separate library or an integration on top of birpc? birpc is designed to be simple and lightweight, while the complexity of this PR goes a bit beyond the scope for such a rarely used feature.

@kongmoumou
Copy link
Author

kongmoumou commented Jan 21, 2025

Thank you for your reply!

First, I really love the API design of birpc, which offers a fantastic DX. I’d prefer not to reinvent the wheel. However, I find it challenging to handle multiple returns from a single RPC call under the current ping-pong mechanism. Perhaps you have a better approach in mind? I would love to hear your thoughts.

The context here is that I'm building a browser extension that needs to stream OpenAI results from service worker and relay them back to the content script via RPC calls. IMO this type of usage is becoming increasingly common in the current AI era. I’m hoping to add this as a primitive feature to birpc, and I want to ensure it remains opt-in so it doesn't disrupt existing usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants