-
Notifications
You must be signed in to change notification settings - Fork 9.4k
improve error handling SchemaBuilder #39799
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
base: 2.4-develop
Are you sure you want to change the base?
Conversation
Extending error context : Previous: ```Warning: Undefined array key "type" in /var/www/html/magento/vendor/magento/framework/Setup/Declaration/Schema/Db/SchemaBuilder.php on line 91``` Now: ```Warning: Undefined array key "type" in /var/www/html/magento/vendor/magento/framework/Setup/Declaration/Schema/Db/SchemaBuilder.php on line 91 Error processing table catalog_eav_attribute column is_used_in_autocomplete``` Help undersend the issue without heavy debugging!
Hi @Genaker. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
For tables that are defined without columns and marked with You could consider adding the following check in your processing logic, e.g., around line 89: |
is it for me? Who should add? |
@magento create issue |
@magento run all tests |
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.
Hello @Genaker,
Thanks for the contribution!
Please refer to the below review points & fix the failed static tests. Other failures seems flaky to me.
Thanks
|
||
// Throw a new exception with the extended message | ||
// This preserves the original error but adds our context | ||
throw new \Exception($errorMessage, $e->getCode(), $e); |
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.
With throwing the error also ensure that the new error message is properly surfaced in logs.
"%s\nError processing table %s column %s", | ||
$e->getMessage(), | ||
$keyTable, | ||
$keyColumn |
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.
As per this comment #39799 (comment), please add a check with table is defined without columns.
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.
Hello @engcom-Hotel , the check is already there at line 101
|
||
// Throw a new exception with the extended message | ||
// This preserves the original error but adds our context | ||
throw new \Exception($errorMessage, $e->getCode(), $e); |
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.
Instead of rethrowing a generic \Exception
, consider a specific exception type, e.g., \Magento\Framework\Exception\LocalizedException
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.
unfortunately i can't do this now. However feel free edit this PR.
@magento run all tests |
@magento run all tests |
Hello @engcom-Hotel I have fixed the review comments, static & unit test failures. Could you please review it ? |
@magento run Database Compare, Integration Tests, WebAPI Tests |
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.
Extending error context :
Previousle:
Warning: Undefined array key "type" in /var/www/html/magento/vendor/magento/framework/Setup/Declaration/Schema/Db/SchemaBuilder.php on line 91
Few Moments later:
Warning: Undefined array key "type" in /var/www/html/magento/vendor/magento/framework/Setup/Declaration/Schema/Db/SchemaBuilder.php on line 91 Error processing table catalog_eav_attribute column is_used_in_autocomplete
Help undersend the issue without heavy debugging!
Description (*)
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)
Resolved issues: