Collect transport fallback errors to provide better errors (#4293)

This commit is contained in:
BrennanConroy 2018-12-05 14:27:07 -08:00 committed by GitHub
parent a51ff6e6ad
commit c487f019f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 109 additions and 56 deletions

View File

@ -2622,14 +2622,12 @@
"balanced-match": { "balanced-match": {
"version": "1.0.0", "version": "1.0.0",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"brace-expansion": { "brace-expansion": {
"version": "1.1.11", "version": "1.1.11",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"balanced-match": "^1.0.0", "balanced-match": "^1.0.0",
"concat-map": "0.0.1" "concat-map": "0.0.1"
@ -2644,20 +2642,17 @@
"code-point-at": { "code-point-at": {
"version": "1.1.0", "version": "1.1.0",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"concat-map": { "concat-map": {
"version": "0.0.1", "version": "0.0.1",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"console-control-strings": { "console-control-strings": {
"version": "1.1.0", "version": "1.1.0",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"core-util-is": { "core-util-is": {
"version": "1.0.2", "version": "1.0.2",
@ -2774,8 +2769,7 @@
"inherits": { "inherits": {
"version": "2.0.3", "version": "2.0.3",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"ini": { "ini": {
"version": "1.3.5", "version": "1.3.5",
@ -2787,7 +2781,6 @@
"version": "1.0.0", "version": "1.0.0",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"number-is-nan": "^1.0.0" "number-is-nan": "^1.0.0"
} }
@ -2802,7 +2795,6 @@
"version": "3.0.4", "version": "3.0.4",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"brace-expansion": "^1.1.7" "brace-expansion": "^1.1.7"
} }
@ -2810,14 +2802,12 @@
"minimist": { "minimist": {
"version": "0.0.8", "version": "0.0.8",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"minipass": { "minipass": {
"version": "2.2.4", "version": "2.2.4",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"safe-buffer": "^5.1.1", "safe-buffer": "^5.1.1",
"yallist": "^3.0.0" "yallist": "^3.0.0"
@ -2836,7 +2826,6 @@
"version": "0.5.1", "version": "0.5.1",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"minimist": "0.0.8" "minimist": "0.0.8"
} }
@ -2917,8 +2906,7 @@
"number-is-nan": { "number-is-nan": {
"version": "1.0.1", "version": "1.0.1",
"bundled": true, "bundled": true,
"dev": true, "dev": true
"optional": true
}, },
"object-assign": { "object-assign": {
"version": "4.1.1", "version": "4.1.1",
@ -2930,7 +2918,6 @@
"version": "1.4.0", "version": "1.4.0",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"wrappy": "1" "wrappy": "1"
} }
@ -3052,7 +3039,6 @@
"version": "1.0.2", "version": "1.0.2",
"bundled": true, "bundled": true,
"dev": true, "dev": true,
"optional": true,
"requires": { "requires": {
"code-point-at": "^1.0.0", "code-point-at": "^1.0.0",
"is-fullwidth-code-point": "^1.0.0", "is-fullwidth-code-point": "^1.0.0",

View File

@ -269,29 +269,34 @@ export class HttpConnection implements IConnection {
return; return;
} }
const transportExceptions: any[] = [];
const transports = negotiateResponse.availableTransports || []; const transports = negotiateResponse.availableTransports || [];
for (const endpoint of transports) { for (const endpoint of transports) {
this.connectionState = ConnectionState.Connecting; try {
const transport = this.resolveTransport(endpoint, requestedTransport, requestedTransferFormat); this.connectionState = ConnectionState.Connecting;
if (typeof transport === "number") { const transport = this.resolveTransport(endpoint, requestedTransport, requestedTransferFormat);
this.transport = this.constructTransport(transport); if (typeof transport === "number") {
if (!negotiateResponse.connectionId) { this.transport = this.constructTransport(transport);
negotiateResponse = await this.getNegotiationResponse(url); if (!negotiateResponse.connectionId) {
connectUrl = this.createConnectUrl(url, negotiateResponse.connectionId); negotiateResponse = await this.getNegotiationResponse(url);
} connectUrl = this.createConnectUrl(url, negotiateResponse.connectionId);
try { }
await this.transport!.connect(connectUrl, requestedTransferFormat); await this.transport!.connect(connectUrl, requestedTransferFormat);
this.changeState(ConnectionState.Connecting, ConnectionState.Connected); this.changeState(ConnectionState.Connecting, ConnectionState.Connected);
return; return;
} catch (ex) {
this.logger.log(LogLevel.Error, `Failed to start the transport '${HttpTransportType[transport]}': ${ex}`);
this.connectionState = ConnectionState.Disconnected;
negotiateResponse.connectionId = undefined;
} }
} catch (ex) {
this.logger.log(LogLevel.Error, `Failed to start the transport '${endpoint.transport}': ${ex}`);
this.connectionState = ConnectionState.Disconnected;
negotiateResponse.connectionId = undefined;
transportExceptions.push(`${endpoint.transport} failed: ${ex}`);
} }
} }
throw new Error("Unable to initialize any of the available transports."); if (transportExceptions.length > 0) {
throw new Error(`Unable to connect to the server with any of the available transports. ${transportExceptions.join(" ")}`);
}
throw new Error("None of the transports supported by the client are supported by the server.");
} }
private constructTransport(transport: HttpTransportType) { private constructTransport(transport: HttpTransportType) {
@ -324,15 +329,18 @@ export class HttpConnection implements IConnection {
if ((transport === HttpTransportType.WebSockets && !this.options.WebSocket) || if ((transport === HttpTransportType.WebSockets && !this.options.WebSocket) ||
(transport === HttpTransportType.ServerSentEvents && !this.options.EventSource)) { (transport === HttpTransportType.ServerSentEvents && !this.options.EventSource)) {
this.logger.log(LogLevel.Debug, `Skipping transport '${HttpTransportType[transport]}' because it is not supported in your environment.'`); this.logger.log(LogLevel.Debug, `Skipping transport '${HttpTransportType[transport]}' because it is not supported in your environment.'`);
throw new Error(`'${HttpTransportType[transport]}' is not supported in your environment.`);
} else { } else {
this.logger.log(LogLevel.Debug, `Selecting transport '${HttpTransportType[transport]}'.`); this.logger.log(LogLevel.Debug, `Selecting transport '${HttpTransportType[transport]}'.`);
return transport; return transport;
} }
} else { } else {
this.logger.log(LogLevel.Debug, `Skipping transport '${HttpTransportType[transport]}' because it does not support the requested transfer format '${TransferFormat[requestedTransferFormat]}'.`); this.logger.log(LogLevel.Debug, `Skipping transport '${HttpTransportType[transport]}' because it does not support the requested transfer format '${TransferFormat[requestedTransferFormat]}'.`);
throw new Error(`'${HttpTransportType[transport]}' does not support ${TransferFormat[requestedTransferFormat]}.`);
} }
} else { } else {
this.logger.log(LogLevel.Debug, `Skipping transport '${HttpTransportType[transport]}' because it was disabled by the client.`); this.logger.log(LogLevel.Debug, `Skipping transport '${HttpTransportType[transport]}' because it was disabled by the client.`);
throw new Error(`'${HttpTransportType[transport]}' is disabled by the client.`);
} }
} }
return null; return null;

View File

@ -171,9 +171,9 @@ describe("HttpConnection", () => {
await expect(connection.start(TransferFormat.Text)) await expect(connection.start(TransferFormat.Text))
.rejects .rejects
.toThrow("Unable to initialize any of the available transports."); .toThrow("None of the transports supported by the client are supported by the server.");
}, },
"Failed to start the connection: Error: Unable to initialize any of the available transports."); "Failed to start the connection: Error: None of the transports supported by the client are supported by the server.");
}); });
it("preserves user's query string", async () => { it("preserves user's query string", async () => {
@ -273,9 +273,11 @@ describe("HttpConnection", () => {
await expect(connection.start(TransferFormat.Text)) await expect(connection.start(TransferFormat.Text))
.rejects .rejects
.toThrow("Unable to initialize any of the available transports."); .toThrow(`Unable to connect to the server with any of the available transports. ${negotiateResponse.availableTransports[0].transport} failed: Error: '${negotiateResponse.availableTransports[0].transport}' is disabled by the client.` +
` ${negotiateResponse.availableTransports[1].transport} failed: Error: '${negotiateResponse.availableTransports[1].transport}' is disabled by the client.`);
}, },
"Failed to start the connection: Error: Unable to initialize any of the available transports."); /Failed to start the connection: Error: Unable to connect to the server with any of the available transports. [a-zA-Z]+\b failed: Error: '[a-zA-Z]+\b' is disabled by the client. [a-zA-Z]+\b failed: Error: '[a-zA-Z]+\b' is disabled by the client./,
/Failed to start the transport '[a-zA-Z]+': Error: '[a-zA-Z]+' is disabled by the client./);
}); });
it(`cannot be started if server's only transport (${HttpTransportType[requestedTransport]}) is masked out by the transport option`, async () => { it(`cannot be started if server's only transport (${HttpTransportType[requestedTransport]}) is masked out by the transport option`, async () => {
@ -311,10 +313,11 @@ describe("HttpConnection", () => {
await connection.start(TransferFormat.Text); await connection.start(TransferFormat.Text);
fail("Expected connection.start to throw!"); fail("Expected connection.start to throw!");
} catch (e) { } catch (e) {
expect(e.message).toBe("Unable to initialize any of the available transports."); expect(e.message).toBe(`Unable to connect to the server with any of the available transports. ${HttpTransportType[requestedTransport]} failed: Error: '${HttpTransportType[requestedTransport]}' is disabled by the client.`);
} }
}, },
"Failed to start the connection: Error: Unable to initialize any of the available transports."); `Failed to start the connection: Error: Unable to connect to the server with any of the available transports. ${HttpTransportType[requestedTransport]} failed: Error: '${HttpTransportType[requestedTransport]}' is disabled by the client.`,
`Failed to start the transport '${HttpTransportType[requestedTransport]}': Error: '${HttpTransportType[requestedTransport]}' is disabled by the client.`);
}); });
}); });
@ -378,9 +381,9 @@ describe("HttpConnection", () => {
const connection = new HttpConnection("http://tempuri.org", options); const connection = new HttpConnection("http://tempuri.org", options);
await expect(connection.start(TransferFormat.Text)) await expect(connection.start(TransferFormat.Text))
.rejects .rejects
.toThrow("Unable to initialize any of the available transports."); .toThrow("None of the transports supported by the client are supported by the server.");
}, },
"Failed to start the connection: Error: Unable to initialize any of the available transports."); "Failed to start the connection: Error: None of the transports supported by the client are supported by the server.");
}); });
it("does not send negotiate request if WebSockets transport requested explicitly and skipNegotiation is true", async () => { it("does not send negotiate request if WebSockets transport requested explicitly and skipNegotiation is true", async () => {
@ -694,12 +697,12 @@ describe("HttpConnection", () => {
await expect(connection.start(TransferFormat.Text)) await expect(connection.start(TransferFormat.Text))
.rejects .rejects
.toEqual(new Error("Unable to initialize any of the available transports.")); .toEqual(new Error("Unable to connect to the server with any of the available transports. ServerSentEvents failed: Error: EventSource constructor called."));
expect(eventSourceConstructorCalled).toEqual(true); expect(eventSourceConstructorCalled).toEqual(true);
}, },
"Failed to start the transport 'ServerSentEvents': Error: EventSource constructor called.", "Failed to start the transport 'ServerSentEvents': Error: EventSource constructor called.",
"Failed to start the connection: Error: Unable to initialize any of the available transports."); "Failed to start the connection: Error: Unable to connect to the server with any of the available transports. ServerSentEvents failed: Error: EventSource constructor called.");
}); });
it("uses WebSocket constructor from options if provided", async () => { it("uses WebSocket constructor from options if provided", async () => {

View File

@ -294,6 +294,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
// Set the initial access token provider back to the original one from options // Set the initial access token provider back to the original one from options
_accessTokenProvider = _httpConnectionOptions.AccessTokenProvider; _accessTokenProvider = _httpConnectionOptions.AccessTokenProvider;
var transportExceptions = new List<Exception>();
if (_httpConnectionOptions.SkipNegotiation) if (_httpConnectionOptions.SkipNegotiation)
{ {
if (_httpConnectionOptions.Transports == HttpTransportType.WebSockets) if (_httpConnectionOptions.Transports == HttpTransportType.WebSockets)
@ -349,12 +351,14 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
if (!Enum.TryParse<HttpTransportType>(transport.Transport, out var transportType)) if (!Enum.TryParse<HttpTransportType>(transport.Transport, out var transportType))
{ {
Log.TransportNotSupported(_logger, transport.Transport); Log.TransportNotSupported(_logger, transport.Transport);
transportExceptions.Add(new TransportFailedException(transport.Transport, "The transport is not supported by the client."));
continue; continue;
} }
if (transportType == HttpTransportType.WebSockets && !IsWebSocketsSupported()) if (transportType == HttpTransportType.WebSockets && !IsWebSocketsSupported())
{ {
Log.WebSocketsNotSupportedByOperatingSystem(_logger); Log.WebSocketsNotSupportedByOperatingSystem(_logger);
transportExceptions.Add(new TransportFailedException("WebSockets", "The transport is not supported on this operating system."));
continue; continue;
} }
@ -363,10 +367,12 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
if ((transportType & _httpConnectionOptions.Transports) == 0) if ((transportType & _httpConnectionOptions.Transports) == 0)
{ {
Log.TransportDisabledByClient(_logger, transportType); Log.TransportDisabledByClient(_logger, transportType);
transportExceptions.Add(new TransportFailedException(transportType.ToString(), "The transport is disabled by the client."));
} }
else if (!transport.TransferFormats.Contains(transferFormatString, StringComparer.Ordinal)) else if (!transport.TransferFormats.Contains(transferFormatString, StringComparer.Ordinal))
{ {
Log.TransportDoesNotSupportTransferFormat(_logger, transportType, transferFormat); Log.TransportDoesNotSupportTransferFormat(_logger, transportType, transferFormat);
transportExceptions.Add(new TransportFailedException(transportType.ToString(), $"The transport does not support the '{transferFormat}' transfer format."));
} }
else else
{ {
@ -385,6 +391,9 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
catch (Exception ex) catch (Exception ex)
{ {
Log.TransportFailed(_logger, transportType, ex); Log.TransportFailed(_logger, transportType, ex);
transportExceptions.Add(new TransportFailedException(transportType.ToString(), ex.Message, ex));
// Try the next transport // Try the next transport
// Clear the negotiation response so we know to re-negotiate. // Clear the negotiation response so we know to re-negotiate.
negotiationResponse = null; negotiationResponse = null;
@ -394,7 +403,14 @@ namespace Microsoft.AspNetCore.Http.Connections.Client
if (_transport == null) if (_transport == null)
{ {
throw new InvalidOperationException("Unable to connect to the server with any of the available transports."); if (transportExceptions.Count > 0)
{
throw new AggregateException("Unable to connect to the server with any of the available transports.", transportExceptions);
}
else
{
throw new NoTransportSupportedException("None of the transports supported by the client are supported by the server.");
}
} }
} }

View File

@ -0,0 +1,18 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
namespace Microsoft.AspNetCore.Http.Connections.Client
{
/// <summary>
/// Exception thrown during negotiate when there are no supported transports between the client and server.
/// </summary>
public class NoTransportSupportedException : Exception
{
public NoTransportSupportedException(string message)
: base(message)
{
}
}
}

View File

@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
namespace Microsoft.AspNetCore.Http.Connections.Client
{
/// <summary>
/// Exception thrown during negotiate when a transport fails to connect.
/// </summary>
public class TransportFailedException : Exception
{
public string TransportType { get; }
public TransportFailedException(string transportType, string message, Exception innerException = null)
: base($"{transportType} failed: {message}", innerException)
{
TransportType = transportType;
}
}
}

View File

@ -154,8 +154,10 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
transport: new TestTransport(onTransportStart: OnTransportStart)), transport: new TestTransport(onTransportStart: OnTransportStart)),
async (connection) => async (connection) =>
{ {
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => connection.StartAsync(TransferFormat.Text)); var ex = await Assert.ThrowsAsync<AggregateException>(() => connection.StartAsync(TransferFormat.Text));
Assert.Equal("Unable to connect to the server with any of the available transports.", ex.Message); Assert.Equal("Unable to connect to the server with any of the available transports. " +
"(WebSockets failed: Transport failed to start) (ServerSentEvents failed: Transport failed to start) (LongPolling failed: Transport failed to start)",
ex.Message);
// If websockets aren't supported then we expect one less attmept to start. // If websockets aren't supported then we expect one less attmept to start.
if (!TestHelpers.IsWebSocketsSupported()) if (!TestHelpers.IsWebSocketsSupported())
@ -343,7 +345,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
CreateConnection(httpHandler, loggerFactory: LoggerFactory, transport: sse), CreateConnection(httpHandler, loggerFactory: LoggerFactory, transport: sse),
async (connection) => async (connection) =>
{ {
await Assert.ThrowsAsync<InvalidOperationException>( await Assert.ThrowsAsync<AggregateException>(
() => connection.StartAsync(TransferFormat.Text).OrTimeout()); () => connection.StartAsync(TransferFormat.Text).OrTimeout());
}); });
} }

View File

@ -37,17 +37,16 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
} }
[Fact] [Fact]
public Task StartThrowsFormatExceptionIfNegotiationResponseHasNoTransports() public Task ConnectionCannotBeStartedIfNoCommonTransportsBetweenClientAndServer()
{ {
return RunInvalidNegotiateResponseTest<InvalidOperationException>(ResponseUtils.CreateNegotiationContent(transportTypes: 0), "Unable to connect to the server with any of the available transports."); return RunInvalidNegotiateResponseTest<AggregateException>(ResponseUtils.CreateNegotiationContent(transportTypes: HttpTransportType.ServerSentEvents),
"Unable to connect to the server with any of the available transports. (ServerSentEvents failed: The transport is disabled by the client.)");
} }
[Theory] [Fact]
[InlineData(HttpTransportType.None)] public Task ConnectionCannotBeStartedIfNoTransportProvidedByServer()
[InlineData(HttpTransportType.ServerSentEvents)]
public Task ConnectionCannotBeStartedIfNoCommonTransportsBetweenClientAndServer(HttpTransportType serverTransports)
{ {
return RunInvalidNegotiateResponseTest<InvalidOperationException>(ResponseUtils.CreateNegotiationContent(transportTypes: serverTransports), "Unable to connect to the server with any of the available transports."); return RunInvalidNegotiateResponseTest<NoTransportSupportedException>(ResponseUtils.CreateNegotiationContent(transportTypes: HttpTransportType.None), "None of the transports supported by the client are supported by the server.");
} }
[Theory] [Theory]