Skip to content

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