search

cloudstateio / cloudstate

Distributed State Management for Serverless
https://cloudstate.io
Apache License 2.0
763 stars 97 forks source link

Empty streamed responses are not actually connected #493

Closed pvlugter closed 3 years ago

pvlugter commented 3 years ago

The presence part of the chat sample uses stream connection and cancellation and a Vote CRDT to track online presence. This is currently not working: cancelling a call to the connect method doesn't cause a stream cancel event. This is since support for metadata was added (#366).

The metadata support for streamed methods uses the first streamed response message to extract the metadata to put into response headers, and then connects the response stream. The connect method in the presence sample doesn't send any messages, so it doesn't connect the stream, so it can't be cancelled.

A workaround to get the chat example working is to add a return {}; to the end of the connect method, so that it sends an initial empty response and is connected.

Given that we're putting headers in the response and these are extracted from the first message, at the moment we can't also connect a possibly empty stream body to the response, as we can't return a response until we have the metadata for headers. So we now have a new requirement that streamed responses at least send an initial message to be connected.

We should either update the samples and document this (for the next released version of Cloudstate) or change the approach to support empty streams again.

sleipnir commented 3 years ago

I think it would be interesting to assess whether we could get around this with some approach in the proxy instead of forcing behavior on the side of the library or the user role. I think it would be interesting to endure the old behavior.

pvlugter commented 3 years ago

I have a simple fix for this in the proxy. I'll look at adding some tests.