feat(error): implement comprehensive error handling and logging system#52
Conversation
- Add production-grade logging service with levels (debug, info, warn, error, fatal) - Add Logger class with context enrichment and optional remote logging - Create ErrorBoundary component for catching React errors - Create ErrorFallback component with organic design styling - Add CompactErrorFallback for inline error display - Implement global error handler with standardized AppError type - Add error codes and user-friendly error messages - Create retry utility with exponential backoff and jitter - Add retry presets (quick, standard, patient, aggressive) - Implement useNetworkError hook for network status monitoring - Implement useIsOnline hook for simple connectivity checks - Integrate ErrorBoundary in root layout - Initialize global error handlers on app start Closes #16 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughInitializes global error handling and logging, adds React ErrorBoundary and fallback UI, implements a retry/backoff utility and network-aware hooks, exposes lib exports, and integrates error/logger setup plus onboarding redirect logic into the root layout. New tests cover logger, retry, and error-handler modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as React Component
participant ErrorBoundary as ErrorBoundary
participant ErrorHandler as Error Handler
participant Logger as Logger Service
participant Fallback as ErrorFallback UI
Component->>Component: render / lifecycle
Component--xComponent: throws error
ErrorBoundary->>ErrorBoundary: getDerivedStateFromError()
ErrorBoundary->>ErrorHandler: handleError(error, { component: ... })
ErrorHandler->>Logger: logger.error/fatal(...)
Logger->>Logger: buffer log entry
ErrorBoundary->>ErrorBoundary: componentDidCatch()
ErrorBoundary->>Fallback: render fallback UI
Fallback-->>User: show message + retry
User->>Fallback: presses retry
Fallback->>ErrorBoundary: invoke onRetry -> resetErrorBoundary()
ErrorBoundary->>Component: re-render children
sequenceDiagram
participant App as Application
participant Hook as useNetworkError
participant NetInfo as NetInfo Service
participant ErrorHandler as Error Handler
participant RetryFn as Async Operation
App->>Hook: mount hook
Hook->>NetInfo: subscribe()
NetInfo-->>Hook: state update (online/offline)
Hook->>Hook: update network state & log via logger
App->>Hook: retryIfOnline(asyncFn)
Hook->>Hook: isOnline?
alt Online
Hook->>RetryFn: execute asyncFn (with retry util as needed)
RetryFn-->>Hook: result / error
Hook->>Hook: clear/set error state
Hook-->>App: return result
else Offline
Hook->>ErrorHandler: handleError(NETWORK_OFFLINE)
ErrorHandler->>Logger: logger.warn(...)
Hook-->>App: return null
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Add tests for Logger class (log levels, filtering, context, buffering) - Add tests for error handler (createAppError, parseError, handleError) - Add tests for retry utility (retry, withRetry, presets, error detection) - Increases test coverage from ~5% to ~34% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 35-38: Update the dependency version for
`@react-native-async-storage/async-storage` in package.json from "^2.0.0" to a
newer 2.x patch (e.g., "^2.2.0" or later) to avoid Kotlin/KSP Android build
issues; edit the dependency entry for
"@react-native-async-storage/async-storage" in package.json and run your package
manager's install command to lock the updated version.
In `@src/components/error/ErrorFallback.tsx`:
- Around line 103-107: The "Report Issue" Pressable in the ErrorFallback
component is missing an onPress handler so the button is non-functional; add an
onPress prop to the Pressable (or create a handler method like handleReportIssue
inside ErrorFallback) that either triggers a placeholder action (e.g., show an
alert/toast saying "Coming soon") or opens the real reporting flow, and include
a TODO comment if it's a temporary stub; ensure the handler is properly
bound/passed to the Pressable's onPress prop so the button becomes interactive.
In `@src/hooks/use-network-error.ts`:
- Around line 40-67: The effect's event listener closure wrongly reads a stale
network state (it captures the `network` value when the effect was created),
causing incorrect "Device came back online" detection; change to track previous
connection state with a ref (e.g., create a React ref prevIsConnectedRef and
update it whenever you setNetwork) and inside the NetInfo.addEventListener
callback compare state.isConnected with prevIsConnectedRef.current (instead of
`network.isConnected`), then after handling logging update
prevIsConnectedRef.current to state.isConnected so `useEffect` no longer relies
on the `network` closure for correct online/offline detection in the `useEffect`
+ `NetInfo.addEventListener` listener.
In `@src/lib/retry.ts`:
- Around line 72-119: The retry function can be called with maxAttempts = 0
which makes the for-loop skip, leaves lastError undefined, and causes throw
undefined; fix this by validating or normalizing maxAttempts at start of retry
(or DEFAULT_OPTIONS merge) so it is at least 1, and if the provided value is
invalid throw a clear Error; update the retry function (refer to function name
retry, local variable maxAttempts, and lastError) to either set maxAttempts =
Math.max(1, maxAttempts) or immediately throw a descriptive Error when
maxAttempts < 1, and ensure the final thrown value is a proper Error (not
undefined) and onFailure/onRetry callbacks receive a meaningful error object.
🧹 Nitpick comments (8)
src/components/error/ErrorBoundary.tsx (2)
52-62: Reset key comparison may miss array length changes.The current comparison only checks if values at existing indices changed. If
resetKeysarray length changes (e.g.,[a, b]→[a, b, c]), the reset won't trigger since indices 0 and 1 still match.♻️ Suggested fix to also detect length changes
componentDidUpdate(prevProps: Props): void { // Reset error state when resetKeys change if (this.state.hasError && this.props.resetKeys) { + const prevKeys = prevProps.resetKeys ?? []; + const currKeys = this.props.resetKeys; + + const hasResetKeyChanged = + prevKeys.length !== currKeys.length || + currKeys.some((key, index) => key !== prevKeys[index]); - const hasResetKeyChanged = this.props.resetKeys.some( - (key, index) => key !== prevProps.resetKeys?.[index] - ); if (hasResetKeyChanged) { this.resetErrorBoundary(); } } }
97-108: Consider addingdisplayNamefor better debugging.The HOC returns a generic function name which makes debugging harder in React DevTools. Adding a
displayNameimproves the developer experience.♻️ Suggested improvement
export function withErrorBoundary<P extends object>( WrappedComponent: React.ComponentType<P>, fallback?: ReactNode ) { - return function WithErrorBoundary(props: P) { + function WithErrorBoundary(props: P) { return ( <ErrorBoundary fallback={fallback}> <WrappedComponent {...props} /> </ErrorBoundary> ); - }; + } + + WithErrorBoundary.displayName = `withErrorBoundary(${WrappedComponent.displayName || WrappedComponent.name || 'Component'})`; + return WithErrorBoundary; }src/lib/retry.ts (1)
186-196: Rate limit detection relies on message content.The
isRateLimitErrorchecks for "429" in the error message, but HTTP errors may carry the status code in a separate property (e.g.,error.statusorerror.statusCode) rather than the message. Consider enhancing detection for common HTTP error shapes.♻️ Optional enhancement
export function isRateLimitError(error: unknown): boolean { if (error instanceof Error) { const message = error.message.toLowerCase(); - return ( + const messageMatch = message.includes('rate limit') || message.includes('too many requests') || - message.includes('429') - ); + message.includes('429'); + + // Check for status code property common in HTTP errors + const status = (error as { status?: number; statusCode?: number }).status ?? + (error as { status?: number; statusCode?: number }).statusCode; + + return messageMatch || status === 429; } return false; }app/_layout.tsx (1)
19-21: Module-level side effects may complicate testing and initialization order.
setupGlobalErrorHandlers()andlogger.info()are invoked during module evaluation. This can cause issues with:
- Testing: side effects run on import
- Initialization order: dependencies may not be ready
- Multiple imports: though bundlers usually dedupe, edge cases exist
Consider moving these calls inside a
useEffectinRootLayoutor using a dedicated initialization pattern.♻️ Suggested refactor
-// Initialize global error handlers -setupGlobalErrorHandlers(); -logger.info('ThumbCode app started'); - function RootLayoutNav() { // ... existing code } export default function RootLayout() { + useEffect(() => { + setupGlobalErrorHandlers(); + logger.info('ThumbCode app started'); + }, []); + return ( <ErrorBoundary>src/lib/error-handler.ts (3)
180-182: Type guard may produce false positives.The check
'code' in error && 'severity' in errorwill match any Error that happens to havecodeandseverityproperties, even if they weren't created bycreateAppError. Consider also checking forerror.name === 'AppError':♻️ Proposed stricter type guard
export function isAppError(error: unknown): error is AppError { - return error instanceof Error && 'code' in error && 'severity' in error; + return ( + error instanceof Error && + error.name === 'AppError' && + 'code' in error && + 'severity' in error + ); }
134-151: String-based error detection is fragile but acceptable.Error message matching (
'Network request failed','timeout') may miss errors with different message formats across platforms or libraries. This is a common pattern and acceptable for initial implementation, but consider documenting the limitation or adding additional detection patterns over time.
240-251: Minor: Comment is inaccurate.The comment says "Handle unhandled promise rejections" but React Native's
ErrorUtilshandles all uncaught JavaScript errors, not just promise rejections. Promise rejections are typically handled separately viaglobal.HermesInternal?.enablePromiseRejectionTrackeror similar.📝 Proposed comment fix
if (errorUtils) { - // Handle unhandled promise rejections + // Handle uncaught JavaScript errors const originalHandler = errorUtils.getGlobalHandler?.();src/lib/logger.ts (1)
212-226: Consider adding a timeout for remote logging requests.The
fetchcall has no timeout, which could cause hanging requests if the remote endpoint is unresponsive. While failures are silently ignored (appropriate for logging), hanging requests accumulate resources.♻️ Proposed fix with AbortController timeout
private async sendToRemote(entry: LogEntry): Promise<void> { if (!this.config.remoteEndpoint) return; const payload = { ...entry, appVersion: this.config.appVersion, environment: this.config.environment, }; + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); + - await fetch(this.config.remoteEndpoint, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), - }); + try { + await fetch(this.config.remoteEndpoint, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + signal: controller.signal, + }); + } finally { + clearTimeout(timeoutId); + } }
| "@anthropic-ai/sdk": "^0.32.0", | ||
| "@react-native-async-storage/async-storage": "^2.0.0", | ||
| "@react-native-community/netinfo": "^11.4.1", | ||
| "@react-navigation/native": "^7.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@react-native-community/netinfo 11.4.1 React Native 0.76.0 compatibility requirements
💡 Result:
Short answer: Yes — @react-native-community/netinfo 11.4.1 is compatible with React Native 0.76. The package supports React Native >= 0.60 (autolinking) and lists react-native as a peer dependency; RN 0.76 meets that requirement. [1][2]
Sources:
- package README / npm (React Native compatibility: supports RN 0.60+ with autolinking). [1]
- package metadata for v11.4.1 (peerDependency react-native; v11.4.1 published). [2]
References:
[1] npm package README for @react-native-community/netinfo.
[2] jsDocs / npm metadata for @react-native-community/netinfo v11.4.1.
🌐 Web query:
@react-native-async-storage/async-storage 2.0.0 React Native 0.76.0 compatibility
💡 Result:
Short answer: Generally yes — @react-native-async-storage/async-storage v2.x supports React Native >=0.60 (so it should work with RN 0.76.0). [1][2]
Notes and cautions:
- The official docs state it requires RN 0.60+ (autolinking etc.). [1]
- There are reported Android build issues for version 2.0.0 related to Kotlin/KSP/Kotlin-2 toolchain (you may hit "ksp is too old for kotlin-2" errors). If you use Async Storage 2.0.0 on RN 0.76.0 and your Android toolchain uses Kotlin 2.x, you may need to either upgrade the library to a later patch that fixes Kotlin compatibility or align/downgrade your Kotlin/KSP versions per the issue discussion. [2]
- The npm package currently has newer 2.x releases (e.g., 2.2.0); prefer the latest 2.x patch release to avoid known 2.0.0 bugs. [3]
Recommendations:
- Use the latest
@react-native-async-storage/async-storage2.x (not exactly 2.0.0) if possible. [3] - If you must stay on 2.0.0 and see Kotlin/KSP errors, align your Android Kotlin/KSP plugin to the versions compatible with the library (or downgrade Kotlin to 1.9.x) as discussed in the GitHub issue. [2]
Sources:
[1] Async Storage install docs (requires RN 0.60+).
[2] GitHub issue reporting Kotlin/KSP compatibility with version 2.0.0.
[3] npm package page showing newer 2.x releases (e.g., 2.2.0).
Upgrade @react-native-async-storage/async-storage to the latest 2.x patch version (≥2.2.0).
While @react-native-community/netinfo@^11.4.1 is compatible with React Native 0.76.0, @react-native-async-storage/async-storage@2.0.0 has documented Android build issues with Kotlin 2.x/KSP toolchains (reported as "ksp is too old for kotlin-2" errors). Upgrade to a newer 2.x patch release (e.g., 2.2.0 or later) to avoid potential Android build failures.
🤖 Prompt for AI Agents
In `@package.json` around lines 35 - 38, Update the dependency version for
`@react-native-async-storage/async-storage` in package.json from "^2.0.0" to a
newer 2.x patch (e.g., "^2.2.0" or later) to avoid Kotlin/KSP Android build
issues; edit the dependency entry for
"@react-native-async-storage/async-storage" in package.json and run your package
manager's install command to lock the updated version.
- Add onPress handler to Report Issue button in ErrorFallback - Add onReportIssue prop for custom report handling - Fix stale closure in useNetworkError by using ref for previous state - Validate maxAttempts in retry function (normalize to at least 1) - Initialize lastError to avoid throwing undefined - Remove unused type import in error-handler tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
1 participant
Summary
This PR implements a comprehensive error handling and logging system for ThumbCode, addressing Issue #16 (CRITICAL priority).
Key Features
Logging Service (
src/lib/logger.ts)Error Boundary Components (
src/components/error/)ErrorBoundary- React error boundary that catches errors in child componentsErrorFallback- Full-screen error display with organic design stylingCompactErrorFallback- Inline error display for smaller UI sectionswithErrorBoundaryHOC for easy wrapping of componentsGlobal Error Handler (
src/lib/error-handler.ts)AppErrortype with standardized structureparseError()for converting any error to AppErrorhandleError()for consistent error processingRetry Utility (
src/lib/retry.ts)withRetrywrapper for creating retryable functionsNetwork Error Hook (
src/hooks/use-network-error.ts)useNetworkError- Full network state monitoring with error handlinguseIsOnline- Simple connectivity check hookIntegration
Test Plan
Closes #16
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.