Handle parsing exceptions on BitMarkdownViewer initialization (#11008) #11018
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe BitMarkdownViewer component was refactored to centralize markdown parsing logic within a new helper method, ParseAndRender, which includes error handling to catch exceptions during parsing. The OnMarkdownSet method was changed to a synchronous void method that triggers ParseAndRender asynchronously, improving robustness against script loading failures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BitMarkdownViewer
participant MarkdownService
User->>BitMarkdownViewer: Sets Markdown content
BitMarkdownViewer->>BitMarkdownViewer: OnMarkdownSet() (sync)
BitMarkdownViewer->>BitMarkdownViewer: ParseAndRender() (async)
alt Markdown parsing succeeds
BitMarkdownViewer->>MarkdownService: Parse(markdown)
MarkdownService-->>BitMarkdownViewer: Parsed HTML
BitMarkdownViewer->>BitMarkdownViewer: StateHasChanged()
else Exception occurs
BitMarkdownViewer->>BitMarkdownViewer: Set HTML to error message
BitMarkdownViewer->>BitMarkdownViewer: StateHasChanged()
end
Assessment against linked issues
Poem
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownViewer/BitMarkdownViewer.razor.cs (2)
38-42: Consider the implications of the fire-and-forget async pattern.The change from
async Tasktovoidwith fire-and-forget (_ = ParseAndRender()) introduces potential risks:
- Unhandled exceptions: If
StateHasChanged()throws an exception withinParseAndRender(), it won't be caught by the caller- Inconsistent patterns:
OnInitializedAsyncawaitsParseAndRender()while this method uses fire-and-forgetWhile this approach is understandable for parameter setters (to avoid blocking), consider wrapping the fire-and-forget call in additional error handling:
private void OnMarkdownSet() { if (IsRendered is false) return; - _ = ParseAndRender(); + _ = Task.Run(async () => + { + try + { + await ParseAndRender(); + } + catch + { + // Log or handle unhandled exceptions from ParseAndRender + } + }); }
45-57: Improve error handling specificity and logging.The centralized parsing logic with error handling addresses the PR objective well. However, consider these improvements:
- Generic catch: The bare
catchblock hides important error information that could help with debugging- Error message: The hardcoded HTML error message could be more informative
private async Task ParseAndRender() { try { _html = await _markdownService.Parse(Markdown, _cts.Token); } - catch + catch (Exception ex) { + // Log the actual exception for debugging + // _logger?.LogError(ex, "Failed to parse markdown content"); _html = "<b>Failed to parse the markdown!</b>"; } StateHasChanged(); }The
StateHasChanged()placement after both success and failure paths is correct for ensuring UI updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownViewer/BitMarkdownViewer.razor.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (2)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MarkdownViewer/BitMarkdownViewer.razor.cs (2)
31-31: LGTM: Centralized parsing logic.Good refactoring to use the centralized
ParseAndRendermethod. Theawaitis appropriate in the initialization context.
29-57: Overall assessment: Changes successfully address the PR objective.The refactoring effectively centralizes markdown parsing logic and adds the requested exception handling for initialization issues. Key improvements:
✅ Centralized logic:
ParseAndRendermethod consolidates parsing behavior
✅ Error handling: Try-catch prevents crashes during parsing failures
✅ UI consistency:StateHasChanged()ensures UI updates in all scenarios
✅ Resource management: Proper use of cancellation tokenThe changes maintain existing functionality while improving robustness against parsing failures, successfully closing issue #11008.
msynk
changed the title
msynk
deleted the
11008-blazorui-markdownviewer-unhandled-failed-script-initialization
branch
1 participant
closes #11008
Summary by CodeRabbit
Bug Fixes
Refactor