Skip to content

Added configureConnection delegate to CircuitStartOptions #44623

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.

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

Closed
wants to merge 1 commit into from

Conversation

surayya-MS
Copy link
Member

@surayya-MS surayya-MS commented Oct 18, 2022

Added configureConnection delegate to CircuitStartOptions

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Description

Fixes #18840 by enabling user to configure HubConnection object like this:

<script>
        Blazor.start({
            configureConnection: function (connection) {
                connection.serverTimeoutInMilliseconds = 120000;
            }
        });
</script>

@surayya-MS surayya-MS requested a review from a team as a code owner October 18, 2022 15:37
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Oct 18, 2022
@surayya-MS
Copy link
Member Author

surayya-MS commented Oct 18, 2022

No automated tests for this. Tested manually:

  1. Add configureConnection to the script in _Layout.cshtml inside BlazorServerApp sample:
<script>
        Blazor.start({
            logLevel: 1, // LogLevel.Debug
            configureSignalR: builder => builder.configureLogging("debug"), // LogLevel.Debug
            configureConnection: function (connection) {
                connection.serverTimeoutInMilliseconds = 120000;
            }
        });
</script>
  1. Start debugging with breakpoint on IncrementCount method in Pages/Counter.razor, click the increment button on Counter page and wait

Result: 'Error: Server timeout elapsed without receiving a message from the server.' appeared in 2 minutes.

@surayya-MS surayya-MS requested a review from javiercn October 18, 2022 15:57
@TanayParikh
Copy link
Contributor

I'm probably missing some context here, so apologies if this has already been discussed. Given we can already configure the server timeout as follows:

<script>
  Blazor.start({
    configureSignalR: function (builder) {
      builder.serverTimeoutInMilliseconds = 30000;
      builder.keepAliveIntervalInMilliseconds = 15000;
    }
});
</script>

As we have documented here. What's the motivation to provide a separate configureConnection method? Is it to potentially abstract away the concept of the builder object / SignalR?

@javiercn
Copy link
Member

javiercn commented Oct 18, 2022 via email

@javiercn
Copy link
Member

javiercn commented Oct 18, 2022 via email

@TanayParikh
Copy link
Contributor

Is the HTTP timeout the same thing?

builder.httpConnectionOptions.timeout may be useful?

cc/ @guardrex

@guardrex
Copy link
Contributor

guardrex commented Oct 18, 2022

Seemed ok per Brennan's feedback at ...

dotnet/AspNetCore.Docs#27197 (comment)

He didn't remark directly on it, but it was in the preceding comment to show what I was placing in the doc.

I'm OOF this evening, but I'll work it first thing Wednesday morning and sort it out.

@guardrex
Copy link
Contributor

Am I looking at the wrong API? These are here, unless this is totally the wrong thing ...

https://github.com/dotnet/aspnetcore/blob/main/src/SignalR/clients/ts/signalr/src/HubConnection.ts#L74-L88

@TanayParikh
Copy link
Contributor

TanayParikh commented Oct 18, 2022

I believe that one you linked is the HubConnection, whereas configureSignalR provides the HubConnectionBuilder. If it works, that's great, however the above distinction probably means the API isn't available as it's currently presented.

@guardrex
Copy link
Contributor

I see. Ok ... I think Brennan was perhaps looking at the text and didn't spot my code problem. I have time first thing in the morning to fix it up. Now tracked by ...

Foul SignalR code example fix needed
dotnet/AspNetCore.Docs#27320

@surayya-MS
Copy link
Member Author

I'm probably missing some context here, so apologies if this has already been discussed. Given we can already configure the server timeout as follows:

<script>
  Blazor.start({
    configureSignalR: function (builder) {
      builder.serverTimeoutInMilliseconds = 30000;
      builder.keepAliveIntervalInMilliseconds = 15000;
    }
});
</script>

As we have documented here. What's the motivation to provide a separate configureConnection method? Is it to potentially abstract away the concept of the builder object / SignalR?

I tried to set serverTimeoutInMilliseconds through configuresignalR method. It doesn't work. We need to fix the docs

@guardrex
Copy link
Contributor

guardrex commented Oct 19, 2022

I'm working on that right now, @surayya-MS, on the cross-linked docs issue.

UPDATE: I ran into some unrelated client-side logging problems that I'm also asking about on the docs issue. This aspect is resolved now, and I'll add an update to the PR on this side-scenario.

@surayya-MS ...

  • Nevermind on the question that I just posted to you earlier for bullet 1 here. What I've done to temporarily address the doc problem is comment-out the incorrect guidance until Brennan and I can chat about what the correct approaches are. The text was always correct on what the settings should be. The problems are that (a) for 7.0 and earlier, we need client-side config for serverTimeoutInMilliseconds and keepAliveIntervalInMilliseconds for Blazor Server, and (b) for hosted Blazor WebAssembly ... idk even what a workaround would be for the client at the moment. We'll discuss it and figure it out on the docs issue.
  • The only approach I see at the moment is for clients of Blazor Server apps posted as a workaround at Customizable serverTimeoutInMilliseconds in Server-Side Blazor #18840 (comment).

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.

Customizable serverTimeoutInMilliseconds in Server-Side Blazor
4 participants