🧹 [Refactor] Replace console.error with proper error handling in CalendarScreen#57
🧹 [Refactor] Replace console.error with proper error handling in CalendarScreen#57TargetMisser wants to merge 2 commits intomainfrom
Conversation
Co-authored-by: TargetMisser <52361977+TargetMisser@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Refactors CalendarScreen error handling to use the shared handleError utility for more consistent logging/alert behavior across calendar and import flows.
Changes:
- Replaced several
console.error/console.warn/Alert.alertcatch blocks withhandleError(...). - Updated catch clauses to use
unknownand (in some cases) wrap errors with a higher-level message/context. - Marked some error paths as “silent” (no user alert) via
handleError(..., ..., true).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (e: unknown) { | ||
| handleError(e instanceof Error ? new Error(`Errore (${step}): ${e.message}`, { cause: e }) : e, 'import'); | ||
| } |
There was a problem hiding this comment.
In this catch, the user-facing message includes the step only when the thrown value is an Error. If something throws a non-Error (string/object), handleError will end up alerting String(e) and the step context is lost compared to the previous behavior. Consider always wrapping into a new Error that includes step (and attach the original value as cause) so the alert stays consistent and actionable.
| } catch (e: unknown) { | ||
| handleError(e instanceof Error ? new Error('Errore nel parsing del PDF', { cause: e }) : e, 'import'); | ||
| setImportModalVisible(false); | ||
| setImportStep('idle'); | ||
| } |
There was a problem hiding this comment.
This catch only wraps the error (with the friendly message) when e is an Error; for non-Error throws the alert message becomes String(e) instead of the intended "Errore nel parsing del PDF". Consider always passing a user-friendly Error message here (and attach the original thrown value as cause) so the user sees a consistent, localized message regardless of what was thrown.
| if (__DEV__) console.error(e); | ||
| Alert.alert('Errore', t('calImportError')); | ||
| } catch (e: unknown) { | ||
| handleError(e instanceof Error ? new Error(t('calImportError'), { cause: e }) : e, 'import'); |
There was a problem hiding this comment.
Similar to the other catch blocks: if a non-Error value is thrown, this will bypass the friendly t('calImportError') message and show String(e) to the user. Consider always wrapping with the translated message (and storing the original thrown value as cause) so the user-visible alert remains consistent.
| handleError(e instanceof Error ? new Error(t('calImportError'), { cause: e }) : e, 'import'); | |
| handleError( | |
| new Error(t('calImportError'), { | |
| cause: e instanceof Error ? e : new Error(String(e)), | |
| }), | |
| 'import', | |
| ); |
Co-authored-by: TargetMisser <52361977+TargetMisser@users.noreply.github.com>
🎯 What: Replaced individual local error handling (
console.error,Alert.alert,console.warn) with the centralizedhandleErrorutility fromsrc/utils/errorHandler.tsinsidesrc/screens/CalendarScreen.tsx.💡 Why: Using a centralized error handler improves error tracking and consistency across the app, ensuring that errors are logged uniformly and displayed to the user via UI alerts where appropriate.
✅ Verification: Ran
npm run typecheckandnpx jest --passWithNoTestssuccessfully. Also passed original error objects using the{ cause: e }syntax to preserve original stack traces.✨ Result: Improved maintainability, cleaner code and consistent error handling throughout the Calendar screen.
PR created automatically by Jules for task 9463199734765884671 started by @TargetMisser