From ee6b0887c3d5ba8e99950663875fa5b524b07854 Mon Sep 17 00:00:00 2001 From: Dominion116 Date: Thu, 2 Jul 2026 16:36:20 +0100 Subject: [PATCH] fix: harden ErrorBoundary reset and logging --- src/components/ErrorBoundary.test.tsx | 101 +++++++++++++++++--------- src/components/ErrorBoundary.tsx | 57 ++++++++++++--- 2 files changed, 112 insertions(+), 46 deletions(-) diff --git a/src/components/ErrorBoundary.test.tsx b/src/components/ErrorBoundary.test.tsx index 4cd3998..0181966 100644 --- a/src/components/ErrorBoundary.test.tsx +++ b/src/components/ErrorBoundary.test.tsx @@ -1,5 +1,5 @@ -import { fireEvent,render, screen } from "@testing-library/react"; -import { describe, expect, it, vi, beforeEach } from "vitest"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { ErrorBoundary } from "./ErrorBoundary"; @@ -7,15 +7,14 @@ const ThrowError = () => { throw new Error("Test error!"); }; -let originalEnv: string | undefined; - -beforeEach(() => { - originalEnv = process.env.NODE_ENV; +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); }); describe("ErrorBoundary", () => { it("renders default fallback when child throws, and resets when try again is clicked", () => { - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => {}); render( @@ -33,8 +32,6 @@ describe("ErrorBoundary", () => { // Clicking reset should try to re-render the children // (It will just throw again because we always throw in ThrowError, but it resets state) fireEvent.click(resetBtn); - - consoleSpy.mockRestore(); }); it("renders custom fallback prop and passes error and reset function", () => { @@ -47,7 +44,7 @@ describe("ErrorBoundary", () => { )); // Suppress console.error for expected thrown error - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => {}); render( @@ -65,8 +62,6 @@ describe("ErrorBoundary", () => { // Reset should be callable and reset the error state (though it will just throw again because we still render ThrowError) // but we can verify it doesn't crash. fireEvent.click(resetBtn); - - consoleSpy.mockRestore(); }); it("calls onError callback with error and info when child throws", () => { @@ -80,18 +75,38 @@ describe("ErrorBoundary", () => { ); expect(onErrorSpy).toHaveBeenCalled(); + expect(consoleSpy).not.toHaveBeenCalledWith( + "[sorokit-ui] Uncaught error:", + expect.any(Error), + expect.any(String) + ); const errorArg = onErrorSpy.mock.calls[0][0]; const infoArg = onErrorSpy.mock.calls[0][1]; expect(errorArg).toBeInstanceOf(Error); expect(errorArg.message).toBe("Test error!"); expect(infoArg).toHaveProperty("componentStack"); + }); - consoleSpy.mockRestore(); + it("does not call console.error in production mode", () => { + vi.stubEnv("DEV", false); + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + render( + + + + ); + + expect(consoleSpy).not.toHaveBeenCalledWith( + "[sorokit-ui] Uncaught error:", + expect.any(Error), + expect.any(String) + ); }); - it("does not call console.error in production mode when onError is provided", () => { - process.env.NODE_ENV = "production"; + it("calls onError instead of console.error in production mode when provided", () => { + vi.stubEnv("DEV", false); const onErrorSpy = vi.fn(); const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); @@ -101,15 +116,16 @@ describe("ErrorBoundary", () => { ); - expect(consoleSpy).not.toHaveBeenCalled(); + expect(consoleSpy).not.toHaveBeenCalledWith( + "[sorokit-ui] Uncaught error:", + expect.any(Error), + expect.any(String) + ); expect(onErrorSpy).toHaveBeenCalled(); - - consoleSpy.mockRestore(); - process.env.NODE_ENV = originalEnv; }); it("calls console.error in development mode by default", () => { - process.env.NODE_ENV = "development"; + vi.stubEnv("DEV", true); const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); render( @@ -124,42 +140,55 @@ describe("ErrorBoundary", () => { expect.any(Error), expect.any(String) ); - - consoleSpy.mockRestore(); - process.env.NODE_ENV = originalEnv; }); - it("reset key triggers component remount rather than re-render", () => { - let mountCount = 0; - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + it("reset key remounts children with fresh state to avoid an error loop", () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + + let shouldRecoverAfterReset = false; + let mountedWithFreshState = false; const TestComponent = () => { - mountCount++; - if (mountCount === 1) { - throw new Error("First mount error"); + if (!shouldRecoverAfterReset) { + throw new Error("Corrupted child state"); } + + mountedWithFreshState = true; + return
Mounted successfully
; }; - const { rerender } = render( + render( ); - // First mount throws error - expect(mountCount).toBe(1); expect(screen.getByText("Something went wrong")).toBeInTheDocument(); - // Click reset to trigger remount const resetBtn = screen.getByRole("button", { name: /try again/i }); + shouldRecoverAfterReset = true; fireEvent.click(resetBtn); - // Component should be remounted (mountCount increases) - expect(mountCount).toBe(2); + expect(mountedWithFreshState).toBe(true); expect(screen.getByTestId("test-content")).toBeInTheDocument(); expect(screen.getByText("Mounted successfully")).toBeInTheDocument(); + }); + + it("applies scoped container styling when isolate is true", () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + + const { container } = render( + + + + ); + + const scopedFallback = container.firstElementChild; - consoleSpy.mockRestore(); + expect(scopedFallback).toHaveClass("overflow-hidden"); + expect(scopedFallback).toHaveClass("rounded-xl"); + expect(scopedFallback).toHaveClass("border"); + expect(scopedFallback).toHaveClass("min-h-[260px]"); }); }); diff --git a/src/components/ErrorBoundary.tsx b/src/components/ErrorBoundary.tsx index 7f6d54a..4649f24 100644 --- a/src/components/ErrorBoundary.tsx +++ b/src/components/ErrorBoundary.tsx @@ -2,39 +2,76 @@ import { AlertCircleIcon, Refresh01Icon } from "@hugeicons/core-free-icons"; import { HugeiconsIcon } from "@hugeicons/react"; import { Component, type ErrorInfo, type ReactNode } from "react"; +import { cn } from "../lib/utils"; + interface Props { children: ReactNode; /** Custom fallback UI. Receives the error and a reset callback. */ fallback?: (error: Error, reset: () => void) => ReactNode; + /** Called when the boundary catches an error. */ + onError?: (error: Error, info: ErrorInfo) => void; + /** Render fallback content as an in-page scoped panel instead of a full-page state. */ + isolate?: boolean; } interface State { error: Error | null; + resetKey: number; } export class ErrorBoundary extends Component { - state: State = { error: null }; + state: State = { error: null, resetKey: 0 }; - static getDerivedStateFromError(error: Error): State { + static getDerivedStateFromError(error: Error): Partial { return { error }; } componentDidCatch(error: Error, info: ErrorInfo) { - console.error("[sorokit-ui] Uncaught error:", error, info.componentStack); + if (this.props.onError) { + this.props.onError(error, info); + return; + } + + if (import.meta.env.DEV) { + console.error("[sorokit-ui] Uncaught error:", error, info.componentStack); + } } - reset = () => this.setState({ error: null }); + reset = () => + this.setState((state) => ({ + error: null, + resetKey: state.resetKey + 1, + })); render() { - const { error } = this.state; - if (!error) return this.props.children; + const { error, resetKey } = this.state; + const { children, fallback, isolate } = this.props; - if (this.props.fallback) { - return this.props.fallback(error, this.reset); + if (!error) return
{children}
; + + if (fallback) { + const fallbackNode = fallback(error, this.reset); + + return isolate ? ( +
+ {fallbackNode} +
+ ) : ( + fallbackNode + ); } return ( -
+
{

{import.meta.env.DEV ? error.message - : "See the browser console for details."} + : "Details are hidden in production."}