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

[8.x] Model::delete throw LogicException not Exception #36914

Merged

Conversation

lachlankrautz
Copy link
Contributor

@lachlankrautz lachlankrautz commented Apr 8, 2021

Issue

Currently eloquent model->delete() checks to see if a primary key is set on the model and throws a top level Exception if missing. This causes many static analysis tools to suggest using try catch anytime a model is deleted. Since this exception is only thrown when there is a mistake in the code (not setting the primary key) it could be changed to throw a LogicException instead.

Change

This PR changes the exception thrown by Model::delete() from \Exception to \LogicException.

Why logic exception?

LogicException:
Exception that represents error in the program logic. This kind of exceptions should directly lead to a fix in your code.

Static analysis tools typically consider LogicException to be unchecked and won't suggest callers use try catch. This should reduce the amount of unnecessary try catch blocks in those projects.

Will this break anything?

Since LogicException extends Exception any user code that used to check for Exception will still work.

@taylorotwell taylorotwell merged commit 5e0ef03 into laravel:8.x Apr 8, 2021
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.

2 participants