Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Commit ecab6f6

Browse files
moozzykmoozzyk
committed
PR feedback
1 parent 34078d9 commit ecab6f6

File tree

5 files changed

+16
-25
lines changed

5 files changed

+16
-25
lines changed

‎src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ private Task StartAsyncCore()
108108
if (ChangeState(from: ConnectionState.Disconnected, to: ConnectionState.Connecting) != ConnectionState.Disconnected)
109109
{
110110
return Task.FromException(
111-
new InvalidOperationException("Cannot start a connection that is not in the Disconnected state."));
111+
new InvalidOperationException($"Cannot start a connection that is not in the {nameof(ConnectionState.Disconnected)} state."));
112112
}
113113

114114
_startTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -120,12 +120,10 @@ private Task StartAsyncCore()
120120
if (t.IsFaulted)
121121
{
122122
_startTcs.SetException(t.Exception.InnerException);
123-
Closed?.Invoke(t.Exception);
124123
}
125124
else if (t.IsCanceled)
126125
{
127126
_startTcs.SetCanceled();
128-
Closed?.Invoke(t.Exception);
129127
}
130128
else
131129
{
@@ -209,10 +207,6 @@ private async Task StartAsyncInternal()
209207
{
210208
Closed?.Invoke(t.Exception.InnerException);
211209
}
212-
else if (t.IsCanceled)
213-
{
214-
Closed?.Invoke(t.Exception);
215-
}
216210
else
217211
{
218212
Closed?.Invoke(null);
@@ -447,7 +441,7 @@ private async Task SendAsyncCore(byte[] data, CancellationToken cancellationToke
447441

448442
public async Task StopAsync()
449443
{
450-
lock(_stateChangeLock)
444+
lock (_stateChangeLock)
451445
{
452446
if (!(_connectionState == ConnectionState.Connecting || _connectionState == ConnectionState.Connected))
453447
{
@@ -563,7 +557,7 @@ public void Dispose()
563557

564558
private ConnectionState ChangeState(ConnectionState from, ConnectionState to)
565559
{
566-
lock(_stateChangeLock)
560+
lock (_stateChangeLock)
567561
{
568562
var state = _connectionState;
569563
if (_connectionState == from)

‎src/Microsoft.AspNetCore.Sockets.Client.Http/Internal/SocketClientLoggerExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ internal static class SocketClientLoggerExtensions
151151
LoggerMessage.Define<DateTime, string>(LogLevel.Information, new EventId(18, nameof(StoppingClient)), "{time}: Connection Id {connectionId}: Stopping client.");
152152

153153
private static readonly Action<ILogger, DateTime, string, string, Exception> _exceptionThrownFromCallback =
154-
LoggerMessage.Define<DateTime, string, string>(LogLevel.Error, new EventId(19, nameof(ExceptionThrownFromCallback)), "{time}: Connection Id {connectionId}: An exception was thrown from the '{callback}' callback");
154+
LoggerMessage.Define<DateTime, string, string>(LogLevel.Error, new EventId(19, nameof(ExceptionThrownFromCallback)), "{time}: Connection Id {connectionId}: An exception was thrown from the '{callback}' callback.");
155155

156156
private static readonly Action<ILogger, DateTime, string, Exception> _disposingClient =
157157
LoggerMessage.Define<DateTime, string>(LogLevel.Information, new EventId(20, nameof(DisposingClient)), "{time}: Connection Id {connectionId}: Disposing client.");

‎test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ public async Task CanStopAndStartConnection(IHubProtocol protocol, TransportType
116116
await connection.StartAsync().OrTimeout();
117117
var result = await connection.InvokeAsync<string>("Echo", originalMessage).OrTimeout();
118118
Assert.Equal(originalMessage, result);
119-
await connection.StopAsync();
120-
await connection.StartAsync();
119+
await connection.StopAsync().OrTimeout();
120+
await connection.StartAsync().OrTimeout();
121121
result = await connection.InvokeAsync<string>("Echo", originalMessage).OrTimeout();
122122
Assert.Equal(originalMessage, result);
123123
}
@@ -139,7 +139,7 @@ public async Task CanStartConnectionFromClosedEvent()
139139
using (StartLog(out var loggerFactory))
140140
{
141141
const string originalMessage = "SignalR";
142-
var httpConnection = new HttpConnection(new Uri(_serverFixture.Url + "/default"));
142+
var httpConnection = new HttpConnection(new Uri(_serverFixture.Url + "/default"), loggerFactory);
143143
var connection = new HubConnection(httpConnection, new JsonHubProtocol(), loggerFactory);
144144
var restartTcs = new TaskCompletionSource<object>();
145145
connection.Closed += async e =>
@@ -156,8 +156,8 @@ public async Task CanStartConnectionFromClosedEvent()
156156
await connection.StartAsync().OrTimeout();
157157
var result = await connection.InvokeAsync<string>("Echo", originalMessage).OrTimeout();
158158
Assert.Equal(originalMessage, result);
159-
await connection.StopAsync();
160-
await restartTcs.Task;
159+
await connection.StopAsync().OrTimeout();
160+
await restartTcs.Task.OrTimeout();
161161
result = await connection.InvokeAsync<string>("Echo", originalMessage).OrTimeout();
162162
Assert.Equal(originalMessage, result);
163163

‎test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.cs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ public async Task CanStartConnectionThatFailedToStart()
193193
await connection.DisposeAsync().OrTimeout();
194194
}
195195

196-
197196
[Fact]
198197
public async Task CanStartStoppedConnection()
199198
{
@@ -248,14 +247,14 @@ public async Task CanStopStartingConnection()
248247
connection.Closed += e => closeTcs.TrySetResult(null);
249248

250249
var startTask = connection.StartAsync();
251-
await allowStopTcs.Task;
250+
await allowStopTcs.Task.OrTimeout();
252251

253-
await Task.WhenAll(startTask.OrTimeout(), connection.StopAsync().OrTimeout());
252+
await Task.WhenAll(startTask, connection.StopAsync()).OrTimeout();
254253
await closeTcs.Task.OrTimeout();
255254
}
256255

257256
[Fact]
258-
public async Task CanStartConnectionStoppedWithError()
257+
public async Task CanStartConnectionAfterConnectionStoppedWithError()
259258
{
260259
var mockHttpHandler = new Mock<HttpMessageHandler>();
261260
mockHttpHandler.Protected()
@@ -286,16 +285,14 @@ public async Task CanStartConnectionStoppedWithError()
286285
await connection.SendAsync(new byte[] { 0x42 }).OrTimeout();
287286
}
288287
catch { }
289-
await closeTcs.Task.OrTimeout().OrTimeout();
288+
await closeTcs.Task.OrTimeout();
290289
await connection.StartAsync().OrTimeout();
291290
await connection.DisposeAsync().OrTimeout();
292291
}
293292

294293
[Fact]
295294
public async Task CanStopStoppedConnection()
296295
{
297-
var mockHttpHandler = new Mock<HttpMessageHandler>();
298-
299296
var connection = new HttpConnection(new Uri("http://fakeuri.org/"));
300297
await connection.StopAsync().OrTimeout();
301298
await connection.DisposeAsync().OrTimeout();

‎test/Microsoft.AspNetCore.SignalR.Client.Tests/TestConnection.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,16 @@ private async Task ReceiveLoopAsync(CancellationToken token)
138138
}
139139
}
140140
}
141-
Closed(null);
141+
Closed?.Invoke(null);
142142
}
143143
catch (OperationCanceledException)
144144
{
145145
// Do nothing, we were just asked to shut down.
146-
Closed(null);
146+
Closed?.Invoke(null);
147147
}
148148
catch (Exception ex)
149149
{
150-
Closed(ex);
150+
Closed?.Invoke(ex);
151151
}
152152
}
153153

0 commit comments

Comments
 (0)