TgSharp
TgSharp copied to clipboard
Fix CI and unstable transport
Recently CI has been extremely buggy and unreliable, during extensive testing TCP Transport is identified as the culprit. Errors like
System.InvalidOperationException : Couldn't read the packet length (was 0, expected 4)
that would happen because of force disconnections right after the first encrypted write. For this reason and because the transport layer was long overdue for a rewrite we changed the transport to an asynchronous websocket transport + aes-ctr obfuscation
@aarani do you know why the CI for windows still fails?
@aarani do you know why the CI for windows still fails?
nblockchain's CI sercrets needs an update, for checking out the actual CI result check the results on my fork.
@aarani New transport will work through proxy?
yeah, It's not implemented but with a simple code change you can use proxies.
if it's only to be able to use .NETStandard, you can use 4.7.1
2 The versions listed here represent the rules that NuGet uses to determine whether a given .NET Standard library is applicable. While NuGet considers .NET Framework 4.6.1 as supporting .NET Standard 1.5 through 2.0, there are several issues with consuming .NET Standard libraries that were built for those versions from .NET Framework 4.6.1 projects. For .NET Framework projects that need to use such libraries, we recommend that you upgrade the project to target .NET Framework 4.7.2 or higher.
I was suggesting 4.7.1 because it's what we use in geewallet and works fine. Anyway this is a good source, please add the URL to the commit message.
Based on @Laiteux's testing, this commit might cause issues in .NET/.NET Core. These platforms are unsupported by us cause TgSharp is a .NET Framework library but I think it makes sense to fix it before merge
Based on @Laiteux's testing, this commit might cause issues in .NET/.NET Core. These platforms are unsupported by us cause TgSharp is a .NET Framework library but I think it makes sense to fix it before merge
We can fix it later when we migrate to .NETCore/.NET5.
That means I won't be able to keep using TgSharp anymore, since all my projects are .NET 5 or at least .NET Core 3.1. Honestly, who uses .NET Framework anymore...
Or then a second option would be to keep the TcpTransport and let the user decide what he wanna use, but that would introduce more complexity than anything else.
I won't be able to keep using TgSharp anymore
I said we will migrate, but no need to do it in this PR. The smallest this PR is the better, and the earlier we can start merging other PRs. We don't want this PR to become a monster.
Based on @Laiteux's testing, this commit might cause issues in .NET/.NET Core. These platforms are unsupported by us cause TgSharp is a .NET Framework library but I think it makes sense to fix it before merge
We can fix it later when we migrate to .NETCore/.NET5.
I agree! it's not our bug either, so we either have to try to fix our dependency and PR it or just use another websocket library which we can figure out after this gets merged
That means I won't be able to keep using TgSharp anymore, since all my projects are .NET 5 or at least .NET Core 3.1. Honestly, who uses .NET Framework anymore...
Or then a second option would be to keep the TcpTransport and let the user decide what he wanna use, but that would introduce more complexity than anything else.
It's just for this PR not permanent.
This will fix the retry logic:
@@ -136,19 +128,20 @@ namespace TgSharp.Core.Network
await client.SendInstant(finalInit);
}
- public async Task ConnectAsync(int retryCount = 5)
+ public async Task ConnectAsync(int retryCount = 0)
{
- while (retryCount-- != 0)
+ while (retryCount-- != -1)
{
try
{
await InitializeClient();
break;
}
- catch (Exception ex)
+ catch
{
- if (retryCount == 1)
- throw ex;
+ if (retryCount == -1)
+ throw;
+
await Task.Delay(1000);
}
}
I don't have access to this repo therefore I can't do the changes myself for now.
CI is broken now in your branch.
CI is broken now in your branch.
because my session is now broken
because my session is now broken
Let's fix that before you change the retryCount.
any update on this PR?
Last but not least, I'm having trouble finding the MS docs where it recommends to use underscores in method names.
Last but not least, I'm having trouble finding the MS docs where it recommends to use underscores in method names.
which methods are you talking about ?
Last but not least, I'm having trouble finding the MS docs where it recommends to use underscores in method names.
I can't find the original documentation for it, but event handler name Object_EventName is a fairly old convention.
https://docs.microsoft.com/en-us/dotnet/desktop/winforms/event-handlers-overview-windows-forms?view=netframeworkdesktop-4.8
also VS creates event handlers with this same exact naming scheme
any update?
any update?
Nope, and there will never be. Because knocte always had a urge to control every single misplaced comma or underscore despite wanting to take the lead on open-source projects. Hopefully one day he understands you can't have a desire to lead people and a compulsive behavior to "optimize" everyone's code. I would have loved too as well, sadly you'll never see this PR being merged, mainly for that reason. Everyone eventually got tired and that's it, that's why this project is dead. Props to @aarani for his crazy work on everything else and from the very beginning, without that dude I probably wouldn't even be earning a living the way I'm doing it today.