diff --git a/frontend/src/components/editor/boundary/ErrorBoundary.tsx b/frontend/src/components/editor/boundary/ErrorBoundary.tsx index 96d6c4a0742..ca094a6f106 100644 --- a/frontend/src/components/editor/boundary/ErrorBoundary.tsx +++ b/frontend/src/components/editor/boundary/ErrorBoundary.tsx @@ -1,11 +1,21 @@ /* Copyright 2026 Marimo. All rights reserved. */ -import type { PropsWithChildren } from "react"; +import { Provider } from "jotai"; +import { type PropsWithChildren, lazy, Suspense, useState } from "react"; import { type FallbackProps, ErrorBoundary as ReactErrorBoundary, } from "react-error-boundary"; -import { Constants } from "@/core/constants"; +import { store } from "@/core/state/jotai"; import { Button } from "../../ui/button"; +import { Dialog, DialogTrigger } from "../../ui/dialog"; +import { Toaster } from "../../ui/toaster"; +import { TooltipProvider } from "../../ui/tooltip"; + +const FeedbackModal = lazy(() => + import("../chrome/components/feedback-button").then((mod) => ({ + default: mod.FeedbackModal, + })), +); export const ErrorBoundary: React.FC = (props) => { return ( @@ -16,26 +26,38 @@ export const ErrorBoundary: React.FC = (props) => { }; const FallbackComponent: React.FC = (props) => { + const [open, setOpen] = useState(false); + return ( -
+

Something went wrong

         {props.error?.message}
       
-
- If this is an issue with marimo, please report it on{" "} - - GitHub - - . +
+ + + + + {open && ( + + + + + + + + + )} + +
-
); }; diff --git a/frontend/src/components/editor/boundary/__tests__/ErrorBoundary.test.tsx b/frontend/src/components/editor/boundary/__tests__/ErrorBoundary.test.tsx new file mode 100644 index 00000000000..f98025b9d57 --- /dev/null +++ b/frontend/src/components/editor/boundary/__tests__/ErrorBoundary.test.tsx @@ -0,0 +1,311 @@ +/* Copyright 2026 Marimo. All rights reserved. */ + +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import type { PropsWithChildren, ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { MockRequestClient } from "@/__mocks__/requests"; +import { ModalProvider } from "@/components/modal/ImperativeModal"; +import { TooltipProvider } from "@/components/ui/tooltip"; +import { clearToasts } from "@/components/ui/use-toast"; +import { initialNotebookState, notebookAtom } from "@/core/cells/cells"; +import { viewStateAtom } from "@/core/mode"; +import { connectionAtom } from "@/core/network/connection"; +import { requestClientAtom } from "@/core/network/requests"; +import type { EnvironmentInfo } from "@/core/network/types"; +import { filenameAtom } from "@/core/saving/file-state"; +import { store } from "@/core/state/jotai"; +import { WebSocketState } from "@/core/websocket/types"; +import { ErrorBoundary } from "../ErrorBoundary"; + +vi.mock("@/utils/copy", () => ({ + copyToClipboard: vi.fn().mockResolvedValue(undefined), +})); + +const environment: EnvironmentInfo = { + marimo: "1.2.3", + editable: false, + location: "~/.venv/site-packages/marimo", + OS: "Darwin", + "OS Version": "25.0", + Processor: "arm", + "Python Version": "3.12.9", + Locale: "en_US", + Binaries: { Browser: "chrome 140", Node: "v22", uv: "0.11" }, + Dependencies: { click: "8.4.2" }, + "Optional Dependencies": { pandas: "3.0.0" }, + "Experimental Flags": {}, +}; + +const CRASH_MESSAGE = "cell output crashed"; + +function CrashingChild(): never { + throw new Error(CRASH_MESSAGE); +} + +function RecoverableChild({ crash }: { crash: boolean }) { + if (crash) { + throw new Error(CRASH_MESSAGE); + } + return

recovered child

; +} + +function DescendantProviders({ children }: PropsWithChildren) { + return ( + + {children} + + ); +} + +function isExpectedReactExceptionOutput(args: unknown[]): boolean { + return args.some((arg) => { + if (arg instanceof Error) { + return arg.message.includes(CRASH_MESSAGE); + } + return ( + typeof arg === "string" && + (arg.includes(CRASH_MESSAGE) || + arg.includes("The above error occurred") || + arg.includes("React will try to recreate this component tree")) + ); + }); +} + +function suppressExpectedReactException() { + return vi.spyOn(console, "error").mockImplementation((...args) => { + if (isExpectedReactExceptionOutput(args)) { + return; + } + throw new Error(`Unexpected console.error: ${args.map(String).join(" ")}`); + }); +} + +let consoleError: ReturnType | undefined; + +function renderCrashedBoundary(child: ReactNode = ) { + consoleError?.mockRestore(); + consoleError = suppressExpectedReactException(); + return render({child}); +} + +function setRequestClient( + overrides?: Parameters[0], +) { + const client = MockRequestClient.create(overrides); + store.set(requestClientAtom, client); + return client; +} + +async function openReportDialog() { + const trigger = screen.getByRole("button", { name: "Report an issue" }); + trigger.focus(); + fireEvent.click(trigger); + return screen.findByRole("dialog"); +} + +async function expectReportButtonFocused() { + await waitFor(() => { + expect( + screen.getByRole("button", { name: "Report an issue" }), + ).toHaveFocus(); + }); +} + +async function closeReportDialog(method: "escape" | "close") { + const dialog = await screen.findByRole("dialog"); + if (method === "escape") { + fireEvent.keyDown(dialog, { key: "Escape" }); + } else { + fireEvent.click(screen.getByRole("button", { name: "Close" })); + } + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + await expectReportButtonFocused(); +} + +async function expectRealReportModal() { + const dialog = await openReportDialog(); + expect(dialog).toHaveTextContent("Report an issue"); + expect(screen.getByRole("link", { name: "Open GitHub issue" })).toBeVisible(); + expect( + screen.getByRole("checkbox", { name: "Include errors" }), + ).toBeVisible(); + expect( + screen.getByRole("checkbox", { name: "Include notebook code" }), + ).toBeVisible(); + await screen.findByText("Environment details"); + return dialog; +} + +function resetSharedState() { + localStorage.clear(); + clearToasts(); + store.set(requestClientAtom, null); + store.set(notebookAtom, initialNotebookState()); + store.set(viewStateAtom, { mode: "edit", cellAnchor: null }); + store.set(connectionAtom, { state: WebSocketState.OPEN }); + store.set(filenameAtom, "/project/example.py"); +} + +describe("ErrorBoundary report dialog", () => { + beforeEach(() => { + resetSharedState(); + }); + + afterEach(() => { + consoleError?.mockRestore(); + consoleError = undefined; + resetSharedState(); + }); + + it("opens the report modal after a child render exception", async () => { + const { getEnvironmentInfo } = setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + + renderCrashedBoundary(); + + expect(screen.getByText("Something went wrong")).toBeVisible(); + expect(screen.getByText("cell output crashed")).toBeVisible(); + expect(screen.getByTestId("reset-error-boundary-button")).toBeVisible(); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(getEnvironmentInfo).not.toHaveBeenCalled(); + + await expectRealReportModal(); + await waitFor(() => expect(getEnvironmentInfo).toHaveBeenCalledOnce()); + }); + + it("still opens one report dialog after descendant providers disappear", async () => { + const { getEnvironmentInfo } = setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + + renderCrashedBoundary( + + + , + ); + + await expectRealReportModal(); + expect(screen.getAllByRole("dialog")).toHaveLength(1); + await waitFor(() => expect(getEnvironmentInfo).toHaveBeenCalledOnce()); + }); + + it("shows partial diagnostics when the request client is missing", async () => { + renderCrashedBoundary(); + + await openReportDialog(); + await screen.findByText("Environment details"); + + expect( + screen.getByText("Server environment information unavailable"), + ).toBeVisible(); + expect( + screen.queryByText("Loading environment details…"), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Retry" }), + ).not.toBeInTheDocument(); + expect( + screen.getByRole("checkbox", { name: "Include notebook code" }), + ).toBeDisabled(); + expect(screen.getByText("Notebook source is unavailable.")).toBeVisible(); + const link = screen.getByRole("link", { name: "Open GitHub issue" }); + expect(link.getAttribute("href") ?? "").toContain("&env="); + }); + + it("does not request environment or source while the report dialog is closed", async () => { + localStorage.setItem( + "marimo:issue-report:include-code", + JSON.stringify(true), + ); + const { getEnvironmentInfo, readCode } = setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + readCode: vi.fn().mockResolvedValue({ contents: "import marimo" }), + }); + + renderCrashedBoundary(); + + expect(screen.getByText("Something went wrong")).toBeVisible(); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(getEnvironmentInfo).not.toHaveBeenCalled(); + expect(readCode).not.toHaveBeenCalled(); + }); + + it("restores focus to the report button after closing with Escape", async () => { + setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + renderCrashedBoundary(); + await openReportDialog(); + await closeReportDialog("escape"); + expect(screen.getByText(CRASH_MESSAGE)).toBeVisible(); + }); + + it("restores focus to the report button after closing with the close control", async () => { + setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + renderCrashedBoundary(); + await openReportDialog(); + await closeReportDialog("close"); + expect(screen.getByText(CRASH_MESSAGE)).toBeVisible(); + }); + + it("reopens the report dialog while the original error remains", async () => { + setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + renderCrashedBoundary(); + await openReportDialog(); + await closeReportDialog("escape"); + expect(screen.getByText(CRASH_MESSAGE)).toBeVisible(); + await expectRealReportModal(); + expect(screen.getByText(CRASH_MESSAGE)).toBeVisible(); + }); + + it("restores the child after try again once the failure is cleared", () => { + setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + const view = renderCrashedBoundary(); + expect(screen.getByText("Something went wrong")).toBeVisible(); + + view.rerender( + + + , + ); + fireEvent.click(screen.getByTestId("reset-error-boundary-button")); + + expect(screen.getByText("recovered child")).toBeVisible(); + expect(screen.queryByText("Something went wrong")).not.toBeInTheDocument(); + }); + + it("does not read notebook source when opened with fresh inclusion preferences", async () => { + const { readCode } = setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + readCode: vi.fn().mockResolvedValue({ contents: "import marimo" }), + }); + + renderCrashedBoundary(); + await expectRealReportModal(); + + expect(readCode).not.toHaveBeenCalled(); + }); + + it("renders the copy toast after the ancestor toaster is gone", async () => { + setRequestClient({ + getEnvironmentInfo: vi.fn().mockResolvedValue(environment), + }); + renderCrashedBoundary(); + await expectRealReportModal(); + + fireEvent.click( + screen.getByRole("button", { name: "Copy environment JSON" }), + ); + + expect(await screen.findByText("Environment details copied")).toBeVisible(); + }); +}); diff --git a/frontend/src/components/editor/chrome/components/__tests__/feedback-button.test.tsx b/frontend/src/components/editor/chrome/components/__tests__/feedback-button.test.tsx index 2a0ce95474d..b5307c2f996 100644 --- a/frontend/src/components/editor/chrome/components/__tests__/feedback-button.test.tsx +++ b/frontend/src/components/editor/chrome/components/__tests__/feedback-button.test.tsx @@ -208,4 +208,98 @@ describe("FeedbackModal issue reporting", () => { expect(href).toContain(encodeURIComponent('"marimo": "1.2.3"')); }); }); + + it("shows partial environment details when the request client is missing", async () => { + store.set(requestClientAtom, null); + render(, { wrapper }); + + await screen.findByText("Environment details"); + expect( + screen.getByText("Server environment information unavailable"), + ).toBeInTheDocument(); + expect( + screen.queryByText("Loading environment details…"), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Retry" }), + ).not.toBeInTheDocument(); + + const link = screen.getByRole("link", { name: "Open GitHub issue" }); + const href = link.getAttribute("href") ?? ""; + expect(href).toContain("&env="); + expect(href).toContain(encodeURIComponent("Environment Collection Error")); + + fireEvent.click( + screen.getByRole("button", { name: "Copy environment JSON" }), + ); + await waitFor(() => + expect(copyModule.copyToClipboard).toHaveBeenCalledWith( + expect.stringContaining("Environment Collection Error"), + ), + ); + }); + + it("disables source collection when the request client is missing", async () => { + localStorage.setItem( + "marimo:issue-report:include-code", + JSON.stringify(true), + ); + store.set(connectionAtom, { state: WebSocketState.CONNECTING }); + store.set(requestClientAtom, null); + render(, { wrapper }); + + await screen.findByText("Environment details"); + expect( + screen.getByRole("checkbox", { name: "Include notebook code" }), + ).toBeDisabled(); + expect( + screen.getByText("Notebook source is unavailable."), + ).toBeInTheDocument(); + expect( + screen.queryByText("Connect the notebook to include its source."), + ).not.toBeInTheDocument(); + const link = screen.getByRole("link", { name: "Open GitHub issue" }); + expect(link.getAttribute("href") ?? "").not.toContain("reproduction-code="); + }); + + it("preserves partial diagnostics when the environment request throws", async () => { + store.set( + requestClientAtom, + MockRequestClient.create({ + getEnvironmentInfo: vi.fn().mockImplementation(() => { + throw new Error("offline"); + }), + }), + ); + render(, { wrapper }); + + await screen.findByText("Server environment information unavailable"); + expect(screen.getByRole("button", { name: "Retry" })).toBeInTheDocument(); + expect( + screen.getByText(/Environment Collection Error/), + ).toBeInTheDocument(); + }); + + it("replaces partial diagnostics after a successful environment retry", async () => { + const getEnvironmentInfo = vi + .fn() + .mockRejectedValueOnce(new Error("offline")) + .mockResolvedValueOnce(environment); + store.set( + requestClientAtom, + MockRequestClient.create({ getEnvironmentInfo }), + ); + render(, { wrapper }); + + await screen.findByText("Server environment information unavailable"); + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + + await screen.findByText(/"marimo": "1.2.3"/); + expect( + screen.queryByText("Server environment information unavailable"), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Retry" }), + ).not.toBeInTheDocument(); + }); }); diff --git a/frontend/src/components/editor/chrome/components/feedback-button.tsx b/frontend/src/components/editor/chrome/components/feedback-button.tsx index 2818f2497ff..ba1737a77e3 100644 --- a/frontend/src/components/editor/chrome/components/feedback-button.tsx +++ b/frontend/src/components/editor/chrome/components/feedback-button.tsx @@ -36,7 +36,7 @@ import { import { useNotebookCodeAvailable } from "@/core/meta/code-visibility"; import { getMarimoVersion } from "@/core/meta/globals"; import { connectionAtom } from "@/core/network/connection"; -import { useRequestClient } from "@/core/network/requests"; +import { requestClientAtom } from "@/core/network/requests"; import { filenameAtom } from "@/core/saving/file-state"; import { store } from "@/core/state/jotai"; import { WebSocketState } from "@/core/websocket/types"; @@ -74,6 +74,27 @@ const CollapsiblePreview: React.FC<{ content: string }> = ({ content }) => { ); }; +function getNotebookSourceUnavailableReason(args: { + hasRequestClient: boolean; + filename: string | null; + codeAvailable: boolean; + connectionState: WebSocketState; +}): string | undefined { + if (!args.hasRequestClient) { + return "Notebook source is unavailable."; + } + if (args.filename === null) { + return "Save the notebook first."; + } + if (!args.codeAvailable) { + return "Notebook source is hidden in this view."; + } + if (args.connectionState !== WebSocketState.OPEN) { + return "Connect the notebook to include its source."; + } + return undefined; +} + export const FeedbackButton: React.FC = ({ children }) => { const { openModal, closeModal } = useImperativeModal(); @@ -85,13 +106,15 @@ export const FeedbackButton: React.FC = ({ children }) => { }; export const FeedbackModal: React.FC<{ - onClose: () => void; + onClose?: () => void; }> = () => { - const { getEnvironmentInfo, readCode } = useRequestClient(); - const environmentRequest = useAsyncData( - async () => getEnvironmentInfo(), - [getEnvironmentInfo], - ); + const requestClient = useAtomValue(requestClientAtom); + const environmentRequest = useAsyncData(async () => { + if (requestClient == null) { + return undefined; + } + return requestClient.getEnvironmentInfo(); + }, [requestClient]); const notebook = useAtomValue(notebookAtom); const errors = getCellErrorEntries(store); @@ -102,18 +125,13 @@ export const FeedbackModal: React.FC<{ const codeAvailable = useNotebookCodeAvailable(cells); const filename = useAtomValue(filenameAtom); const connection = useAtomValue(connectionAtom); - const notebookSourceAvailable = - filename !== null && - codeAvailable && - connection.state === WebSocketState.OPEN; - - const notebookSourceReason = notebookSourceAvailable - ? undefined - : filename === null - ? "Save the notebook first." - : !codeAvailable - ? "Notebook source is hidden in this view." - : "Connect the notebook to include its source."; + const notebookSourceReason = getNotebookSourceUnavailableReason({ + hasRequestClient: requestClient != null, + filename, + codeAvailable, + connectionState: connection.state, + }); + const notebookSourceAvailable = notebookSourceReason === undefined; const [includeErrors, setIncludeErrors] = useLocalStorage( "marimo:issue-report:include-errors", @@ -127,7 +145,7 @@ export const FeedbackModal: React.FC<{ const environment: EnvironmentDiagnostics | undefined = environmentRequest.data ? enrichEnvironment(environmentRequest.data, navigator.userAgent) - : environmentRequest.status === "error" + : environmentRequest.status === "error" || requestClient == null ? createPartialEnvironment( getMarimoVersion(), navigator.userAgent, @@ -137,12 +155,12 @@ export const FeedbackModal: React.FC<{ : undefined; const codeRequest = useAsyncData(async () => { - if (!includeCode || !notebookSourceAvailable) { + if (!includeCode || !notebookSourceAvailable || requestClient == null) { return undefined; } - const { contents } = await readCode(); + const { contents } = await requestClient.readCode(); return contents; - }, [includeCode, notebookSourceAvailable, readCode]); + }, [includeCode, notebookSourceAvailable, requestClient]); let githubIssueUrl = Constants.bugReportUrl; let omitted: string[] = []; @@ -255,7 +273,7 @@ export const FeedbackModal: React.FC<{ )}
- {environmentRequest.status === "pending" && ( + {environmentRequest.status === "pending" && requestClient != null && (
Loading environment details… @@ -266,18 +284,20 @@ export const FeedbackModal: React.FC<{
)} - {environmentRequest.status === "error" && ( + {(environmentRequest.status === "error" || requestClient == null) && (
Server environment information unavailable - + {requestClient != null && ( + + )}
)} diff --git a/frontend/src/components/ui/use-toast.ts b/frontend/src/components/ui/use-toast.ts index 1b33c62da99..2f09e86a10c 100644 --- a/frontend/src/components/ui/use-toast.ts +++ b/frontend/src/components/ui/use-toast.ts @@ -161,6 +161,14 @@ function dispatch(action: Action) { }); } +export function clearToasts() { + for (const timeout of toastTimeouts.values()) { + clearTimeout(timeout); + } + toastTimeouts.clear(); + dispatch({ type: "REMOVE_TOAST" }); +} + type Toast = Omit; function toast({