Code-Sharp / WampSharp

A C# implementation of WAMP (The Web Application Messaging Protocol)
http://wampsharp.net
Other
385 stars 83 forks source link

IWampChannel.Open() cancellation/timeout support #324

Closed esso23 closed 3 years ago

esso23 commented 3 years ago

Hi Elad,

I wanna ask, is there a way how to provide CancellationToken or timeout to Channel.Open() method? The default timeout is quite high. Cancellation support would be nice aswell.

Channel I'm using:

            var channel = channelFactory
                .ConnectToRealm(config.Configuration.RealmName)
                .WebSocketTransport(new Uri(config.Configuration.ServerAddress))
                .MsgpackSerialization()
                .Authenticator(new TicketAuthenticator(config.Configuration.AuthId, config.Configuration.AuthTicket))
                .Build();

Thanks Esso

darkl commented 3 years ago

There's nothing built-in of course, but you can use

var openTask = channel.Open();
await Task.WhenAny(openTask, Task.Delay(1500)).ConfigureAwait(false);

if (openTask.IsCompleted)
{
// ...
}
esso23 commented 3 years ago

That would technically work, but in that case I do not get the exception which is thrown after the actual timeout so I won't know why it actually failed. Also the openTask will continue running in the background for no reason.

Wouldn't it be better to implement this functionality on Open() method?

darkl commented 3 years ago

Yeah, it would be better. I will try to include this in a future release.

darkl commented 3 years ago

So I tried implementing CancellationToken support in the branch open-cancellationToken. However, since WampSharp was not designed with CancellationToken in mind (it was designed around the api of WebSocket4Net), it won't be easy to fully support CancellationToken. For instance, it is currently kind of impossible to use the given CancellationToken while sending the HELLO message or waiting for the WELCOME message. I did add support for passing a CancellationToken to the ConnectAsync method, which will allow you to cancel your request before the WebSocket connection was established, but again, this is far from being a full solution. You can try it out (there are packages available on the NuGet feed on GitHub, see here how to set it up) and let me know if it helps. If not, I will not merge it.

esso23 commented 3 years ago

Tried it today, but it doesn't seem to work. It gets stuck on Open forever (I'm connecting to a host that's unreachable). Here's the code:

            var factory = new WampChannelFactory();

            mChannel = factory.ConnectToRealm(mRealmName)
                .WebSocket4NetTransport(mServerAddress)
                .SetSecurityOptions(ConfigureSecurityOptions)
                .MsgpackSerialization()
                .Authenticator(new JwtClientAuthenticator(mUserName, mAuthToken))
                .Build();

            CancellationTokenSource channelOpenTokenSource = new CancellationTokenSource();

            channelOpenTokenSource.CancelAfter(TimeSpan.FromSeconds(5));

            try
            {
                await mChannel.Open(channelOpenTokenSource.Token);
            }
            catch (OperationCanceledException e)
            {
                throw new TimeoutException();
            }
            catch (Exception ex)
            {
                throw;
            }
darkl commented 3 years ago

You can't use it with WebSocket4Net, they don't have this functionality built in. That's another problem with this feature. Try using WampSharp.WebSockets instead.

Elad

On Wed, Dec 30, 2020, 20:57 Andrej Kmetík notifications@github.com wrote:

Tried it today, but it doesn't seem to work.

        var factory = new WampChannelFactory();

        mChannel = factory.ConnectToRealm(mRealmName)
            .WebSocket4NetTransport(mServerAddress)
            .SetSecurityOptions(ConfigureSecurityOptions)
            .MsgpackSerialization()
            .Authenticator(new JwtClientAuthenticator(mUserName, mAuthToken))
            .Build();

        CancellationTokenSource channelOpenTokenSource = new CancellationTokenSource();

        channelOpenTokenSource.CancelAfter(TimeSpan.FromSeconds(5));

        try
        {
            await mChannel.Open(channelOpenTokenSource.Token);
        }
        catch (OperationCanceledException e)
        {
            throw new TimeoutException();
        }
        catch (Exception ex)
        {
            throw;
        }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Code-Sharp/WampSharp/issues/324#issuecomment-752815281, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIS75UZRMWVDNRDRLWBR7LSXPLAJANCNFSM4S2BZPHA .

esso23 commented 3 years ago

I tried it with WampSharp.WebSockets and it throws an exception immediately when the host is unreachable, so the cancellation is not required in this case.

Anyway, I need to use WebSocket4Net, because I cannot connect to secure websocket with unsigned certificate with System.Net.WebSocket - apparently it's a known issue since 2018 at least, so the hopes of this getting fixed are minimal https://github.com/mono/mono/issues/8660 And yea, I'm developing a Xamarin.Forms app. I guess I'm gonna be forced to make some ugly workaround for this.

About WebSocket4Net, I guess WampSharp is not using this version of WebSocket4Net (since I see async methods with cancellation support there), but I see you there as a contributor, which is kinda confusing, so I just want know the story.

darkl commented 3 years ago

I wasn't aware that there are new versions of WebSocket4Net. The latest version on NuGet Gallery is from January 2018. The README file of the repository you linked to says

It is a completely new version which is base on SuperSocket 2.0 and dotnet core. This version is still in progress, so please don't use it in production for now.

Happy new year!

Elad

esso23 commented 3 years ago

Oh I didn't know that pull requests get forked with a repository aswell, my bad :)

Happy new year to you aswell.

darkl commented 3 years ago

It's actually not a fork of the repository, he just overridden the main branch with the new version of code, see this commit.

Elad