feat: enhance exception handling with standardized error codes#155
Merged
feat: enhance exception handling with standardized error codes#155
Conversation
Added new constants for common HTTP error codes in the Exception class. Updated various parts of the codebase to utilize these constants for improved error handling, including specific validation and internal error messages in the Appwrite, JSON, and CSV source and destination classes.
Contributor
WalkthroughAdds HTTP-like numeric error code constants to Migration\Exception and updates exception construction across migration sources, destinations, and Target to attach explicit integer codes and preserve original throwables as previous exceptions. Validation and internal-error cases are classified (e.g., CODE_VALIDATION, CODE_INTERNAL, CODE_NOT_FOUND, CODE_UNAUTHORIZED). Multiple sources (Appwrite, Firebase, CSV, JSON, NHost, Supabase, CSV/Local destinations) and Target now coerce, map, or compute status codes (NHost adds helper mapping methods) and rethrow errors with standardized codes while retaining original messages and chaining. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Migration/Sources/Appwrite.php`:
- Around line 209-213: The 403 branch currently throws a new Exception without
preserving the original code or previous exception; update the branch in
Appwrite.php so the thrown exception includes the 403 status code and the
original exception as the previous (mirror how the else branch forwards
$e->getCode() and $e as previous). Locate the conditional that checks if
($e->getCode() === 403) and change the throw to include the code and $e as the
previous exception so callers see the standardized code and original stack.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd4a39a7-c212-4cf1-995d-91ad6eeed953
📒 Files selected for processing (7)
src/Migration/Destinations/Appwrite.phpsrc/Migration/Destinations/JSON.phpsrc/Migration/Exception.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/JSON.phpsrc/Migration/Target.php
… across migration sources and destinations
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/Migration/Sources/Supabase.php (2)
379-385:⚠️ Potential issue | 🟠 MajorUse auth metadata in the
exportUsers()error wrapper.This catch still records user export failures as
TYPE_BUCKETinGROUP_STORAGE, so the new status code is attached to the wrong resource and the wrong group.💡 Minimal fix
} catch (\Throwable $e) { $this->addError(new Exception( - Resource::TYPE_BUCKET, - Transfer::GROUP_STORAGE, + Resource::TYPE_USER, + Transfer::GROUP_AUTH, message: $e->getMessage(), code: parent::statusCodeFromThrowable($e), previous: $e )); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Sources/Supabase.php` around lines 379 - 385, The catch in exportUsers() wraps user export failures with the wrong resource/group (uses Resource::TYPE_BUCKET and Transfer::GROUP_STORAGE); change the addError call inside the catch to use the auth/user metadata constants (e.g. Resource::TYPE_USER and Transfer::GROUP_AUTH) while preserving message: $e->getMessage(), code: parent::statusCodeFromThrowable($e), and previous: $e so the status is attached to the correct resource/group.
467-473:⚠️ Potential issue | 🟠 MajorTag
exportFiles()failures as file errors.This wrapper still reports file-export failures as
TYPE_BUCKET, which makes downstream error reporting collapse bucket and file failures together.💡 Minimal fix
} catch (\Throwable $e) { $this->addError(new Exception( - Resource::TYPE_BUCKET, + Resource::TYPE_FILE, Transfer::GROUP_STORAGE, message: $e->getMessage(), code: parent::statusCodeFromThrowable($e), previous: $e )); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Sources/Supabase.php` around lines 467 - 473, The catch in exportFiles() currently tags failures as Resource::TYPE_BUCKET; update that call so the created Exception uses Resource::TYPE_FILE (or the codebase's file-specific resource constant) instead of TYPE_BUCKET so exportFiles() errors are classified as file errors; locate the addError(new Exception(...)) invocation in the exportFiles() catch block in Supabase.php and replace the Resource::TYPE_BUCKET argument with the file resource constant while leaving the other arguments (Transfer::GROUP_STORAGE, message, code, previous) unchanged.src/Migration/Destinations/Appwrite.php (1)
667-717:⚠️ Potential issue | 🟠 MajorDon't drop the standardized code during column rollback.
Line 701 now throws
CODE_INTERNAL, but the rollback catch immediately replaces it with a freshUtopia\Migration\Exceptionthat has code0and no previous throwable. This path still loses the new classification.💡 Minimal fix
- } catch (\Throwable) { + } catch (\Throwable $e) { $this->database->deleteDocument('attributes', $column->getId()); if (isset($twoWayAttribute)) { $this->database->deleteDocument('attributes', $twoWayAttribute->getId()); } throw new Exception( resourceName: $resource->getName(), resourceGroup: $resource->getGroup(), resourceId: $resource->getId(), message: 'Failed to create column', + code: (int) $e->getCode() ?: Exception::CODE_INTERNAL, + previous: $e, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Destinations/Appwrite.php` around lines 667 - 717, The rollback catch currently swallows the original Throwable (from createAttribute or createRelationship) and replaces it with a new Utopia\Migration\Exception with code 0 and no previous throwable; change the catch to catch (\Throwable $e) and when throwing the new Exception in the catch include the original $e as the previous throwable (or at minimum preserve its code via $e->getCode()) so the original error classification and stack are not lost — update the catch block around createAttribute/createRelationship (referencing createAttribute, createRelationship, deleteDocument and the thrown Exception) to attach $e as the previous exception or copy its code into the new Exception.src/Migration/Sources/Firebase.php (1)
39-43:⚠️ Potential issue | 🟠 MajorValidate
project_idbefore assigning$this->projectID.Line 147 validation in
report()is unreachable whenproject_idis missing. The constructor at line 42 directly dereferences$serviceAccount['project_id']into the typedstringproperty$projectID, which fails beforereport()executes. Move validation to the constructor to throw the standardized exception early.Suggested fix
public function __construct(array $serviceAccount) { + if (!isset($serviceAccount['project_id']) || !\is_string($serviceAccount['project_id']) || $serviceAccount['project_id'] === '') { + throw new \Exception('Invalid Firebase Service Account', Exception::CODE_UNAUTHORIZED); + } + $this->serviceAccount = $serviceAccount; $this->projectID = $serviceAccount['project_id']; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Sources/Firebase.php` around lines 39 - 43, The constructor in class Firebase currently dereferences $serviceAccount['project_id'] into the typed property $projectID which causes an unhandled error when project_id is absent; move the validation from report() into the __construct(array $serviceAccount) so you explicitly check isset($serviceAccount['project_id']) and that it is a non-empty string, and if not throw the same standardized exception used in report() (preserve the exception type/message) before assigning $this->projectID, ensuring $this->serviceAccount is still set as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Migration/Destinations/Appwrite.php`:
- Around line 667-717: The rollback catch currently swallows the original
Throwable (from createAttribute or createRelationship) and replaces it with a
new Utopia\Migration\Exception with code 0 and no previous throwable; change the
catch to catch (\Throwable $e) and when throwing the new Exception in the catch
include the original $e as the previous throwable (or at minimum preserve its
code via $e->getCode()) so the original error classification and stack are not
lost — update the catch block around createAttribute/createRelationship
(referencing createAttribute, createRelationship, deleteDocument and the thrown
Exception) to attach $e as the previous exception or copy its code into the new
Exception.
In `@src/Migration/Sources/Firebase.php`:
- Around line 39-43: The constructor in class Firebase currently dereferences
$serviceAccount['project_id'] into the typed property $projectID which causes an
unhandled error when project_id is absent; move the validation from report()
into the __construct(array $serviceAccount) so you explicitly check
isset($serviceAccount['project_id']) and that it is a non-empty string, and if
not throw the same standardized exception used in report() (preserve the
exception type/message) before assigning $this->projectID, ensuring
$this->serviceAccount is still set as needed.
In `@src/Migration/Sources/Supabase.php`:
- Around line 379-385: The catch in exportUsers() wraps user export failures
with the wrong resource/group (uses Resource::TYPE_BUCKET and
Transfer::GROUP_STORAGE); change the addError call inside the catch to use the
auth/user metadata constants (e.g. Resource::TYPE_USER and Transfer::GROUP_AUTH)
while preserving message: $e->getMessage(), code:
parent::statusCodeFromThrowable($e), and previous: $e so the status is attached
to the correct resource/group.
- Around line 467-473: The catch in exportFiles() currently tags failures as
Resource::TYPE_BUCKET; update that call so the created Exception uses
Resource::TYPE_FILE (or the codebase's file-specific resource constant) instead
of TYPE_BUCKET so exportFiles() errors are classified as file errors; locate the
addError(new Exception(...)) invocation in the exportFiles() catch block in
Supabase.php and replace the Resource::TYPE_BUCKET argument with the file
resource constant while leaving the other arguments (Transfer::GROUP_STORAGE,
message, code, previous) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c73f54a-0e4c-47b3-8406-45b1a035fc08
📒 Files selected for processing (10)
src/Migration/Destinations/Appwrite.phpsrc/Migration/Destinations/CSV.phpsrc/Migration/Destinations/Local.phpsrc/Migration/Sources/Appwrite.phpsrc/Migration/Sources/Appwrite/Reader/Database.phpsrc/Migration/Sources/CSV.phpsrc/Migration/Sources/Firebase.phpsrc/Migration/Sources/JSON.phpsrc/Migration/Sources/NHost.phpsrc/Migration/Sources/Supabase.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Migration/Sources/CSV.php
- src/Migration/Sources/JSON.php
abnegate
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added new constants for common HTTP error codes in the Exception class. Updated various parts of the codebase to utilize these constants for improved error handling, including specific validation and internal error messages in the Appwrite, JSON, and CSV source and destination classes.
Summary by CodeRabbit
Bug Fixes
Refactor