-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Show better message when SQLite is not enabled #431
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
Conversation
…etter error message
@uikolas, IMO if we already have the ConsoleEventSubscriber you should add your code there. Just rename it to |
*/ | ||
public function onKernelException(GetResponseForExceptionEvent $event) | ||
{ | ||
if (!extension_loaded('sqlite3')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be done only if the exception is a DBAL exception IMO. Otherwise, it could hide other issues happening earlier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i agree with that. But the problem is that from $event->getException() i get Twig_Error_Runtime and don't know why. Maybe is documented somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, any exception thrown during a Twig template rendering is wrapped in a Twig_Error_Runtime. You can check the chain of previous exceptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, didn't know that. Ok :)
// We must get the original exception. | ||
$previousException = $exception->getPrevious(); | ||
|
||
if ($previousException instanceof DriverException && !extension_loaded('sqlite3')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the driver exception may still be the main exception depending on where it happens (it can happen in the controller)
$previousException = $exception->getPrevious(); | ||
|
||
// Driver exception may happen in controller or in twig template rendering | ||
$isDriverException = ($exception instanceof DriverException || $previousException instanceof DriverException); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should make sure that a connection is configured to use an sqlite
driver because it can be an error with another one (mysql
, postgres
and etc.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are totally correct :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uikolas, you could use a platform name:
$entityManager->getConnection()->getDatabasePlatform()->getName() === 'sqlite'
@uikolas thanks for improving the detection of the SQLite requirement! |
…eptions (javiereguiluz) This PR was merged into the master branch. Discussion ---------- Improved the event subscriber that listens to database exceptions This finishes #431 by doing some renaming and minor tweaks. Commits ------- 3387540 Improved the event subscriber that listens to database exceptions
As mentioned in #423
I added to show better error message in the browser, if SQLite is not enabled. (I think that message should contain about sqllite3 and pdo_sqlite to enable)
But i have have doubts, how to show it? As a exception or simple text response?