-
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?
Changes from 6 commits
0f5347c
85e70c1
205ca1b
f35b258
86966f7
393e74a
5a4d0e5
5903d7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
<?php | ||
/** | ||
* Copyright 2017 Adobe All rights reserved. | ||
* See COPYING.txt for license details. | ||
* Copyright 2017 Adobe | ||
* All Rights Reserved. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
|
@@ -16,6 +16,8 @@ | |
use Magento\Framework\Setup\Declaration\Schema\Sharding; | ||
use Magento\Framework\Config\FileResolverByModule; | ||
use Magento\Framework\Setup\Declaration\Schema\Declaration\ReaderComposite; | ||
use Psr\Log\LoggerInterface; | ||
use Magento\Framework\Exception\LocalizedException; | ||
|
||
/** | ||
* This type of builder is responsible for converting ENTIRE data, that comes from db | ||
|
@@ -27,6 +29,7 @@ | |
* | ||
* @see Schema | ||
* @inheritdoc | ||
* @SuppressWarnings(PHPMD.CouplingBetweenObjects) | ||
*/ | ||
class SchemaBuilder | ||
{ | ||
|
@@ -55,30 +58,39 @@ class SchemaBuilder | |
*/ | ||
private $readerComposite; | ||
|
||
/** | ||
* @var LoggerInterface | ||
*/ | ||
private $logger; | ||
|
||
/** | ||
* Constructor. | ||
* | ||
* @param ElementFactory $elementFactory | ||
* @param DbSchemaReaderInterface $dbSchemaReader | ||
* @param Sharding $sharding | ||
* @param ReaderComposite $readerComposite | ||
* @param LoggerInterface $logger | ||
*/ | ||
public function __construct( | ||
ElementFactory $elementFactory, | ||
DbSchemaReaderInterface $dbSchemaReader, | ||
Sharding $sharding, | ||
ReaderComposite $readerComposite | ||
ReaderComposite $readerComposite, | ||
LoggerInterface $logger | ||
) { | ||
$this->elementFactory = $elementFactory; | ||
$this->dbSchemaReader = $dbSchemaReader; | ||
$this->sharding = $sharding; | ||
$this->readerComposite = $readerComposite; | ||
$this->logger = $logger; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
* @SuppressWarnings(PHPMD.CyclomaticComplexity) | ||
* @SuppressWarnings(PHPMD.NPathComplexity) | ||
* @throws LocalizedException | ||
*/ | ||
public function build(Schema $schema) | ||
{ | ||
|
@@ -88,8 +100,22 @@ public function build(Schema $schema) | |
foreach ($data['table'] as $keyTable => $tableColumns) { | ||
$tableColumns['column'] ??= []; | ||
foreach ($tableColumns['column'] as $keyColumn => $columnData) { | ||
if ($columnData['type'] == 'json') { | ||
$tablesWithJsonTypeField[$keyTable] = $keyColumn; | ||
try { | ||
if ($columnData['type'] == 'json') { | ||
$tablesWithJsonTypeField[$keyTable] = $keyColumn; | ||
} | ||
} catch (\Throwable $e) { | ||
// Create a new exception with extended context message | ||
$errorMessage = sprintf( | ||
"%s\nError processing table %s column %s", | ||
$e->getMessage(), | ||
$keyTable, | ||
$keyColumn | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Hello @engcom-Hotel , the check is already there at line 101 |
||
); | ||
$this->logger->error($errorMessage); | ||
// Throw a new exception with the extended message | ||
// This preserves the original error but adds our context | ||
throw new LocalizedException(new Phrase($errorMessage)); | ||
} | ||
} | ||
} | ||
|
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.
New Params in Constructor should be null and assign it with ObjectManager