patterncsharpMinor
Async telnet connection over StreamSocket
Viewed 0 times
asyncstreamsockettelnetoverconnection
Problem
My team and I have a very poor understanding of best practices in relation to telnet. We have
I'm trying to understand
Task.Delay and Task.Wait in the code, including async voids and from what I understand those are potential areas that can cause deadlocks and other issues.I'm trying to understand
async in relation to running a telnet client. Is this a safe implementation of cancellation tokens and telnet connections?public enum TelnetClientStatus : int
{
Unset = 0,
Connected= 1,
Disconnected = 2,
Failed = 3
}
public TelnetClientStatus Status { get; set; } = TelnetClientStatus.Unset;
public async Task OpenConnection()
{
//Don't allow the session to keep trying to open after the 5 seconds
var cancellationTokenSource = new CancellationTokenSource();
cancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(5));
Status = await Task.Run(MakeConnection, cancellationTokenSource.Token);
}
private async Task MakeConnection()
{
_streamSocket = new StreamSocket();
_dataWriter = new DataWriter(_streamSocket.OutputStream);
_dataReader = new DataReader(_streamSocket.InputStream)
{
InputStreamOptions = InputStreamOptions.Partial
};
try
{
await _streamSocket.ConnectAsync(new HostName(HostName), Port.ToString());
return TelnetClientStatus.Connected;
}
catch (TaskCanceledException) //All other exceptions need to be investigated not ignored
{
//I understand a connection failed due to timeout, give the app some flag to work with
//Maybe retry the connection?
return TelnetClientStatus.Failed;
}
catch (Exception ex)
{
throw ex;
}
}Solution
I see at least two points.
-
if you have a
-
Having the
-
if you have a
catch to catch a sytem Exception (wich is not good btw., it would be better to catch specific exceptions) and the only action is to throw that exception then its useless to have. If you need to rethrow an exception you should always omit the ex so just throw;, otherwise the StackTrace will be broken. -
Having the
public setter of the Status property is breaking encapsulation. Why should a user of the object be able to set this property? You should make it private.Context
StackExchange Code Review Q#114683, answer score: 5
Revisions (0)
No revisions yet.