-
Notifications
You must be signed in to change notification settings - Fork 373
addresses connection issues #931
base: master
Are you sure you want to change the base?
Changes from 2 commits
2bcae5b
5778366
afd6d06
0dd6fdb
f697dac
bc87d0a
a266b89
e674cf1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,23 +12,38 @@ namespace TLSharp.Core.Network | |
|
|
||
| public class TcpTransport : IDisposable | ||
| { | ||
| private readonly TcpClient tcpClient; | ||
| private readonly NetworkStream stream; | ||
| private TcpClient tcpClient; | ||
| private NetworkStream stream; | ||
| private int sendCounter = 0; | ||
| TcpClientConnectionHandler handler; | ||
| string address; | ||
| int port; | ||
| IPAddress ipAddress; | ||
|
|
||
| public TcpTransport(string address, int port, TcpClientConnectionHandler handler = null) | ||
| { | ||
| this.handler = handler; | ||
| this.address = address; | ||
| this.port = port; | ||
| ipAddress = IPAddress.Parse(address); | ||
| } | ||
|
|
||
| public async Task Connect() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this method is async its name should have the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok then before this fix, it could last only 30 minutes. |
||
| { | ||
| if (handler == null) | ||
| { | ||
| var ipAddress = IPAddress.Parse(address); | ||
| var endpoint = new IPEndPoint(ipAddress, port); | ||
|
|
||
| if (tcpClient != null) | ||
| { | ||
| tcpClient.Close(); | ||
| } | ||
| tcpClient = new TcpClient(ipAddress.AddressFamily); | ||
| sendCounter = 0; | ||
|
|
||
| try { | ||
| tcpClient.Connect (endpoint); | ||
| try | ||
| { | ||
| await tcpClient.ConnectAsync(ipAddress, port); | ||
| } catch (Exception ex) { | ||
| throw new Exception ($"Problem when trying to connect to {endpoint}; either there's no internet connection or the IP address version is not compatible (if the latter, consider using DataCenterIPVersion enum)", | ||
| throw new Exception ($"Problem when trying to connect to {ipAddress}:{port}; either there's no internet connection or the IP address version is not compatible (if the latter, consider using DataCenterIPVersion enum)", | ||
| ex); | ||
| } | ||
| } | ||
|
|
@@ -102,7 +117,7 @@ public bool IsConnected | |
| { | ||
| get | ||
| { | ||
| return this.tcpClient.Connected; | ||
| return this.tcpClient != null && this.tcpClient.Connected; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,8 @@ public class TelegramClient : IDisposable | |
| private List<TLDcOption> dcOptions; | ||
| private TcpClientConnectionHandler handler; | ||
| private DataCenterIPVersion dcIpVersion; | ||
| private ISessionStore store; | ||
| string sessionUserId; | ||
|
|
||
| public Session Session | ||
| { | ||
|
|
@@ -56,22 +58,32 @@ public TelegramClient(int apiId, string apiHash, | |
| if (string.IsNullOrEmpty(apiHash)) | ||
| throw new MissingApiConfigurationException("API_HASH"); | ||
|
|
||
| if (store == null) | ||
| store = new FileSessionStore(); | ||
| this.store = store ?? new FileSessionStore(); | ||
| this.sessionUserId = sessionUserId; | ||
|
|
||
| this.apiHash = apiHash; | ||
| this.apiId = apiId; | ||
| this.handler = handler; | ||
| this.dcIpVersion = dcIpVersion; | ||
|
|
||
| session = Session.TryLoadOrCreateNew(store, sessionUserId); | ||
| session = Session.TryLoadOrCreateNew(this.store, sessionUserId); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| transport = new TcpTransport (session.DataCenter.Address, session.DataCenter.Port, this.handler); | ||
| } | ||
|
|
||
| public async Task ConnectAsync(bool reconnect = false, CancellationToken token = default(CancellationToken)) | ||
| { | ||
| token.ThrowIfCancellationRequested(); | ||
|
|
||
| if (!transport.IsConnected) | ||
| { | ||
| // we must recreate the session because it might track dirty information | ||
| // of a connection that maybe was disconnected, reusing that session will cause errors | ||
| session = Session.TryLoadOrCreateNew(store, sessionUserId); | ||
| await transport.Connect(); | ||
| } | ||
| if (!transport.IsConnected) | ||
|
solarin marked this conversation as resolved.
Outdated
|
||
| throw new Exception("Connection to Telegram failed"); | ||
|
|
||
| if (session.AuthKey == null || reconnect) | ||
| { | ||
| var result = await Authenticator.DoAuthentication(transport, token).ConfigureAwait(false); | ||
|
|
||
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