Skip to content
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

EntityRemoveException should leverage the previous: $exception constructor parameters #6615

Open
allan-simon opened this issue Dec 3, 2024 · 2 comments
Labels
Milestone

Comments

@allan-simon
Copy link

Short description of what this feature will allow to do:

It will allow to have a more precise context of the original exception

Currently for EntityRemoveException (and certainly other exception in easyadmin bundle) the usage is the following

        } catch (ForeignKeyConstraintViolationException $e) {

            throw new EntityRemoveException(['entity_name' => $context->getEntity()->getName(), 'message' => $e->getMessage()]);

        }

i.e

        } catch (CodeException $e) {
            throw new BusinessException([]);
        }

This allow to have the original CodeException message but not it's stacktrace , so information are loose

instead, PHP already have a builtin mechanism for that , which is the previous: Exception $e parameter in Exception constructors which allow to do the following construction (adminting you don't override the constructor

        } catch (CodeException $e) {
            throw new BusinessException('my business message',  previous: $e );
        }

This allow to

  1. keep both the business and code stacktrace
  2. allow you to not have to reconstruct a "original message : %s"

(One short paragraph is enough)

Example of how to use this feature
(Show some PHP code and/or YAML config explaining how to use this feature in a real app)

transform

final class EntityRemoveException extends BaseException
{
    public function __construct(array $parameters = [])
    {
        $exceptionContext = new ExceptionContext(
            'exception.entity_remove',
            sprintf('There is a ForeignKeyConstraintViolationException for the Doctrine entity associated with "%s". Solution: disable the "delete" action for this CRUD controller or configure the "cascade={"remove"}" attribute for the related field in the Doctrine entity. Full exception: %s', $parameters['entity_name'], $parameters['message']),
            $parameters,
            409
        );

        parent::__construct($exceptionContext);
    }
}

into

final class EntityRemoveException extends BaseException
{
    public function __construct(string $entityName, Exception $previous)
    {
        $exceptionContext = new ExceptionContext(
            'exception.entity_remove',
            sprintf('There is a ForeignKeyConstraintViolationException for the Doctrine entity associated with "%s". Solution: disable the "delete" action for this CRUD controller or configure the "cascade={"remove"}" attribute for the related field in the Doctrine entity.', $parameters['entity_name']),
            $parameters,
            409
        );

        parent::__construct($exceptionContext, previous $previous);
    }
}

so that you could then do

        } catch (ForeignKeyConstraintViolationException $e) {

            throw new EntityRemoveException($context->getEntity()->getName(),  previous: $e);

        }
@allan-simon
Copy link
Author

if you want I can propose a PR

@javiereguiluz
Copy link
Collaborator

@allan-simon fantastic and detailed explanation! Thanks a lot Allan. Yes, I think that your proposal makes a lot of sense. If you can make a PR as you proposed, that would be even more amazing. Thanks a lot!

@javiereguiluz javiereguiluz added this to the 4.x milestone Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants