Skip to content

Blazor Disable Non-WebSockets Transports by Default #34644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jul 27, 2021
Merged

Blazor Disable Non-WebSockets Transports by Default #34644

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c4b3a17
Blazor Disable Non-WebSockets Transports by Default
TanayParikh Jul 23, 2021
fab2c82
Update dist .js files
TanayParikh Jul 23, 2021
e03d084
Added UnsupportedTransportWebSocketsError
TanayParikh Jul 23, 2021
f1dbafc
Merge branch 'main' into taparik/disableNonWSTransportsByDefault
TanayParikh Jul 23, 2021
895afb2
Added FailedToStartTransportError
TanayParikh Jul 23, 2021
ce0ba7d
E2E Tests
TanayParikh Jul 25, 2021
b2dc979
Updated async awaits
TanayParikh Jul 26, 2021
dfd0128
Remove ErrorIfBrowserDoesNotSupportWebSockets
TanayParikh Jul 26, 2021
3bb93eb
MultipleErrors Approach
TanayParikh Jul 26, 2021
1c3e8f6
Always show error notification if `connection.start` throws
TanayParikh Jul 26, 2021
221503a
Merge branch 'main' into taparik/disableNonWSTransportsByDefault
TanayParikh Jul 26, 2021
b55777f
PR Feedback
TanayParikh Jul 26, 2021
61a27b7
Fix transport failed exception handling
TanayParikh Jul 26, 2021
6b6c71e
TransportsServerStartup && ErrorIfClientAttemptsWebSocketsWithServerO…
TanayParikh Jul 26, 2021
5f8b828
Fix Build
TanayParikh Jul 26, 2021
0668383
Update src/Components/test/testassets/TestServer/Pages/Transports.cshtml
TanayParikh Jul 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Components/Server/src/Builder/ComponentEndpointRouteBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ public static ComponentEndpointConventionBuilder MapBlazorHub(
throw new ArgumentNullException(nameof(path));
}

return endpoints.MapBlazorHub(path, configureOptions: _ => { });
// Only support the WebSockets transport type by default
return endpoints.MapBlazorHub(path, configureOptions: options => { options.Transports = HttpTransportType.WebSockets; });
}

/// <summary>
Expand Down
20 changes: 18 additions & 2 deletions src/Components/Web.JS/src/Boot.Server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { DotNet } from '@microsoft/dotnet-js-interop';
import { Blazor } from './GlobalExports';
import { HubConnectionBuilder, HubConnection } from '@microsoft/signalr';
import { HubConnectionBuilder, HubConnection, HttpTransportType } from '@microsoft/signalr';
import { MessagePackHubProtocol } from '@microsoft/signalr-protocol-msgpack';
import { showErrorNotification } from './BootErrors';
import { shouldAutoStart } from './BootCommon';
Expand Down Expand Up @@ -85,7 +85,7 @@ async function initializeConnection(options: CircuitStartOptions, logger: Logger
(hubProtocol as unknown as { name: string }).name = 'blazorpack';

const connectionBuilder = new HubConnectionBuilder()
.withUrl('_blazor')
.withUrl('_blazor', HttpTransportType.WebSockets)
.withHubProtocol(hubProtocol);

options.configureSignalR(connectionBuilder);
Expand Down Expand Up @@ -130,6 +130,7 @@ async function initializeConnection(options: CircuitStartOptions, logger: Logger
await connection.start();
} catch (ex) {
unhandledError(connection, ex, logger);
showUnavailableTransportErrorMessage(ex);
}

DotNet.attachDispatcher({
Expand Down Expand Up @@ -163,3 +164,18 @@ Blazor.start = boot;
if (shouldAutoStart()) {
boot();
}

function showUnavailableTransportErrorMessage(ex: Error) {
const unableToConnectTransportMessage = "Unable to connect to the server with any of the available transports.";
if (!ex.message.startsWith(unableToConnectTransportMessage)) {
return;
}

if (ex.message.includes(`'WebSockets' is not supported in your environment.`)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we be able to change SignalR to throw a type and use that here instead of string sniffing: https://developer.mozilla.org/en-US/docs/web/javascript/reference/statements/throw#throw_an_object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc/ @BrennanConroy would you be open to this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against throwing a specific error type at

aspnetcore/src/SignalR/clients/ts/signalr/src/HttpConnection.ts

Line 408 in 3eb0fa8

return Promise.reject(new Error(`Unable to connect to the server with any of the available transports. ${transportExceptions.join(" ")}`));

Much better than doing string matching since those can change at any time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added a Error.name here:

e03d084#diff-757d83bcf707b34888c824c952e4717638803af7b5372f6c5da1eb838f1a5814R450-R452

Due to the way exception nesting is done in the HttpConnection, having the name itself propagate up would likely require a lot more extensive changes. Instead, I'm still relying a bit on string sniffing, but I'm sniffing the custom Error.name instead of the message.

e03d084#diff-7c47c684c78e3f5b5eeecb28b249b03afcefcd90a925cc20b69ec2cdec261752R134

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was wrong with changing the type of exception we throw?

Copy link
Contributor Author

@TanayParikh TanayParikh Jul 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're referring to here:

aspnetcore/src/SignalR/clients/ts/signalr/src/HttpConnection.ts

Line 408 in 3eb0fa8

return Promise.reject(new Error(`Unable to connect to the server with any of the available transports. ${transportExceptions.join(" ")}`));

Several different issues can result in an exception being thrown there. So changing the exception type being thrown there wouldn't provide us with specific enough information (ie. diagnose a WS failed connection).

If it's for changing the exception type here instead of the name:
e03d084#diff-757d83bcf707b34888c824c952e4717638803af7b5372f6c5da1eb838f1a5814R451

I just wanted to avoid adding an additional exception type when the spec compliant Error.name would be sufficient for our purposes. Let me know if you feel otherwise and I can add it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated 895afb2.

showErrorNotification('Unable to connect, please ensure you are using an updated browser and WebSockets are available.');
} if (ex.message.includes(`'${HttpTransportType[HttpTransportType.WebSockets]}' is disabled by the client.`)) {
showErrorNotification('Unable to connect, please enable WebSockets on the client.');
} else if (ex.message.includes(`'${HttpTransportType[HttpTransportType.LongPolling]}' is disabled by the client.`)) {
showErrorNotification('Unable to connect, please enable LongPolling on the client.');
}
}
6 changes: 5 additions & 1 deletion src/Components/Web.JS/src/BootErrors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
let hasFailed = false;

export async function showErrorNotification() {
export async function showErrorNotification(customErrorMessage: string = '') {
let errorUi = document.querySelector('#blazor-error-ui') as HTMLElement;
if (errorUi) {
errorUi.style.display = 'block';

if (customErrorMessage && errorUi.firstChild) {
errorUi.firstChild.textContent = `\n\t${customErrorMessage}\t\n`;
}
}

if (!hasFailed) {
Expand Down