Skip to content

Conversation

@TanayParikh
Copy link
Contributor

@TanayParikh TanayParikh commented Sep 20, 2021

We reverted enforcing WebSocket based transports via #36656.

This PR adds back some of those error messages, along with a new error message specifically mentioning that Long Polling is being utilized.

TODO: add E2E tests.

Fixes: #36817

@TanayParikh TanayParikh requested a review from a team as a code owner September 20, 2021 21:31
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Sep 20, 2021

// Check if the connection is established using the long polling transport,
// using the `features.inherentKeepAlive` property only present with long polling.
if ((connection as any).connection.features.inherentKeepAlive) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spoke with @BrennanConroy regarding best means to identify if a connection is Long Polling. He suggested relying on this property implicitly (instead of explicitly adding a marker to the SignalR HttpTransport). Will be adding a E2E test that validates this behavior.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should log a warning, not an error, since the app will continue working.

// Check if the connection is established using the long polling transport,
// using the `features.inherentKeepAlive` property only present with long polling.
if ((connection as any).connection.features.inherentKeepAlive) {
logger.log(LogLevel.Error, 'Failed to connect via WebSockets, using the Long Polling fallback transport. This may be due to a VPN or proxy blocking the connection. To troubleshoot this, visit https://aka.ms/blazor-server-using-fallback-long-polling.');
Copy link
Contributor Author

@TanayParikh TanayParikh Sep 20, 2021

Choose a reason for hiding this comment

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

https://aka.ms/blazor-server-using-fallback-long-polling will point to a new docs page which discusses common pitfalls and troubleshooting steps to ensure WebSockets are utilized (ie. enabled on IIS, enabled on azure portal, etc.). May just end up extending https://docs.microsoft.com/en-us/aspnet/core/blazor/host-and-deploy/server?view=aspnetcore-6.0#signalr-configuration.

@TanayParikh
Copy link
Contributor Author

Regarding the server-side logging of long polling, SignalR already takes care of this for us via;

	 dbug: Microsoft.AspNetCore.Http.Connections.Internal.Transports.LongPollingTransport[4]
      Client disconnected from Long Polling endpoint for connection.

Other than that, there isn't an explicit way for us to recognize the connection type in the hub, but I believe this may be sufficient for tracking purposes.

@TanayParikh
Copy link
Contributor Author

📆 with E2E tests, should be good to merge now.

@TanayParikh TanayParikh changed the title Blazor Server Add Console Error if Long Polling Blazor Server Add Console Warning if Long Polling Sep 23, 2021
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Looks great!

@TanayParikh TanayParikh merged commit c17f75d into main Sep 23, 2021
@TanayParikh TanayParikh deleted the taparik/improveLoggingOnLongPollingFallback branch September 23, 2021 15:54
@ghost ghost added this to the 7.0-preview1 milestone Sep 23, 2021
@TanayParikh
Copy link
Contributor Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1266548760

@github-actions
Copy link
Contributor

@TanayParikh an error occurred while backporting to release/6.0, please check the run log for details!

Error: @TanayParikh is not a repo collaborator, backporting is not allowed.

TanayParikh added a commit that referenced this pull request Sep 23, 2021
* Blazor Server Add Console Error if Long Polling

* E2E Testing

* Fix tests

* Update Boot.Server.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Blazor Server Logging for the Long Polling Transport

3 participants