Skip to content
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

cancel() for client streaming requests #1849

Open
martin-traverse opened this issue Jan 8, 2023 · 0 comments
Open

cancel() for client streaming requests #1849

martin-traverse opened this issue Jan 8, 2023 · 0 comments

Comments

@martin-traverse
Copy link
Contributor

protobuf.js version: 7.1.2

Hi, I'm using protobuf.js streams for data upload/download in a data platform. As far as I can tell, there is no way to cancel an upload / client streaming call? I've looked at the streaming examples here and also the code for rpc.Service, there doesn't appear to be any cancel method or event (in general Service doesn't seem to respond to events, it just sends them for clients to respond to).

Calling end() is probably fine for e.g. a chat app, where you stop sending messages at some point and any messages sent before that point are delivered. But for a data upload you need to be able to cancel if there is an error on the client side. Suppose e.g. you are uploading a file from a network share that becomes unavailable, you really need to cancel the stream rather than letting it complete with only part of the file saved. This is only relevant for uploads, downloads (server streaming) are fine with error events.

Given the relationship between rpc.Service and the rpcImpl mechanism, I'd guess this would have to be implemented by passing some special sentinel value into rpcImpl, either by defining some static empty buffer or (better IMO) by allowing the rpcImpl requestData parameter to be UInt8Array | Signal for some signal type - perhaps an enum in the rpc namespace? Appreciate this is an API change because it requires the client-supplied rpcImpl to handle signals. One solution would be to add a constructor param in the Service class that disables signals by default, if signals are disabled we could throw an exception if cancel() is called, or perhaps emit an error event and report an error through the callback.

If you think this sounds interesting / useful I'd be happy to submit a PR for consideration? Please let me know!

For the time being I have worked around the limitation by adding a .cancel() after generating my API code and putting a basic implementation there, but having it in the framework would be much better and perhaps useful to others as well.

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

No branches or pull requests

1 participant