addresses connection issues #931
base: master
Are you sure you want to change the base?
Conversation
! IsConnected is restored as it was before + TcpTransport has got a connect method which properly initializes the underlying tcpclient
…d by the server" but the following communication was not succesfully because the session contained dirty information as well as the sendCounter in TcpTransport. this commit fixes this situation, so a connection can be succesfully re-enstabilished and the communication restarted.
TLSharp.Core/Network/TcpTransport.cs
Outdated
private int sendCounter = 0; | ||
TcpClientConnectionHandler handler; | ||
string address; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
address is not reused, so you don't need to create a field for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, seems it is needed, howevr these 3 new fields can be marked as readonly
TLSharp.Core/Network/TcpTransport.cs
Outdated
ipAddress = IPAddress.Parse(address); | ||
} | ||
|
||
public async Task Connect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this method is async its name should have the Async
suffix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can change, but also send and receive in the same class are async and are not named as such. i suggest to keep the same style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a convention by Microsoft, the creators of C#; if you see any current API that doesn't fit this pattern we should rename it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok then
guys, i think we should push this fix asap, i have been testing on a software which has been running for half a day now, perfectly handling the reconnection.
before this fix, it could last only 30 minutes.
TLSharp.Core/TelegramClient.cs
Outdated
|
||
this.apiHash = apiHash; | ||
this.apiId = apiId; | ||
this.handler = handler; | ||
this.dcIpVersion = dcIpVersion; | ||
|
||
session = Session.TryLoadOrCreateNew(store, sessionUserId); | ||
session = Session.TryLoadOrCreateNew(this.store, sessionUserId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we just remove this line above? session will be initialized again anyway in ConnectAsync
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately not, because the following like needs it.
to save one initialization of session we must add extra code in the Connect method
You make it sound like if this PR fixed connection problems. It doesn't. It just allows the developer to reconnect using the same instance of TelegramClient. You could have done the same before by creating a new TelegramClient instance. |
true, but still a great improvement |
Hey Mario I wrote an alternative PR with much simpler change, wouldn't that achieve the same as this?: #937 |
yes, it looks fine and easier. let me test it for a couple of hours. |
reverted tcptransport
Excuse me @solarin I don´t know how to use your new version. I have researched some papers and I think that the problem is caused by TCP client. It´s not posible to know if que the socket is open in real time. Maybe usefull an special connection mode based in some kind of keep alive tcp socket: Thank you very much for you time. |
! IsConnected is restored as it was before