orm
orm copied to clipboard
[Feature Request] Add source to ConversionException
Feature Request
Q | A |
---|---|
New Feature | yes |
RFC | yes/no |
BC Break | no? |
Summary
It would be helpful if the thrown ConversionException included the table and field of the source of the conversion exception:
Could not convert database value "7/18/25" to Doctrine Type datestr_immutable. Expected format: Y-m-d
#0 src\Doctrine\DBAL\Types\DateStrImmutableType.php(***): Doctrine\DBAL\Types\ConversionException::conversionFailedFormat()
#1 vendor\doctrine\orm\src\Internal\Hydration\AbstractHydrator.php(322): App\Doctrine\DBAL\Types\DateStrImmutableType->convertToPHPValue()
#2 vendor\doctrine\orm\src\Internal\Hydration\ObjectHydrator.php(323): Doctrine\ORM\Internal\Hydration\AbstractHydrator->gatherRowData()
#3 vendor\doctrine\orm\src\Internal\Hydration\ObjectHydrator.php(143): Doctrine\ORM\Internal\Hydration\ObjectHydrator->hydrateRowData()
#4 vendor\doctrine\orm\src\Internal\Hydration\AbstractHydrator.php(168): Doctrine\ORM\Internal\Hydration\ObjectHydrator->hydrateAllData()
#5 vendor\doctrine\orm\src\Persisters\Entity\BasicEntityPersister.php(912): Doctrine\ORM\Internal\Hydration\AbstractHydrator->hydrateAll()
#6 vendor\doctrine\orm\src\EntityRepository.php(110): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->loadAll()
#7 vendor\doctrine\doctrine-bundle\src\Repository\ServiceEntityRepositoryProxy.php(72): Doctrine\ORM\EntityRepository->findBy()
#8 src\Controller\MyController.php(***): Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepositoryProxy->findBy()
#11 vendor\symfony\http-kernel\HttpKernel.php(183): App\Controller\MyController->getObject()
#12 vendor\symfony\http-kernel\HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#13 vendor\symfony\http-kernel\Kernel.php(182): Symfony\Component\HttpKernel\HttpKernel->handle()
#14 vendor\symfony\runtime\Runner\Symfony\HttpKernelRunner.php(35): Symfony\Component\HttpKernel\Kernel->handle()
#15 vendor\autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
#16 public\index.php(12): require_once('...')
#17 {main}
This is a somewhat poor example, given that I'm using a custom type, and I should probably just fix my App\Doctrine\DBAL\Types\DateStrImmutableType->convertToPHPValue()
to handle the "m/d/Y" format, but regardless of that minor nit, it turns out the offending field was 4 layers of relationships deep from the originally requested object.
It would be greatly beneficial, imo, to add some sort of improved messaging to the exception.
Proposed Fix
My quick initial naive implementation I did as a test to see if this was even actionable:
- wrap the entire switch statement in Doctrine\ORM\Internal\Hydration\AbstractHydrator->gatherRowData in a try/catch block:
catch (\Doctrine\DBAL\Types\ConversionException $ex) {
throw new \Doctrine\DBAL\Types\ConversionException(\sprintf('%s, in field "%s.%s"', $ex->getMessage(), $dqlAlias, $fieldName), previous: $ex);
}
- add a catch block to AbstractHydrator->hydrateAll:
catch (\Doctrine\DBAL\Types\ConversionException $ex) {
throw new \Doctrine\DBAL\Types\ConversionException(
str_replace(
array_map(fn($e) => "\"$e.", array_keys($resultSetMapping->aliasMap)),
array_map(fn($e) => "\"$e.", array_values($resultSetMapping->aliasMap)),
$ex->getMessage()
),
previous: $ex->getPrevious()
);
}
- Resulting error message:
Could not convert database value "7/18/25" to Doctrine Type datestr_immutable. Expected format: Y-m-d, in field "App\Doctrine\Entity\External\[TABLE].[FIELD]"
A more proper fix would be to either
a. add className
and fieldName
properties to \Doctrine\DBAL\Types\ConversionException
, or
b. extend ConversionException
and add those properties to the extended class (in some ORM exception namespace)
However, there are a few possible issues I can foresee:
-
ObjectHydrator
has a internal note: "Highly performance-sensitive code." I'm not sure what the impact of adding the try-catch exception handling would have ongatherRowData
. It could (should?) probably be hoisted up to wrap theforeach
block, as$fieldName
would be in scope by the time theConversionException
happens. - ~~
$dqlAlias
only exists in the default switch case. I don't know how to handle the other two cases. Maybe just the field name, and~~ thehydrateAll
catch block tries to figure out the column owner, a la$rsm->declaringClasses[array_search($ex->field, $rsm->fieldMappings, true)]
?
What are everyone's thoughts on this? I'd be willing to submit a PR if there's any interest, or rather lack of opposition, to adding this feature.