Skip to content

fix(doctrine): Add a proper exception when a doctrine manager could not be found for a resource class #6995

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

Merged
merged 2 commits into from
Mar 7, 2025

Conversation

lyrixx
Copy link
Contributor

@lyrixx lyrixx commented Mar 4, 2025

Q A
Branch? 4.0, but I'm not sure it's the lowest maintained branch
Tickets
License MIT
Doc PR

It could occur when using a DTO

@lyrixx lyrixx force-pushed the doctrine-manager-no-found branch from 40eee46 to 457553d Compare March 4, 2025 14:57

Verified

This commit was signed with the committer’s verified signature.
ljharb Jordan Harband
…ot be found for a resource class
@lyrixx lyrixx force-pushed the doctrine-manager-no-found branch from 457553d to 5b47a4e Compare March 4, 2025 15:07
@lyrixx lyrixx changed the title Add a proper exception when a doctrine manager could not be found for a resource class fix(doctrine): Add a proper exception when a doctrine manager could not be found for a resource class Mar 4, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@soyuka soyuka merged commit a434173 into api-platform:4.0 Mar 7, 2025
57 of 59 checks passed
@lyrixx lyrixx deleted the doctrine-manager-no-found branch March 7, 2025 11:05
$manager = $this->managerRegistry->getManagerForClass($entityClass);
if (null === $manager) {
throw new RuntimeException(\sprintf('No manager found for class "%s". Are you sure it\'s an entity?', $entityClass));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this exceptional case relates to what the developer wrote; should this not be considered a LogicException instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so as this may be that the manager isn't properly configured at runtime.

Copy link
Contributor Author

@lyrixx lyrixx Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @rvanlaak (I put a logic exception in the first place)
I don't see how the manager could not be properly configured... But not a big deal imho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants