From 0f55ac90621bfd8879d5802315e3ecfc42861fce Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 5 Aug 2025 10:16:53 +0000 Subject: [PATCH] fix: improve Mermaid diagram error handling with helpful suggestions - Add enhanced error messages with specific suggestions for common syntax errors - Detect unclosed brackets, braces, incomplete arrows, and missing diagram types - Add comprehensive test coverage for error handling scenarios - Improve error display with proper whitespace formatting Fixes #6712 --- .../src/components/common/MermaidBlock.tsx | 63 +++- .../common/__tests__/MermaidBlock.spec.tsx | 287 ++++++++++++++++++ 2 files changed, 347 insertions(+), 3 deletions(-) create mode 100644 webview-ui/src/components/common/__tests__/MermaidBlock.spec.tsx diff --git a/webview-ui/src/components/common/MermaidBlock.tsx b/webview-ui/src/components/common/MermaidBlock.tsx index 95c795fdc5..5b61619c74 100644 --- a/webview-ui/src/components/common/MermaidBlock.tsx +++ b/webview-ui/src/components/common/MermaidBlock.tsx @@ -95,6 +95,53 @@ export default function MermaidBlock({ code }: MermaidBlockProps) { const { showCopyFeedback, copyWithFeedback } = useCopyToClipboard() const { t } = useAppTranslation() + // Helper function to enhance error messages with suggestions + const enhanceErrorMessage = (originalError: string, code: string): string => { + let enhancedMessage = originalError + + // Check for common syntax errors + if (originalError.includes("LINK_ID") && originalError.includes("Expecting")) { + // Count brackets to check for unclosed brackets + const openBrackets = (code.match(/\[/g) || []).length + const closeBrackets = (code.match(/\]/g) || []).length + const openBraces = (code.match(/\{/g) || []).length + const closeBraces = (code.match(/\}/g) || []).length + + if (openBrackets > closeBrackets) { + enhancedMessage += + "\n\nSuggestion: You have unclosed square brackets '['. Make sure all node labels are properly closed with ']'." + } + if (openBraces > closeBraces) { + enhancedMessage += + "\n\nSuggestion: You have unclosed curly braces '{'. Make sure all decision nodes are properly closed with '}'." + } + + // Check for incomplete node definitions + if (code.includes("@") && !code.includes("]") && !code.includes("}")) { + enhancedMessage += + "\n\nSuggestion: Node labels containing special characters like '@' should be properly enclosed in brackets." + } + } + + // Check for other common issues + if (originalError.includes("Parse error") && code.trim().endsWith("-->")) { + enhancedMessage += + "\n\nSuggestion: Your diagram appears to end with an arrow '-->'. Make sure to complete the connection with a target node." + } + + if ( + !code.trim().startsWith("graph") && + !code.trim().startsWith("flowchart") && + !code.trim().startsWith("sequenceDiagram") && + !code.trim().startsWith("classDiagram") + ) { + enhancedMessage += + "\n\nSuggestion: Make sure your diagram starts with a valid diagram type (e.g., 'flowchart TD', 'graph LR', 'sequenceDiagram', etc.)." + } + + return enhancedMessage + } + // 1) Whenever `code` changes, mark that we need to re-render a new chart useEffect(() => { setIsLoading(true) @@ -121,7 +168,8 @@ export default function MermaidBlock({ code }: MermaidBlockProps) { }) .catch((err) => { console.warn("Mermaid parse/render failed:", err) - setError(err.message || "Failed to render Mermaid diagram") + const enhancedError = enhanceErrorMessage(err.message || "Failed to render Mermaid diagram", code) + setError(enhancedError) }) .finally(() => { setIsLoading(false) @@ -207,7 +255,12 @@ export default function MermaidBlock({ code }: MermaidBlockProps) { backgroundColor: "var(--vscode-editor-background)", borderTop: "none", }}> -
+
{error}
@@ -216,7 +269,11 @@ export default function MermaidBlock({ code }: MermaidBlockProps) {
) : ( - + )} diff --git a/webview-ui/src/components/common/__tests__/MermaidBlock.spec.tsx b/webview-ui/src/components/common/__tests__/MermaidBlock.spec.tsx new file mode 100644 index 0000000000..0af44d0852 --- /dev/null +++ b/webview-ui/src/components/common/__tests__/MermaidBlock.spec.tsx @@ -0,0 +1,287 @@ +import { render, screen, waitFor } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { vi } from "vitest" +import mermaid from "mermaid" +import MermaidBlock from "../MermaidBlock" + +// Mock mermaid module +vi.mock("mermaid", () => ({ + default: { + initialize: vi.fn(), + parse: vi.fn(), + render: vi.fn(), + }, +})) + +// Mock vscode API +vi.mock("@src/utils/vscode", () => ({ + vscode: { + postMessage: vi.fn(), + }, +})) + +// Mock translation hook +vi.mock("@src/i18n/TranslationContext", () => ({ + useAppTranslation: () => ({ + t: (key: string) => { + const translations: Record = { + "common:mermaid.loading": "Loading diagram...", + "common:mermaid.render_error": "Failed to render diagram", + } + return translations[key] || key + }, + }), +})) + +// Mock clipboard hook +let mockCopyWithFeedback = vi.fn() +vi.mock("@src/utils/clipboard", () => ({ + useCopyToClipboard: () => ({ + showCopyFeedback: false, + copyWithFeedback: mockCopyWithFeedback, + }), +})) + +// Mock CodeBlock component +vi.mock("../CodeBlock", () => ({ + default: ({ source, language }: { source: string; language: string }) => ( +
+ {source} +
+ ), +})) + +// Mock MermaidButton component +vi.mock("@/components/common/MermaidButton", () => ({ + MermaidButton: ({ children }: { children: React.ReactNode }) =>
{children}
, +})) + +// Mock canvas API for SVG to PNG conversion +const mockToDataURL = vi.fn(() => "data:image/png;base64,mockpngdata") +const mockGetContext = vi.fn(() => ({ + fillStyle: "", + fillRect: vi.fn(), + drawImage: vi.fn(), + imageSmoothingEnabled: true, + imageSmoothingQuality: "high", +})) + +HTMLCanvasElement.prototype.toDataURL = mockToDataURL +HTMLCanvasElement.prototype.getContext = mockGetContext as any + +describe("MermaidBlock", () => { + beforeEach(() => { + vi.clearAllMocks() + mockCopyWithFeedback = vi.fn() + mockToDataURL.mockClear() + mockGetContext.mockClear() + }) + + it("renders loading state initially", () => { + vi.mocked(mermaid.parse).mockReturnValue(new Promise(() => {})) // Never resolves + render() + expect(screen.getByText("Loading diagram...")).toBeInTheDocument() + }) + + it("renders mermaid diagram successfully", async () => { + const svgContent = "Test Diagram" + vi.mocked(mermaid.parse).mockResolvedValue({} as any) + vi.mocked(mermaid.render).mockResolvedValue({ svg: svgContent } as any) + + render() + + await waitFor(() => { + const container = screen.getByTestId("svg-container") + expect(container.innerHTML).toBe(svgContent) + }) + }) + + describe("Error handling", () => { + it("displays error message when mermaid parsing fails", async () => { + const errorMessage = "Parse error on line 2: Expecting 'AMP', 'COLON', got 'LINK_ID'" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + }) + + it("shows enhanced error message for unclosed brackets", async () => { + const errorMessage = "Parse error on line 2: Expecting 'AMP', 'COLON', got 'LINK_ID'" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + + // Click to expand error + const errorHeader = screen.getByText("Failed to render diagram").parentElement + await userEvent.click(errorHeader!) + + await waitFor(() => { + const errorDetails = screen.getByText(/You have unclosed square brackets/) + expect(errorDetails).toBeInTheDocument() + }) + }) + + it("shows enhanced error message for unclosed braces", async () => { + const errorMessage = "Parse error on line 2: Expecting 'AMP', 'COLON', got 'LINK_ID'" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + + // Click to expand error + const errorHeader = screen.getByText("Failed to render diagram").parentElement + await userEvent.click(errorHeader!) + + await waitFor(() => { + const errorDetails = screen.getByText(/You have unclosed curly braces/) + expect(errorDetails).toBeInTheDocument() + }) + }) + + it("shows suggestion for incomplete arrow connections", async () => { + const errorMessage = "Parse error at end of input" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + + // Click to expand error + const errorHeader = screen.getByText("Failed to render diagram").parentElement + await userEvent.click(errorHeader!) + + await waitFor(() => { + const errorDetails = screen.getByText(/Your diagram appears to end with an arrow/) + expect(errorDetails).toBeInTheDocument() + }) + }) + + it("shows suggestion for missing diagram type", async () => { + const errorMessage = "Parse error on line 1" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + + // Click to expand error + const errorHeader = screen.getByText("Failed to render diagram").parentElement + await userEvent.click(errorHeader!) + + await waitFor(() => { + const errorDetails = screen.getByText(/Make sure your diagram starts with a valid diagram type/) + expect(errorDetails).toBeInTheDocument() + }) + }) + + it("shows code block when error is expanded", async () => { + const code = "flowchart TD A[Incomplete" + const errorMessage = "Parse error" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + + // Click to expand error + const errorHeader = screen.getByText("Failed to render diagram").parentElement + await userEvent.click(errorHeader!) + + await waitFor(() => { + const codeBlock = screen.getByTestId("code-block") + expect(codeBlock).toBeInTheDocument() + expect(codeBlock).toHaveAttribute("data-language", "mermaid") + expect(codeBlock).toHaveTextContent(code) + }) + }) + + it("allows copying error message and code", async () => { + const code = "flowchart TD A[Incomplete" + const errorMessage = "Parse error" + vi.mocked(mermaid.parse).mockRejectedValue(new Error(errorMessage)) + + render() + + await waitFor(() => { + expect(screen.getByText("Failed to render diagram")).toBeInTheDocument() + }) + + // Find and click copy button + const copyButton = screen.getByRole("button") + await userEvent.click(copyButton) + + expect(mockCopyWithFeedback).toHaveBeenCalledWith( + expect.stringContaining(`Error: ${errorMessage}`), + expect.any(Object), + ) + expect(mockCopyWithFeedback).toHaveBeenCalledWith(expect.stringContaining("```mermaid"), expect.any(Object)) + }) + }) + + it("renders mermaid diagram and allows interaction", async () => { + const svgContent = '' + vi.mocked(mermaid.parse).mockResolvedValue({} as any) + vi.mocked(mermaid.render).mockResolvedValue({ svg: svgContent } as any) + + render() + + await waitFor(() => { + const container = screen.getByTestId("svg-container") + expect(container.innerHTML).toBe(svgContent) + }) + + // Verify the SVG container is clickable + const svgContainer = screen.getByTestId("svg-container") + expect(svgContainer).toBeInTheDocument() + expect(svgContainer).toHaveStyle({ cursor: "pointer" }) + }) + + it("debounces diagram rendering", async () => { + const { rerender } = render() + + // Initial parse call + expect(mermaid.parse).toHaveBeenCalledTimes(0) + + // Wait for debounce + await waitFor( + () => { + expect(mermaid.parse).toHaveBeenCalledTimes(1) + }, + { timeout: 600 }, + ) + + // Quick re-renders should not trigger immediate parse + rerender() + rerender() + + // Should still be 1 call + expect(mermaid.parse).toHaveBeenCalledTimes(1) + + // Wait for new debounce + await waitFor( + () => { + expect(mermaid.parse).toHaveBeenCalledTimes(2) + }, + { timeout: 600 }, + ) + + // Should parse the latest code + expect(mermaid.parse).toHaveBeenLastCalledWith("flowchart TD\\n A --> D") + }) +})