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 <noreply@anthropic.com>
This commit is contained in:
Misaka
2026-04-04 17:11:59 +08:00
parent 1e0bb1de24
commit 3d5adb74d8
6 changed files with 177 additions and 71 deletions

View File

@@ -1,6 +1,7 @@
import React from 'react' import React from 'react'
import { AuthenticatedAppShell } from './components/app/AuthenticatedAppShell' import { AuthenticatedAppShell } from './components/app/AuthenticatedAppShell'
import { UnauthenticatedApp } from './components/app/UnauthenticatedApp' import { UnauthenticatedApp } from './components/app/UnauthenticatedApp'
import { ErrorBoundary } from './components/ErrorBoundary'
import PlaywrightDownloadDialog from './components/PlaywrightDownloadDialog' import PlaywrightDownloadDialog from './components/PlaywrightDownloadDialog'
import { useAppBootstrap } from './hooks/useAppBootstrap' import { useAppBootstrap } from './hooks/useAppBootstrap'
@@ -49,51 +50,57 @@ function App(): React.JSX.Element {
// Show Playwright download dialog first (before authentication check) // Show Playwright download dialog first (before authentication check)
if (showPlaywrightDownload) { if (showPlaywrightDownload) {
return ( return (
<PlaywrightDownloadDialog <ErrorBoundary scope="PlaywrightDownload">
isOpen={showPlaywrightDownload} <PlaywrightDownloadDialog
onClose={() => {}} isOpen={showPlaywrightDownload}
onDownloadComplete={handlePlaywrightDownloadComplete} onClose={() => {}}
/> onDownloadComplete={handlePlaywrightDownloadComplete}
/>
</ErrorBoundary>
) )
} }
if (!isAuthenticated) { if (!isAuthenticated) {
return ( return (
<UnauthenticatedApp <ErrorBoundary scope="UnauthenticatedApp">
isAuthenticating={isAuthenticating} <UnauthenticatedApp
showLoginDialog={showLoginDialog} isAuthenticating={isAuthenticating}
showUserSelection={showUserSelection} showLoginDialog={showLoginDialog}
computerName={computerName} showUserSelection={showUserSelection}
currentUser={currentUser} computerName={computerName}
allUsers={allUsers} currentUser={currentUser}
errorMessage={errorMessage} allUsers={allUsers}
onLogin={handleLogin} errorMessage={errorMessage}
onLoginCancel={handleLoginCancel} onLogin={handleLogin}
onSelectUser={handleUserSelect} onLoginCancel={handleLoginCancel}
onUserSelectionCancel={handleUserSelectionCancel} onSelectUser={handleUserSelect}
onError={showError} onUserSelectionCancel={handleUserSelectionCancel}
logoutButtonRef={logoutButtonRef} onError={showError}
/> logoutButtonRef={logoutButtonRef}
/>
</ErrorBoundary>
) )
} }
return ( return (
<AuthenticatedAppShell <ErrorBoundary scope="AuthenticatedApp">
currentUser={currentUser} <AuthenticatedAppShell
currentPage={currentPage} currentUser={currentUser}
onNavigate={setCurrentPage} currentPage={currentPage}
updateStatus={updateStatus} onNavigate={setCurrentPage}
updateCatalog={updateCatalog} updateStatus={updateStatus}
showUpdateDialog={showUpdateDialog} updateCatalog={updateCatalog}
onOpenUpdateDialog={openUpdateDialog} showUpdateDialog={showUpdateDialog}
onCloseUpdateDialog={() => setShowUpdateDialog(false)} onOpenUpdateDialog={openUpdateDialog}
onInstallUserRelease={handleInstallUserRelease} onCloseUpdateDialog={() => setShowUpdateDialog(false)}
onDownloadAndInstallAdminRelease={handleAdminDownloadAndInstall} onInstallUserRelease={handleInstallUserRelease}
onRefreshCatalog={refreshUpdateDialogState} onDownloadAndInstallAdminRelease={handleAdminDownloadAndInstall}
shouldShowLogout={shouldShowLogout} onRefreshCatalog={refreshUpdateDialogState}
onLogout={handleLogout} shouldShowLogout={shouldShowLogout}
logoutButtonRef={logoutButtonRef} onLogout={handleLogout}
/> logoutButtonRef={logoutButtonRef}
/>
</ErrorBoundary>
) )
} }

View File

@@ -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<ErrorBoundaryProps, ErrorBoundaryState> {
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 (
<div className="flex min-h-screen items-center justify-center bg-slate-50 p-8">
<div className="w-full max-w-md rounded-xl border border-slate-200 bg-white p-8 text-center shadow-lg">
<div className="mx-auto mb-4 flex h-14 w-14 items-center justify-center rounded-full bg-rose-100">
<AlertTriangle size={28} className="text-rose-600" />
</div>
<h2 className="mb-2 text-xl font-bold text-slate-800"></h2>
<p className="mb-4 text-sm text-slate-500">
</p>
<details className="mb-6 text-left">
<summary className="cursor-pointer text-xs text-slate-400 hover:text-slate-600">
</summary>
<pre className="mt-2 max-h-40 overflow-auto rounded-lg bg-slate-100 p-3 text-xs text-slate-700">
{this.state.error?.message}
</pre>
</details>
<button
onClick={this.handleReload}
className="inline-flex items-center gap-2 rounded-lg bg-blue-600 px-5 py-2.5 text-sm font-medium text-white transition hover:bg-blue-700"
>
<RotateCcw size={16} />
</button>
</div>
</div>
)
}
return this.props.children
}
}

View File

@@ -1,6 +1,7 @@
import React, { useEffect, useState, useCallback } from 'react' import React, { useEffect, useState, useCallback } from 'react'
import { DownloadCloud, LoaderCircle, X } from 'lucide-react' import { DownloadCloud, LoaderCircle, X } from 'lucide-react'
import Modal from './ui/Modal' import Modal from './ui/Modal'
import { useLogger } from '../hooks/useLogger'
interface DownloadProgress { interface DownloadProgress {
percent: number // 0-100 percent: number // 0-100
downloadedBytes: number downloadedBytes: number
@@ -25,6 +26,7 @@ export default function PlaywrightDownloadDialog({
const [error, setError] = useState<string | null>(null) const [error, setError] = useState<string | null>(null)
const [isDownloading, setIsDownloading] = useState(false) const [isDownloading, setIsDownloading] = useState(false)
const [showCancelConfirm, setShowCancelConfirm] = useState(false) const [showCancelConfirm, setShowCancelConfirm] = useState(false)
const logger = useLogger('PlaywrightDownload')
// Format bytes to human-readable string // Format bytes to human-readable string
const formatBytes = useCallback((bytes: number): string => { const formatBytes = useCallback((bytes: number): string => {
@@ -99,13 +101,15 @@ export default function PlaywrightDownloadDialog({
try { try {
await window.electron.playwrightBrowser.cancel() await window.electron.playwrightBrowser.cancel()
} catch (err) { } catch (err) {
console.error('Failed to cancel download:', err) logger.error('Failed to cancel download', {
error: err instanceof Error ? err.message : String(err)
})
} finally { } finally {
setShowCancelConfirm(false) setShowCancelConfirm(false)
setIsDownloading(false) setIsDownloading(false)
onClose() onClose()
} }
}, [onClose]) }, [onClose, logger])
const handleConfirmCancel = useCallback(() => { const handleConfirmCancel = useCallback(() => {
void handleCancel() void handleCancel()

View File

@@ -6,6 +6,7 @@
import { useState, useCallback, useEffect } from 'react' import { useState, useCallback, useEffect } from 'react'
import { ReportMetrics } from '../types' import { ReportMetrics } from '../types'
import { parseReportData } from '../utils/parser' import { parseReportData } from '../utils/parser'
import { useLogger } from '../../../hooks/useLogger'
interface UseReportDataResult { interface UseReportDataResult {
isLoading: boolean isLoading: boolean
@@ -26,6 +27,7 @@ export const useReportData = (isAdmin: boolean, isOpen: boolean): UseReportDataR
const [isLoading, setIsLoading] = useState(false) const [isLoading, setIsLoading] = useState(false)
const [error, setError] = useState<string | null>(null) const [error, setError] = useState<string | null>(null)
const [reportData, setReportData] = useState<ReportMetrics[]>([]) const [reportData, setReportData] = useState<ReportMetrics[]>([])
const logger = useLogger('ReportData')
const loadAndAnalyzeReports = useCallback(async () => { const loadAndAnalyzeReports = useCallback(async () => {
if (!isAdmin) return if (!isAdmin) return
@@ -55,7 +57,10 @@ export const useReportData = (isAdmin: boolean, isOpen: boolean): UseReportDataR
return { report, content: contentResult.data } return { report, content: contentResult.data }
} }
} catch (e) { } 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 return null
}) })
@@ -80,7 +85,7 @@ export const useReportData = (isAdmin: boolean, isOpen: boolean): UseReportDataR
} finally { } finally {
setIsLoading(false) setIsLoading(false)
} }
}, [isAdmin]) }, [isAdmin, logger])
const clearData = useCallback(() => { const clearData = useCallback(() => {
setReportData([]) setReportData([])

View File

@@ -139,7 +139,14 @@ export const parseReportData = (
const executionTimeSecs = parseDurationToSeconds(values.executionTimeStr) const executionTimeSecs = parseDurationToSeconds(values.executionTimeStr)
if (values.executionTimeStr === '0秒') { 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 // Try to parse the date

View File

@@ -1,4 +1,5 @@
import { useEffect, RefObject } from 'react' import { useEffect, RefObject } from 'react'
import { useLogger } from './useLogger'
/** /**
* Options for configuring dialog focus management * Options for configuring dialog focus management
@@ -83,6 +84,8 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
shouldCloseOnEscape = true shouldCloseOnEscape = true
} = options } = options
const logger = useLogger('DialogFocus')
// Handle Escape key press // Handle Escape key press
useEffect(() => { useEffect(() => {
if (!isOpen) return if (!isOpen) return
@@ -175,10 +178,10 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
return return
} }
// Fallback: element found but not visible, log warning and try default // 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 { } else {
// Fallback: element not found, log warning and try default // 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 // Delay to ensure portal content is rendered
requestAnimationFrame(setupFocus) requestAnimationFrame(setupFocus)
}, [isOpen, dialogRef, initialFocusSelector]) }, [isOpen, dialogRef, initialFocusSelector, logger])
// Restore focus to trigger element when dialog closes // Restore focus to trigger element when dialog closes
useEffect(() => { useEffect(() => {
@@ -214,50 +217,36 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
// Check if element still exists in DOM // Check if element still exists in DOM
if (!triggerElement || !document.contains(triggerElement)) { if (!triggerElement || !document.contains(triggerElement)) {
if (import.meta.env.DEV) { logger.warn('Trigger element not found in DOM, cannot restore focus')
console.warn('[useDialogFocus] Trigger element not found in DOM, cannot restore focus')
}
return return
} }
// Check if element has a focus method // Check if element has a focus method
if (typeof triggerElement.focus !== 'function') { if (typeof triggerElement.focus !== 'function') {
if (import.meta.env.DEV) { logger.warn('Trigger element does not have a focus method')
console.warn('[useDialogFocus] Trigger element does not have a focus method')
}
return return
} }
// Check if element is visible (not display: none) // Check if element is visible (not display: none)
const style = window.getComputedStyle(triggerElement) const style = window.getComputedStyle(triggerElement)
if (style.display === 'none') { if (style.display === 'none') {
if (import.meta.env.DEV) { logger.warn('Trigger element is display: none, cannot restore focus')
console.warn('[useDialogFocus] Trigger element is display: none, cannot restore focus')
}
return return
} }
if (style.visibility === 'hidden') { if (style.visibility === 'hidden') {
if (import.meta.env.DEV) { logger.warn('Trigger element is visibility: hidden, cannot restore focus')
console.warn(
'[useDialogFocus] Trigger element is visibility: hidden, cannot restore focus'
)
}
return return
} }
// Check if element is disabled // Check if element is disabled
if (triggerElement instanceof HTMLButtonElement && triggerElement.disabled) { if (triggerElement instanceof HTMLButtonElement && triggerElement.disabled) {
if (import.meta.env.DEV) { logger.warn('Trigger element is disabled, cannot restore focus')
console.warn('[useDialogFocus] Trigger element is disabled, cannot restore focus')
}
// Try to find nearest enabled ancestor or fallback to body // Try to find nearest enabled ancestor or fallback to body
const focusableParent = findNearestFocusableElement(triggerElement) const focusableParent = findNearestFocusableElement(triggerElement)
if (focusableParent) { if (focusableParent) {
focusableParent.focus({ preventScroll: true }) focusableParent.focus({ preventScroll: true })
if (import.meta.env.DEV) { logger.debug('Restored focus to nearest focusable ancestor')
console.info('[useDialogFocus] Restored focus to nearest focusable ancestor')
}
} }
return return
} }
@@ -265,13 +254,11 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
// All checks passed, restore focus // All checks passed, restore focus
try { try {
triggerElement.focus({ preventScroll: true }) triggerElement.focus({ preventScroll: true })
if (import.meta.env.DEV) { logger.debug('Successfully restored focus to trigger element')
console.info('[useDialogFocus] Successfully restored focus to trigger element')
}
} catch (error) { } catch (error) {
if (import.meta.env.DEV) { logger.error('Error restoring focus', {
console.error('[useDialogFocus] Error restoring focus:', error) 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 // Use microtask queue to ensure this runs after DOM cleanup
queueMicrotask(restoreFocus) queueMicrotask(restoreFocus)
}, [isOpen, triggerRef]) }, [isOpen, triggerRef, logger])
// Return focus lock configuration // Return focus lock configuration
return { return {