feat: Add centralized ErrorService for consistent error handling#723
feat: Add centralized ErrorService for consistent error handling#723TenFinges wants to merge 3 commits intoAOSSIE-Org:devfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
🎉 Welcome @TenFinges!
We appreciate your contribution! 🚀 |
|
Please note that I've added the new error strings in English only. Previous contributors (or any native speakers) could contribute translations for Bengali, Hindi, Gujarati, Kannada, Marathi, and Punjabi. |
M4dhav
left a comment
There was a problem hiding this comment.
Please fix merge conflicts
776ff7f to
c6e2639
Compare
c6e2639 to
78904fc
Compare
| /// Categorizes an error into an [ErrorType] based on its type and message. | ||
| static ErrorType _categorizeError(dynamic error) { | ||
| if (error is AppwriteException) { | ||
| return _categorizeAppwriteError(error); | ||
| } | ||
|
|
||
| final errorString = error.toString().toLowerCase(); | ||
|
|
||
| if (errorString.contains('network') || | ||
| errorString.contains('socket') || | ||
| errorString.contains('connection') || | ||
| errorString.contains('timeout')) { | ||
| return ErrorType.network; | ||
| } | ||
|
|
||
| if (errorString.contains('auth') || | ||
| errorString.contains('credential') || | ||
| errorString.contains('password') || | ||
| errorString.contains('login')) { | ||
| return ErrorType.authentication; | ||
| } | ||
|
|
||
| if (errorString.contains('storage') || | ||
| errorString.contains('file') || | ||
| errorString.contains('upload')) { | ||
| return ErrorType.storage; | ||
| } | ||
|
|
||
| return ErrorType.general; |
There was a problem hiding this comment.
While this does seem logical enough, could you please share examples of In App Errors and how they would be segregated?
There was a problem hiding this comment.
Here are a few examples of how errors are categorized:
- Network (
ErrorType.network): When a user's device loses connectivity while joining a room (rooms_controller.dart:joinRoom), the Appwrite SDK throws aSocketException: Connection refused— matchessocketandconnection. When a LiveKit WebSocket connection drops during pair chat (livekit_controller.dart), we get aTimeoutException— matchestimeout. - Authentication (
ErrorType.authentication): When Google/GitHub OAuth fails inauthentication_controller.dart:loginWithGoogle(), the error message typically containsauth— categorized as authentication. Also happens when a session expires mid-use andaccount.get()fails with anautherror. - Storage (
ErrorType.storage): When a profile image upload fails inedit_profile_controller.dart:saveProfile()viastorage.createFile(), any non-Appwrite error containingfileoruploadin its message (e.g.,FileSystemException). - General (
ErrorType.general): Fallback for things like aFormatExceptionwhen parsing room data inrooms_controller.dart(doesn't match any keyword), or aRangeError/ unexpected runtime exception.
I removed ErrorType.database and ErrorType.validation from _categorizeError. In our app, database and validation errors originate strictly from Appwrite (document_not_found, general_argument_invalid, etc.) and are properly handled by _categorizeAppwriteError via structured type/code fields. Non-Appwrite Dart exceptions don't produce database or validation errors in our current codebase, so they correctly fall through to ErrorType.general.
| static ErrorType _categorizeAppwriteError(AppwriteException error) { | ||
| final type = error.type ?? ''; | ||
| final code = error.code ?? 0; | ||
|
|
||
| // Authentication errors | ||
| if (type.contains('user_') || | ||
| type.contains('session_') || | ||
| code == 401 || | ||
| code == 403) { | ||
| return ErrorType.authentication; | ||
| } | ||
|
|
||
| // Database/document errors | ||
| if (type.contains('document_') || type.contains('database_')) { | ||
| return ErrorType.database; | ||
| } | ||
|
|
||
| // Storage errors | ||
| if (type.contains('storage_') || type.contains('file_')) { | ||
| return ErrorType.storage; | ||
| } | ||
|
|
||
| // Validation errors |
There was a problem hiding this comment.
Same for this, please share examples of specific errors and how they would be segregated here
There was a problem hiding this comment.
-
Authentication (
ErrorType.authentication):- Login with wrong password (
authentication_controller.dart:login):type: 'user_invalid_credentials',code: 401— matchestype.contains('user_'). - Expired session when fetching rooms or profile:
type: 'user_session_not_found',code: 401— matchestype.contains('session_')andcode == 401. - Unauthorized access to another user's data:
code: 403— matchescode == 403.
- Login with wrong password (
-
Database (
ErrorType.database):- Fetching a deleted room in
rooms_controller.dart:getRoomById():type: 'document_not_found',code: 404— matchestype.contains('document_'). - Deleting an already-ended pair chat in
pair_chat_controller.dart:endChat()(already handled explicitly there fordocument_not_found): matchestype.contains('document_'). - Database query fails when loading user list:
type: 'database_not_found'— matchestype.contains('database_').
- Fetching a deleted room in
-
Storage (
ErrorType.storage):- Profile image upload fails in
edit_profile_controller.dart:saveProfile()viastorage.createFile():type: 'storage_file_type_unsupported'— matchestype.contains('storage_'). - Deleting a nonexistent profile image via
storage.deleteFile():type: 'file_not_found'— matchestype.contains('file_').
- Profile image upload fails in
-
Validation (
ErrorType.validation):- Short password during login/signup (
authentication_controller.dart):type: 'general_argument_invalid',code: 400— matches both the type check andcode == 400. - Invalid email format during
change_email_controller.dart:changeEmailInAuth(): same type.
- Short password during login/signup (
-
Network (
ErrorType.network):- Device is offline: Appwrite SDK returns
code: 0— matchescode == 0. - Appwrite server down:
code: 503orcode: 504— matches those code checks.
- Device is offline: Appwrite SDK returns
Description
Added a centralized ErrorService to handle errors consistently across the app. This replaces scattered error handling with a unified approach that provides user-friendly messages, accessibility support, and centralized logging.
Key changes:
Fixes #722
Type of change
How Has This Been Tested?
Checklist:
Maintainer Checklist