Improve Blazor reconnection experience after the server is restarted#8
Conversation
…flect change in ResumeCircuit
WalkthroughThe changes modify circuit reconnection behavior when persisted state is unavailable. Instead of notifying the client and aborting the connection, the server now logs the condition and returns null. Client-side reconnection and display logic adapts to handle resume failures without forcing page reloads, delegating recovery to client reconnection mechanisms. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ReconnHandler as Reconnection<br/>Handler
participant Server as ComponentHub
participant Display as Reconnect<br/>Display
Client->>ReconnHandler: Reconnect attempt
ReconnHandler->>Server: Resume circuit
Server-->>ReconnHandler: Returns null<br/>(no state available)
ReconnHandler->>Display: Handle resume failure
Display->>Display: Update UI state<br/>(show resume button)
Display-->>Client: Display resume/retry option<br/>(no page reload)
Client->>ReconnHandler: User clicks resume
ReconnHandler->>Server: Retry resume
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/Components/test/E2ETest/ServerExecutionTests/ServerReconnectionWithoutStateTest.cs`:
- Around line 61-64: The staleness check currently calls the extension
property/method IsStale directly which won't compile; change the wait assertion
to pass a lambda that evaluates initialElement.IsStale (e.g., Browser.True(() =>
initialElement.IsStale)) so the check is deferred and executed inside the wait;
locate the assertion using Browser.True and initialElement.IsStale in
ServerReconnectionWithoutStateTest and wrap the IsStale invocation in a lambda
expression.
🧹 Nitpick comments (1)
src/Components/Server/src/ComponentHub.cs (1)
319-323: Consider a more specific log for expected expired/evicted state.
InvalidInputDatamight read as client fault even though this is an expected expiration path. A dedicated log message could reduce noise.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/Components/Server/src/ComponentHub.cssrc/Components/Server/test/Circuits/ComponentHubTest.cssrc/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.tssrc/Components/Web.JS/src/Platform/Circuits/DefaultReconnectionHandler.tssrc/Components/Web.JS/src/Platform/Circuits/UserSpecifiedDisplay.tssrc/Components/test/E2ETest/ServerExecutionTests/ServerReconnectionWithoutStateTest.cssrc/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cssrc/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Layout/ReconnectModal.razorsrc/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Layout/ReconnectModal.razor.js
💤 Files with no reviewable changes (2)
- src/Components/Server/test/Circuits/ComponentHubTest.cs
- src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectionHandler.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/Components/test/E2ETest/ServerExecutionTests/ServerReconnectionWithoutStateTest.cs (3)
src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs (2)
RazorComponentEndpointsStartup(22-309)RazorComponentEndpointsStartup(24-27)src/Shared/E2ETesting/WaitAssert.cs (1)
Click(83-87)src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/WebDriverExtensions.cs (1)
IsStale(122-133)
🪛 ast-grep (0.40.5)
src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.ts
[warning] 135-135: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.status.innerHTML = 'Failed to rejoin.
Please retry or reload the page.'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 138-138: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.status.innerHTML = 'Failed to resume the session.
Please retry or reload the page.'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 135-135: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.status.innerHTML = 'Failed to rejoin.
Please retry or reload the page.'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 138-138: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.status.innerHTML = 'Failed to resume the session.
Please retry or reload the page.'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (14)
src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Layout/ReconnectModal.razor.js (1)
55-55: Good shift to resume-failed UI state.
Avoids an unnecessary reload and keeps the user in the retry flow.src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Components/Layout/ReconnectModal.razor (1)
29-33: UI copy and visibility updates look consistent.
The resume-failed messaging and button visibility match the new flow.src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs (1)
54-61: Clear test-only switch for disabling circuit persistence.
The new configuration branch is straightforward and matches the test scenarios.src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.ts (2)
99-112: Button visibility resets are consistent.
Hiding resume/reload during reconnect avoids stale controls.
134-140: Failure messaging/visibility split looks correct.
Resume vs reconnect failure states are clearly distinguished.src/Components/Web.JS/src/Platform/Circuits/UserSpecifiedDisplay.ts (3)
30-33: Reconnect/remote flags are now explicit.
Clear defaults make the state visible to custom displays.
74-79: Pause event now carries remote context.
Good propagation for UI state updates.
88-96: Failure events include remote metadata.
Helps custom UI tailor messaging consistently.src/Components/test/E2ETest/ServerExecutionTests/ServerReconnectionWithoutStateTest.cs (6)
23-31: Fixture flags align with the no‑state scenario.
Disabling reconnection cache and circuit persistence matches the test’s intent.
33-37: Init navigation and interactivity check look good.
39-41: Configurable defaults are handy for derived tests.
73-102: Covers persistent vs non‑persistent counters across reconnects.
104-148: UI flow assertions are clear and reusable.
151-168: Custom UI variant setup is clear.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Check for page reload using multiple conditions: | ||
| // 1. Previously captured element is stale | ||
| Browser.True(initialElement.IsStale); | ||
| // 2. Counter state is reset |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/Components/test/E2ETest/ServerExecutionTests/ServerReconnectionWithoutStateTest.cs | head -80 | tail -30Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 1836
🏁 Script executed:
rg "IsStale" --type cs -B 2 -A 2 | head -50Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 5699
🏁 Script executed:
rg "Browser\.True" --type cs -B 1 -A 1 | head -40Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 3292
🏁 Script executed:
rg "static.*IsStale|extension.*IsStale" --type cs -B 2 -A 5Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 929
🏁 Script executed:
rg "class Browser|namespace.*Browser" --type cs | head -20Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 2112
🏁 Script executed:
fd "Browser.cs" --type fRepository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 71
🏁 Script executed:
fd "BrowserWait.cs|BrowserExtensions.cs" --type fRepository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 149
🏁 Script executed:
rg "Browser\.True|void True|bool True" --type cs -B 2 -A 3 | grep -A 3 "True.*Func\|True.*Action" | head -40Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 1255
🏁 Script executed:
rg "static.*True\(" --type cs -B 1 -A 4 | head -60Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 2347
🏁 Script executed:
rg "class.*Browser|namespace.*BrowserWait" --type cs | grep -v "BrowserFile\|BrowserTab\|IBrowserFile\|BrowserTest" | head -20Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 2873
🏁 Script executed:
find . -type f -name "*.cs" -path "*/test/*" -o -name "*.cs" -path "*/E2ETest/*" | xargs grep -l "Browser\.True" | head -5 | xargs grep -B 5 "static.*Browser" | head -40Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 395
🏁 Script executed:
rg "public.*True" src/Shared/E2ETesting/ --type cs -B 2 -A 3Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 1406
🏁 Script executed:
rg "public static.*True|public.*True\(" src/Shared/BrowserTesting/src/ --type cs -B 1 -A 5Repository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 71
Fix staleness check invocation.
IsStale is an extension method, so the current usage won't compile; wrap it in a lambda to keep the wait behavior.
🐛 Proposed fix
- Browser.True(initialElement.IsStale);
+ Browser.True(() => initialElement.IsStale());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for page reload using multiple conditions: | |
| // 1. Previously captured element is stale | |
| Browser.True(initialElement.IsStale); | |
| // 2. Counter state is reset | |
| // Check for page reload using multiple conditions: | |
| // 1. Previously captured element is stale | |
| Browser.True(() => initialElement.IsStale()); | |
| // 2. Counter state is reset |
🤖 Prompt for AI Agents
In
`@src/Components/test/E2ETest/ServerExecutionTests/ServerReconnectionWithoutStateTest.cs`
around lines 61 - 64, The staleness check currently calls the extension
property/method IsStale directly which won't compile; change the wait assertion
to pass a lambda that evaluates initialElement.IsStale (e.g., Browser.True(() =>
initialElement.IsStale)) so the check is deferred and executed inside the wait;
locate the assertion using Browser.True and initialElement.IsStale in
ServerReconnectionWithoutStateTest and wrap the IsStale invocation in a lambda
expression.
2 participants
Benchmark PR from qodo-benchmark#32
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.