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

Correct way to stop streaming #38

Closed
rakewell opened this issue Jan 27, 2018 · 6 comments
Closed

Correct way to stop streaming #38

rakewell opened this issue Jan 27, 2018 · 6 comments

Comments

@rakewell
Copy link

Hello,

Many thanks for your work on this project.
I am just starting to use your API and have successfully streamed trade information using the AggregateTradeWebSocketClient with the following code:

        Private WithEvents m_BinanceAggregateTradeSocket As AggregateTradeWebSocketClient

        cancel = New CancellationTokenSource
        streamTask = m_BinanceAggregateTradeSocket.StreamAsync("ETHBTC", cancel.Token)

I retrieve trade updates as expected handling the event on the AggregateTradeWebSocketClient:

        Private Sub m_BinanceAggregateTradeSocket_AggregateTrade(sender As Object, e As 
                 AggregateTradeEventArgs) Handles m_BinanceAggregateTradeSocket.AggregateTrade

I have used the same instance of m_BinanceAggregateTradeSocket to subscribe to multiple symbols and this also works well. The problem is I would like to remove streaming for a symbol at a certain point. When I use the CancellationTokenSource to cancel, it cancels all of the streaming symbols. This is the case even though I create multiple instances of CancellationTokenSource and link each instance to a particular symbol.

What would be the best way to stop streaming a particular symbol?

I have also tried the Unsubscribe function on the WebSocketClient but I can't get this to work.

Thanks in advance for your help.

@sonvister
Copy link
Owner

sonvister commented Jan 27, 2018

@rakewell the combined streams feature is new and I have only implemented unsubscribe at the lowest level IWebSocketStream. This Unsubscribe method (you found) is not meant to be paired with the higher level IBinanceWebSocketClient implementations' Subscribe methods. If you want to unsubscribe you must cancel the stream and initialize a new AggregateTradeWebSocketClient with the desired symbols (if using combined streams).

I am currently working on adding unsubscribe functionality to this class and others so that client instances can be reused after cancelling and restarting the underlying combined stream. Regardless, the only way to alter the data received from Binance is to restart the combined stream once all changes to subscriptions have been made in all higher level classes.

If the stream is not canceled, the unsubscribe functionality in progress will ignore data received for any non-subscribed symbols, but that data will still be streaming.

Alternatively, you can have a separate AggregateTradeWebSocketClient with separate BinanceWebSocketStreams and control them independently (not use combined streams).

Also, I don't understand what you meant by "create multiple instances of CancellationTokenSource and link each instance to a particular symbol." What I think you mean should throw an invalid operation exception when calling StreamAsync multiple times.

@rakewell
Copy link
Author

rakewell commented Jan 28, 2018 via email

@sonvister
Copy link
Owner

@rakewell, I've added Unsubscribe functionality to AggregateTradeWebSocketClient and others in 0.2.0-alpha22. There is an example of usage in CombinedStreamsExample of the BinanceTradeHistory sample application. If you are using TaskController or RetryTaskController, these now have a CancelAsync method to use if unsubscribing or subscribing after streaming begins. Please let me know if this works for you.

@sonvister
Copy link
Owner

sonvister commented Jan 29, 2018

@rakewell, as for the Open and Close events, I already consider them accessible from the higher level classes via IBinanceWebSocketClient.WebSocket.Client. There is no Error event because exceptions are thrown from the StreamAsync() as demonstrated in the TaskController class. I've added better descriptions to these method definitions, which should help, but am still considering removing the StreamAsync extensions that combine subscribe with stream because they seem to cause more confusion than convenience.

@sonvister
Copy link
Owner

@rakewell, I wanted to mention I reconsidered your suggestion for the Open and Close events. At first I didn't like the idea of adding layers of event propagation (bubbling) for a minor convenience. But, even though the events are accessible, they are buried. Since they also make sense within context of the higher level classes, I added the events to IBinanceWebSocketClient and used registration (add/remove) forwarding to avoid creating new events and propagation. The lower-level IWebSocketClient is read-only so this works. This change will be in 0.2.0-alpha23.

@rakewell
Copy link
Author

rakewell commented Jan 29, 2018 via email

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

No branches or pull requests

2 participants