From 3d5adb74d8eda2d1dc3e16f68eb28cd84b6b150d Mon Sep 17 00:00:00 2001 From: Misaka Date: Sat, 4 Apr 2026 17:11:59 +0800 Subject: [PATCH] feat(logging-p0): add ErrorBoundary and replace remaining console.* with logger Add React ErrorBoundary component that captures rendering errors with full component stack and logs them to main process via IPC. Wrap all three App branches (PlaywrightDownload, UnauthenticatedApp, AuthenticatedApp) with scoped boundaries. Replace 13 console.* calls across renderer with structured logger: - useDialogFocus: 10 calls (focus management diagnostics) - PlaywrightDownloadDialog: 1 call (download cancellation error) - useReportData: 1 call (report fetch failure) - parser: 1 call (execution time extraction warning) Co-Authored-By: Claude Opus 4.6 --- src/renderer/src/App.tsx | 79 ++++++++------- src/renderer/src/components/ErrorBoundary.tsx | 96 +++++++++++++++++++ .../components/PlaywrightDownloadDialog.tsx | 8 +- .../report-analysis/hooks/useReportData.ts | 9 +- .../report-analysis/utils/parser.ts | 9 +- src/renderer/src/hooks/useDialogFocus.ts | 47 ++++----- 6 files changed, 177 insertions(+), 71 deletions(-) create mode 100644 src/renderer/src/components/ErrorBoundary.tsx diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index b1425a9..efece7c 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -1,6 +1,7 @@ import React from 'react' import { AuthenticatedAppShell } from './components/app/AuthenticatedAppShell' import { UnauthenticatedApp } from './components/app/UnauthenticatedApp' +import { ErrorBoundary } from './components/ErrorBoundary' import PlaywrightDownloadDialog from './components/PlaywrightDownloadDialog' import { useAppBootstrap } from './hooks/useAppBootstrap' @@ -49,51 +50,57 @@ function App(): React.JSX.Element { // Show Playwright download dialog first (before authentication check) if (showPlaywrightDownload) { return ( - {}} - onDownloadComplete={handlePlaywrightDownloadComplete} - /> + + {}} + onDownloadComplete={handlePlaywrightDownloadComplete} + /> + ) } if (!isAuthenticated) { return ( - + + + ) } return ( - setShowUpdateDialog(false)} - onInstallUserRelease={handleInstallUserRelease} - onDownloadAndInstallAdminRelease={handleAdminDownloadAndInstall} - onRefreshCatalog={refreshUpdateDialogState} - shouldShowLogout={shouldShowLogout} - onLogout={handleLogout} - logoutButtonRef={logoutButtonRef} - /> + + setShowUpdateDialog(false)} + onInstallUserRelease={handleInstallUserRelease} + onDownloadAndInstallAdminRelease={handleAdminDownloadAndInstall} + onRefreshCatalog={refreshUpdateDialogState} + shouldShowLogout={shouldShowLogout} + onLogout={handleLogout} + logoutButtonRef={logoutButtonRef} + /> + ) } diff --git a/src/renderer/src/components/ErrorBoundary.tsx b/src/renderer/src/components/ErrorBoundary.tsx new file mode 100644 index 0000000..ef7bf42 --- /dev/null +++ b/src/renderer/src/components/ErrorBoundary.tsx @@ -0,0 +1,96 @@ +import React from 'react' +import { AlertTriangle, RotateCcw } from 'lucide-react' + +interface ErrorBoundaryProps { + children: React.ReactNode + /** Optional label identifying the boundary scope (e.g. "App", "AuthenticatedShell") */ + scope?: string +} + +interface ErrorBoundaryState { + hasError: boolean + error: Error | null +} + +/** + * React Error Boundary that catches rendering errors in child components, + * logs the full error + component stack to the main process logger, + * and displays a fallback UI. + * + * Cannot use hooks (React constraint), so calls window.electron.logger directly. + */ +export class ErrorBoundary extends React.Component { + constructor(props: ErrorBoundaryProps) { + super(props) + this.state = { hasError: false, error: null } + } + + static getDerivedStateFromError(error: Error): ErrorBoundaryState { + return { hasError: true, error } + } + + componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { + const scope = this.props.scope || 'Unknown' + + // Log to main process via IPC logger + try { + if (typeof window !== 'undefined' && window.electron?.logger?.log) { + window.electron.logger.log('error', `Render error in <${scope}>`, { + error: { + name: error.name, + message: error.message, + stack: error.stack + }, + componentStack: errorInfo.componentStack, + boundaryScope: scope + }) + } + } catch { + // Logging failed — don't make things worse + } + + // Also print to console in development for immediate visibility + if (import.meta.env.DEV) { + console.error(`[ErrorBoundary:${scope}]`, error, errorInfo.componentStack) + } + } + + handleReload = (): void => { + this.setState({ hasError: false, error: null }) + } + + render(): React.ReactNode { + if (this.state.hasError) { + return ( +
+
+
+ +
+

页面出现错误

+

+ 应用发生了未预期的错误，请尝试刷新页面。如果问题持续存在，请联系管理员。 +

+
+ + 错误详情 + + 
+                {this.state.error?.message}
+
+
+ +
+
+ ) + } + + return this.props.children + } +} diff --git a/src/renderer/src/components/PlaywrightDownloadDialog.tsx b/src/renderer/src/components/PlaywrightDownloadDialog.tsx index 74179f1..83e445d 100644 --- a/src/renderer/src/components/PlaywrightDownloadDialog.tsx +++ b/src/renderer/src/components/PlaywrightDownloadDialog.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useState, useCallback } from 'react' import { DownloadCloud, LoaderCircle, X } from 'lucide-react' import Modal from './ui/Modal' +import { useLogger } from '../hooks/useLogger' interface DownloadProgress { percent: number // 0-100 downloadedBytes: number @@ -25,6 +26,7 @@ export default function PlaywrightDownloadDialog({ const [error, setError] = useState(null) const [isDownloading, setIsDownloading] = useState(false) const [showCancelConfirm, setShowCancelConfirm] = useState(false) + const logger = useLogger('PlaywrightDownload') // Format bytes to human-readable string const formatBytes = useCallback((bytes: number): string => { @@ -99,13 +101,15 @@ export default function PlaywrightDownloadDialog({ try { await window.electron.playwrightBrowser.cancel() } catch (err) { - console.error('Failed to cancel download:', err) + logger.error('Failed to cancel download', { + error: err instanceof Error ? err.message : String(err) + }) } finally { setShowCancelConfirm(false) setIsDownloading(false) onClose() } - }, [onClose]) + }, [onClose, logger]) const handleConfirmCancel = useCallback(() => { void handleCancel() diff --git a/src/renderer/src/components/report-analysis/hooks/useReportData.ts b/src/renderer/src/components/report-analysis/hooks/useReportData.ts index 361f394..e2fd737 100644 --- a/src/renderer/src/components/report-analysis/hooks/useReportData.ts +++ b/src/renderer/src/components/report-analysis/hooks/useReportData.ts @@ -6,6 +6,7 @@ import { useState, useCallback, useEffect } from 'react' import { ReportMetrics } from '../types' import { parseReportData } from '../utils/parser' +import { useLogger } from '../../../hooks/useLogger' interface UseReportDataResult { isLoading: boolean @@ -26,6 +27,7 @@ export const useReportData = (isAdmin: boolean, isOpen: boolean): UseReportDataR const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) const [reportData, setReportData] = useState([]) + const logger = useLogger('ReportData') const loadAndAnalyzeReports = useCallback(async () => { if (!isAdmin) return @@ -55,7 +57,10 @@ export const useReportData = (isAdmin: boolean, isOpen: boolean): UseReportDataR return { report, content: contentResult.data } } } catch (e) { - console.warn(`Failed to fetch content for report ${report.key}`, e) + logger.warn('Failed to fetch content for report', { + reportKey: report.key, + error: e instanceof Error ? e.message : String(e) + }) } return null }) @@ -80,7 +85,7 @@ export const useReportData = (isAdmin: boolean, isOpen: boolean): UseReportDataR } finally { setIsLoading(false) } - }, [isAdmin]) + }, [isAdmin, logger]) const clearData = useCallback(() => { setReportData([]) diff --git a/src/renderer/src/components/report-analysis/utils/parser.ts b/src/renderer/src/components/report-analysis/utils/parser.ts index 8c92bcc..9d6e915 100644 --- a/src/renderer/src/components/report-analysis/utils/parser.ts +++ b/src/renderer/src/components/report-analysis/utils/parser.ts @@ -139,7 +139,14 @@ export const parseReportData = ( const executionTimeSecs = parseDurationToSeconds(values.executionTimeStr) if (values.executionTimeStr === '0秒') { - console.warn('Failed to extract execution time from report:', report.key) + try { + window.electron?.logger?.log?.('warn', 'Failed to extract execution time from report', { + reportKey: report.key, + context: 'ReportParser' + }) + } catch { + // Gracefully degrade if logger unavailable + } } // Try to parse the date diff --git a/src/renderer/src/hooks/useDialogFocus.ts b/src/renderer/src/hooks/useDialogFocus.ts index 03dfd8b..6a0063e 100644 --- a/src/renderer/src/hooks/useDialogFocus.ts +++ b/src/renderer/src/hooks/useDialogFocus.ts @@ -1,4 +1,5 @@ import { useEffect, RefObject } from 'react' +import { useLogger } from './useLogger' /** * Options for configuring dialog focus management @@ -83,6 +84,8 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe shouldCloseOnEscape = true } = options + const logger = useLogger('DialogFocus') + // Handle Escape key press useEffect(() => { if (!isOpen) return @@ -175,10 +178,10 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe return } // Fallback: element found but not visible, log warning and try default - console.warn(`Focus element found but not visible: ${initialFocusSelector}`) + logger.warn('Focus element found but not visible', { selector: initialFocusSelector }) } else { // Fallback: element not found, log warning and try default - console.warn(`Focus element not found for selector: ${initialFocusSelector}`) + logger.warn('Focus element not found for selector', { selector: initialFocusSelector }) } } @@ -203,7 +206,7 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe // Delay to ensure portal content is rendered requestAnimationFrame(setupFocus) - }, [isOpen, dialogRef, initialFocusSelector]) + }, [isOpen, dialogRef, initialFocusSelector, logger]) // Restore focus to trigger element when dialog closes useEffect(() => { @@ -214,50 +217,36 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe // Check if element still exists in DOM if (!triggerElement || !document.contains(triggerElement)) { - if (import.meta.env.DEV) { - console.warn('[useDialogFocus] Trigger element not found in DOM, cannot restore focus') - } + logger.warn('Trigger element not found in DOM, cannot restore focus') return } // Check if element has a focus method if (typeof triggerElement.focus !== 'function') { - if (import.meta.env.DEV) { - console.warn('[useDialogFocus] Trigger element does not have a focus method') - } + logger.warn('Trigger element does not have a focus method') return } // Check if element is visible (not display: none) const style = window.getComputedStyle(triggerElement) if (style.display === 'none') { - if (import.meta.env.DEV) { - console.warn('[useDialogFocus] Trigger element is display: none, cannot restore focus') - } + logger.warn('Trigger element is display: none, cannot restore focus') return } if (style.visibility === 'hidden') { - if (import.meta.env.DEV) { - console.warn( - '[useDialogFocus] Trigger element is visibility: hidden, cannot restore focus' - ) - } + logger.warn('Trigger element is visibility: hidden, cannot restore focus') return } // Check if element is disabled if (triggerElement instanceof HTMLButtonElement && triggerElement.disabled) { - if (import.meta.env.DEV) { - console.warn('[useDialogFocus] Trigger element is disabled, cannot restore focus') - } + logger.warn('Trigger element is disabled, cannot restore focus') // Try to find nearest enabled ancestor or fallback to body const focusableParent = findNearestFocusableElement(triggerElement) if (focusableParent) { focusableParent.focus({ preventScroll: true }) - if (import.meta.env.DEV) { - console.info('[useDialogFocus] Restored focus to nearest focusable ancestor') - } + logger.debug('Restored focus to nearest focusable ancestor') } return } @@ -265,13 +254,11 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe // All checks passed, restore focus try { triggerElement.focus({ preventScroll: true }) - if (import.meta.env.DEV) { - console.info('[useDialogFocus] Successfully restored focus to trigger element') - } + logger.debug('Successfully restored focus to trigger element') } catch (error) { - if (import.meta.env.DEV) { - console.error('[useDialogFocus] Error restoring focus:', error) - } + logger.error('Error restoring focus', { + error: error instanceof Error ? error.message : String(error) + }) } } @@ -308,7 +295,7 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe // Use microtask queue to ensure this runs after DOM cleanup queueMicrotask(restoreFocus) - }, [isOpen, triggerRef]) + }, [isOpen, triggerRef, logger]) // Return focus lock configuration return {