-
Notifications
You must be signed in to change notification settings - Fork 0
Implemented Exceptions #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive error handling for MFiles API responses. When API responses indicate failure, the code now throws a custom MFilesErrorException with detailed error information instead of proceeding with potentially invalid data.
- Introduced new DTOs (
MFilesError,Exception,InnerException) to represent error structures from the MFiles API - Created
MFilesErrorExceptionto wrap error responses in a custom exception type - Added error checking in response handlers to throw exceptions on unsuccessful responses
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DTO/MFilesError.php | New DTO representing MFiles API error responses with status, message, and exception details |
| src/DTO/Exception.php | New DTO for exception data within error responses |
| src/DTO/InnerException.php | New DTO for nested inner exception data |
| src/Exceptions/MFilesErrorException.php | Custom exception class that wraps MFilesError DTOs |
| src/Responses/ErrorResponse.php | Response handler to transform error JSON into MFilesError DTOs |
| src/Responses/UploadFileResponse.php | Added error checking before processing response |
| src/Responses/ObjectPropertiesResponse.php | Added error checking before processing response |
| src/Responses/LogInToVaultResponse.php | Added error checking before processing response |
| src/Responses/DownloadFileResponse.php | Added error checking before processing response |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| if (! $response->successful()) { | ||
| throw new MFilesErrorException(ErrorResponse::createDtoFromResponse($response)); | ||
| } |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Missing blank line after the error check block. For consistency with other files in this PR (UploadFileResponse.php, ObjectPropertiesResponse.php, LogInToVaultResponse.php), add a blank line after line 17 before line 18.
| } | |
| } |
| public function __construct( | ||
| public readonly MFilesError $error | ||
| ) { | ||
| parent::__construct($error->message, $error->status); |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second parameter to Exception's constructor is the error code, not the HTTP status code. Exception expects an integer code (typically 0 by default), but using HTTP status codes (200, 404, 500, etc.) as exception codes is semantically incorrect. The status code should be stored separately or used in the message. Consider using 0 or a specific application error code as the exception code instead.
| public function __construct( | |
| public readonly MFilesError $error | |
| ) { | |
| parent::__construct($error->message, $error->status); | |
| public readonly int $status; | |
| public function __construct( | |
| public readonly MFilesError $error | |
| ) { | |
| parent::__construct($error->message, 0); | |
| $this->status = $error->status; |
…r-ag/laravel-m-files into feature-adding-exception-handling
No description provided.