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 token doesn't unsubscribe UserDataWebSocketClient subscription #42

Closed
rakewell opened this issue Jan 31, 2018 · 4 comments
Closed

Comments

@rakewell
Copy link

Hello,

It seems that calling the cancel method on the CancellationTokenSource for the UserDataWebSocketClient instance does't unsubscribe the the user subscription. I notice that when the stream drops out, I execute the cancel method and then try and reconnect using the SubscribeAndStreamAsync method on the UserDataWebSocketClient instance it throws an exception because the user is already subscribed on this line:

        if (_listenKeys.ContainsKey(user))
            throw new InvalidOperationException($"{nameof(UserDataWebSocketClient)}: Already subscribed to user.");

        try

    public virtual async Task SubscribeAsync(IBinanceApiUser user, Action<UserDataEventArgs> callback, CancellationToken token = default)
    {
        Throw.IfNull(user, nameof(user));

        if (_listenKeys.ContainsKey(user))
            throw new InvalidOperationException($"{nameof(UserDataWebSocketClient)}: Already subscribed to user.");

        try
        {
            _listenKeys[user] = await _api.UserStreamStartAsync(user, token)
                .ConfigureAwait(false);

            if (string.IsNullOrWhiteSpace(_listenKeys[user]))
                throw new Exception($"{nameof(IUserDataWebSocketClient)}: Failed to get listen key from API.");

            SubscribeStream(_listenKeys[user], callback);
        }
        catch (OperationCanceledException) { }
        catch (Exception e)
        {
            if (!token.IsCancellationRequested)
            {
                Logger?.LogError(e, $"{nameof(UserDataWebSocketClient)}.{nameof(SubscribeAsync)}");
                throw;
            }
        }
    }

It may be that the cancel token is not supposed to unsubscribe the user and I should just execute the StreamAsync method on the webSocket?

Also, just wondering why the exception doesn't get propagated to the client. It doesn't get raised and the code just continues as if nothing happened. Is it because i'm running it on a different thread Dim cnnTask = Task.Run(Async Sub() ConnectSocket()) ? if so, can you recommend a way that I can get exceptions raised in the client (apologies if this may be a basic question, my parallel programming experience is not great)?

@sonvister
Copy link
Owner

@rakewell I'm somewhat aware UserDataWebSocketClient has issues, I just haven't gotten back around to determine the details (so thanks for the the help there). I'll give it bug status, but in my mind I considered this class in-progress with regard to adding and improving the combined streams functionality. First step being the use of combined streams, but it still needs to be fixed/improved/redesigned to handle disconnect (as you have found) and unsubscribe similar to the other client classes and all need more testing.

@sonvister
Copy link
Owner

sonvister commented Jan 31, 2018

@rakewell as for the Exception not being propagated, that is the behavior of Task by design which has Exception, IsFaulted, and Status properties for you to determine what happened with aTask. Using await handles Exception propagation for you without the use of AggregateException.

@sonvister
Copy link
Owner

@rakewell. I changed IUserDataWebSocketClient back to its previous single stream and single user implementation in SingleUserDataWebSocketClient and I moved the incomplete combined streams implementation to MultiUserDataWebSocketClient. The code is available now... and the changes will be in 0.2.0-alpha24.

@rakewell
Copy link
Author

many thanks @sonvister, I will update, test and let you know if there are any issues.

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