cakephp / app

CakePHP application template
370 stars 391 forks source link

improve error message if debug_kit doesn't work due to TLD whitelist #920

Closed LordSimal closed 1 year ago

LordSimal commented 1 year ago

Refs: https://github.com/cakephp/debug_kit/issues/894

Currently the app template shows a not telling error

DebugKit is not able to connect to the database. 
The datasource configuration "debug_kit" was not found.

if the DebugKit's Toolbar Service is disabled.

The Toolbar Service can only be enabled via the following scenarios:

All other scenarios will result in a disabled DebugKit and therefore the not telling error from above. Therefore all new CakePHP installations which are directly created on a server (and will therefore most likely have a "real" TLD to call that app) will result in a failing DebugKit.

This adjustment will show a more telling error which looks like this:

image

I first thought to add some more conditions to the if but in the end that home template will only work when Debug Mode is on anyways due to https://github.com/cakephp/app/blob/4.x/templates/Pages/home.php#L44

Therefore if we get a connection error from the connection named debug_kit in there we must have debug mode on but the ToolbarService is not enabled.

If the User has no sqlite php extension installed DebugKit itself will log a warning as can be seen here, so this is not our job here as well. Or should we add an extra check + message here as well?

We could also add a default config block like this

    'DebugKit' => [
        //'safeTld' => ['com']
    ]

to the generated app_local.php so people have an easier time to adjust that.

ADmad commented 1 year ago

If the User has no sqlite php extension installed DebugKit itself will log a warning as can be seen here, so this is not our job here as well. Or should we add an extra check + message here as well?

This homepage should be as newbie friendly as possible. Someone just starting out might not even know where to check for the logs. So it would be better if more info was provided here itself.

LordSimal commented 1 year ago

The adjusted error message would look like this:

image
LordSimal commented 1 year ago

Even though I just read, that PHP 7.4 seems to at least enable the PDO Sqlite Driver by default according to https://www.php.net/manual/en/ref.pdo-sqlite.php

ADmad commented 1 year ago

The DebugKit is unable to connect to database is still misleading when it's the safe TLD config that's preventing the toolbar from loading.

LordSimal commented 1 year ago

Any other recommendations or complaints? Otherwise I would merge this.