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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canceling the ConnectAsync() with a CancellationToken #377

Open
Aspher0 opened this issue May 19, 2024 · 5 comments
Open

Canceling the ConnectAsync() with a CancellationToken #377

Aspher0 opened this issue May 19, 2024 · 5 comments

Comments

@Aspher0
Copy link

Aspher0 commented May 19, 2024

Hello dog 馃槃

First of all, thanks for the project, it helped me a lot.
However, I'm having struggle to cancel an ongoing connection attempt initiated with ConnectAsync, would it be possible to maybe add an overload to the ConnectAsync() method with a CancellationToken parameter so we can cancel it anytime please ?
I tried forking the project but I ended up breaking everything, I'm sadly not that talented ahah

I tried disposing the client in both 3.1.1 version and 3.0.8 as someone had suggested in the issues, but no luck, it will keep trying to connect even after disposing.

Thanks in advance for your answer!

@doghappy
Copy link
Owner

Hi @Aspher0, thanks for your feedback. make a new overload to let user pass a CancellationToken is a good idea. but currently we have an option: ConnectionTimeout, the lib will create CancellationToken based on ConnectionTimeout.

What will happen if we have the following code?

using var io = new SocketIO(new SocketIOOptions
{
    ConnectionTimeout = TimeSpan.FromSeconds(1)
});

using var cts = new CancellationTokenSource();
await io.ConnectAsync(cts.Token);
...
cts.Cancel();

Questions:

  1. I just want to say if we added that overload, we will have 2 CancellationTokens, finally which 1 should be used? any suggestion?
  2. could you explain your scenario, why you need to Cancel manually?

@Aspher0
Copy link
Author

Aspher0 commented May 21, 2024

Hello there, thanks for your answer and interest.

First of all, if you want context, I use your library in my project named "GlamMaster" which can be found here, in SocketManager.cs and SocketOnEventsManager.cs : https://github.com/Aspher0/GlamMaster/blob/main/GlamMaster/Socket/

So, I do have a ConnectionTimeout already set, but it doesn't fixes anything. Let me explain.
So, basically, my use case is that my program can have a socket connected or not, and I want the user to be able to abort the connection at any time, without having to wait for the connection timeout to end.
Since there is no way to specify a cancellation token to manually cancel the socket connection, I thought disposing the client would prevent the socket from firing events such as the OnError, OnReconnectAttempt and so on.
But it turns out that those events are still fired even after being unsubscribed, the client disposed and everything else.
I actually had to use some weird shenanigans to make the client not do unexpected behavior, and since I have to actually dispose the client to "cancel it" (or somewhat cancelling it), I have to make a new client everytime I want to make a new connection.
Now, let's say my socket server is down. My users would click the "connect button" and then, seeing it doesnt work, they would click the abort button and try to connect again, maybe more than once. If the server gets back online somehow, multiple clients would be connected at the same time on the server, and multiple clients would be referenced in the program's memory and would probably cause increased memory usage and so on.

I did end up finding a way to avoid every problems, but still, the events still being fired even after being unsubscribe and the client disposed, as well as the other issues I mentionned, makes my solution very messy and dirty.

Now, let's say you make an overload of the ConnectAsync function, you could let the user store their cancellationtoken on their side (you wouldn't need to worry about taking care of it, the user has to take care of that) and then you would just pass the token in the overloaded function and all of its tasks and stuff ?
Then, in your exemple, if the cts.Cancel() is called, the connection would stop and everything would be cancelled and the client could catch the cancellation exception.

Basically :

try
{
    await io.ConnectAsync(cts.Token);
}
catch (OperationCanceledException e)
{
    // The socket connection has been aborted, do stuff
}
// Other catches here if needed
finally
{
    cts.Dispose();
}

I saw that you have a while loop that basically check if the token is cancelled or not, maybe have a boolean variable that specifies if it's a user-specified token or the automatically generated one.

@Aspher0
Copy link
Author

Aspher0 commented May 28, 2024

Hello @doghappy
Any news ? 馃槃
Have a nice day!

@doghappy
Copy link
Owner

Hi, Sorry for the slow response.

My boss gave me too much work 馃ぃ and I have to take care of my baby馃懅, so I can only do maintenance no more than 8 hours a week.

Thanks for your explanation, by default the lib will always try to connect to the server when you calling ConnectAsync(), I'm not sure if setting Reconnection to false will solve your problem.


And here is a solution: https://stackoverflow.com/a/29623382/7771913
I will try the solution later.

Please leave a message if Reconnection = false is useful.

if (useful)
{
    Thread.Sleep(TimeSpan.FromDays(7));
}
DoItAsap();

@Aspher0
Copy link
Author

Aspher0 commented May 29, 2024

Hey @doghappy
Thank you for your answer 馃槃
Don't worry, work is important and family is even more !

I haven't tried the Reconnection = false yet but I'd like to let my users choose the number of reconnection attempts and other options so I don't think it is a viable solution for me. When I have time, I will still try it and tell you if it works.

Also, the stackoverflow link you linked doesn't seem relevant in our case, I don't really see how it could help since your project does not let the user pass a cancellation token to the ConnectAsync() function.

If the solution does not work, I will retry forking your project again and add an overload of the ConnectAsync function and maybe open a pull request, if I can get it to work properly this time 馃槃

Take care !

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

2 participants