Skip to content

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Jan 13, 2023

Alternative to laravel/laravel#6075

Issue

There is an inconsistency between artisan and HTTP requests when using relative file paths as the SQLite database.

Reproduce it

Create a brand new Laravel project, use Sqlite as a database, run migrations, and interact with the database via Tinker.

Note: DB_DATABASE is unchanged and currently set to "laravel".

composer create-project laravel/laravel sqlite-test \
  && cd sqlite-test \
  && sed -i "s/DB_CONNECTION=mysql/DB_CONNECTION=sqlite/" .env \
  && echo "{{ \App\Models\User::first() }}" > resources/views/welcome.blade.php \
  && php artisan migrate \
  && php artisan tinker --execute="User::create(['name' => 'Tim', 'email' => 'tim@laravel.com', 'password' => 'secret']);"

Everything should have worked as expected. Now we will serve the project and visit the homepage.

(sleep 1 && open http://127.0.0.1:8000)& \
php artisan serve

Screen Shot 2023-01-13 at 11 37 57 am

Why this happens

When using a relative path in DB_DATABASE and starting Laravel from the project root via artisan, the relative path resolves to the same thing as base_path('laravel').

When using the same relative path from a web request, the root is no longer the project root. It is now the public directory, so the relative path resolves to the same thing as public_path('laravel').

/private/tmp/sqlite-test/laravel < Artisan
/private/tmp/sqlite-test/public/laravel < Web request

When the SqliteConnector tries to resolve the realpath, it is unable to find the file /private/tmp/sqlite-test/public/laravel, so it returns false.

$path = realpath($config['database']);
// Here we'll verify that the SQLite database exists before going any further
// as the developer probably wants to know if the database exists and this
// SQLite driver will not throw any exception if it does not by default.
if ($path === false) {
throw new SQLiteDatabaseDoesNotExistException($config['database']);
}
return $this->createConnection("sqlite:{$path}", $config, $options);

The improvement (not a fix)

The current exception message is not useful. In Laravel 9.x I would like to solve this by improving the error message presented to the user to at least give some information on how to solve the issue.

Screen Shot 2023-01-13 at 11 24 31 am

Unfortunately the error message doesn't mention the environment variable or the configuration variable, as at this point we don't have any information on which is being used. The component could also be used outside of laravel/laravel.

Inconsistency sucks

You know it. I know it. It sucks that this works in Artisan but doesn't then work in a HTTP request. Just wanna put that out there.

Future fix

Someone smarter than myself may be able to find a good solution to this, but I've tried a bunch of things and everything seemed to have issues.

Windows vs POSIX, ./laravel vs laravel, who knows what else.

@timacdonald timacdonald changed the title [9.x] Improve devloper facing Sqlite missing DB error message [9.x] Improve developer facing Sqlite missing DB error message Jan 13, 2023
@timacdonald timacdonald marked this pull request as ready for review January 13, 2023 01:02
@taylorotwell taylorotwell merged commit 8a79374 into laravel:9.x Jan 13, 2023
@timacdonald timacdonald deleted the sqlite-exception branch February 6, 2023 22:17
taylorotwell added a commit that referenced this pull request Feb 6, 2025
* Support relative paths to SQLite databases

Related: #45626, wintercms/storm#191

* Styling

* Update SQLiteConnector.php

---------

Co-authored-by: Taylor Otwell <taylor@laravel.com>
taylorotwell added a commit to illuminate/database that referenced this pull request Feb 6, 2025
* Support relative paths to SQLite databases

Related: laravel/framework#45626, wintercms/storm#191

* Styling

* Update SQLiteConnector.php

---------

Co-authored-by: Taylor Otwell <taylor@laravel.com>
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