[drupal-devel] [feature] Nice DB error screens

Wed Jul 27 22:51:52 UTC 2005

Issue status update for 
http://drupal.org/node/27231
Post a follow up: 
http://drupal.org/project/comments/add/27231

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     feature requests
 Priority:     normal
 Assigned to:  drumm
 Reported by:  drumm
 Updated by:   wiz
 Status:       patch

That was too quick, I have some more info. The current CVS code does not
work when not accessing the start page, e.g. http://drupal/, but
something at a deeper level (with search engine friendly URLs turned
on) like http://drupal/user/. This case can easily happen with deep
links from another site.  With the patch attached to my previous post
the properly formatted error page appears in this case, too.

wiz

Previous comments:
------------------------------------------------------------------------

Thu, 21 Jul 2005 22:43:16 +0000 : drumm

Attachment: http://drupal.org/files/issues/screenshot2.png (96.1 KB)

Screenshot.

------------------------------------------------------------------------

Thu, 21 Jul 2005 23:21:31 +0000 : drumm

Attachment: http://drupal.org/files/issues/screenshot_10.png (88.44 KB)

------------------------------------------------------------------------

Thu, 21 Jul 2005 23:25:57 +0000 : drumm

Attachment: http://drupal.org/files/issues/druplicon.png (3.98 KB)

New Druplicon which will need to replace misc/druplicon.png. The current
file in that location is a giant flat druplicon which isn't used
anywhere.

------------------------------------------------------------------------

Thu, 21 Jul 2005 23:26:26 +0000 : drumm

Attachment: http://drupal.org/files/issues/maintenance.css (329 bytes)

Some CSS

------------------------------------------------------------------------

Thu, 21 Jul 2005 23:30:34 +0000 : drumm

Attachment: http://drupal.org/files/issues/errorhandling.patch (8.47 KB)

This patch makes the database errors which occur when the database
connection information is wrong or the database isn't up much nicer and
more explanatory. To do this it adds a themeable function,
theme_maintenance_page(), and a way to get Drupal running without a
theme but the ability to execute themeable functions in the usual way.
This means we can use the same code to give upgrade.php a nicer look in
the future.

------------------------------------------------------------------------

Fri, 22 Jul 2005 06:33:57 +0000 : kbahey

I was about to do something similar on a less ambitious scale (mainly
the connect function), because what we get now is ugly and looks
unprofessional.

+1 from me for this. Great job!

------------------------------------------------------------------------

Fri, 22 Jul 2005 06:52:30 +0000 : gordon

+1, This looks great. Get this in and then we can do something with the
upgrade.php system and it will look just great.

------------------------------------------------------------------------

Fri, 22 Jul 2005 19:36:39 +0000 : drumm

Attachment: http://drupal.org/files/issues/errorhandling_0.patch (8.44 KB)

New patch with a couple suggestions from chx.

* $foo === NULL is faster than is_null($foo)
* Removed 'bootstrap' from the function name since it is not really
part of the bootstrap system.

When testing/applying updates #2 and #3 of this issue need to be
handled too.

------------------------------------------------------------------------

Fri, 22 Jul 2005 20:47:18 +0000 : stefan nagtegaal

I indeed like the patch! you have some perfectly helptexts which asure
that people *must* be foulish when they do not know how to setup drupal
the correct way. On that, a very, very fine patch..

Though, code-wise I have one nit-pick, which is propably not even a
real issue. Only, I think it looks better:
+    print theme('maintenance_page', '<p>We were able to connect to the
MySQL database server (which means your username and password is okay)
but not able to select the database.</p>
+<p>The MySQL error was: <em>'. mysql_error() .'</em>.</p>
+<p>Currently, the database is <code>'. substr($url['path'], 1) .'. The
username is '. $url['user'] .' and the database server is '.
$url['host'] .'.

+

+
* Are you sure you have the correct database name?
+
* Are you sure the database exists?
+
* Are you sure the username has permission to access the database?
+
+For more help, see the Installation and upgrading handbook [1]. If you
are unsure what these terms mean you should probably contact your
hosting provider.

');

Wouldn't it be nicer (code-wise) todo:
    $message[] =  'We were able to connect to the MySQL database
server (which means your username and password is okay) but not able to
select the database.';
  $message[] = strtr('The MySQL error was: %error.', array('%error'
=> theme('placeholder', mysql_error())));
  $message[] = etc, etc;
  print theme('maintenance_page', $message);

Also would I encourage you to make a function theme_message($type,
$message), like:
function theme_message($type, $message) {
  switch($type) {
     case 'code':
       $output .= '<code>'. $message .'<code>';
       break;
    case 'placeholder':
      $output .= ''. $message .'';
       break;
     case 'important':
      $output .= ''. $message .'';
       break;
    default:
      $output .= ''. $message .'';
       break;
  }
}

Which makes theme_placeholder() redundant and could be removed.
Codesnippets could be handled in a more consistent way then we
currently do (having a theme_placeholder() only is rediculous imo
afterall).

Also this function could be used to remove the hardcoded HTML from the
helptexts..

The code is untested and needs some love here-and-there. But, when it's
done, I think we will create (codewise) more consistency than we
currently have!

I look forward to see your idea's and hear your opinion about this..
[1] http://drupal.org/node/258

------------------------------------------------------------------------

Fri, 22 Jul 2005 23:27:53 +0000 : drumm

The theme_message() which stefan nagtegaal looks like it should be four
separate themeable functions, one of which already exists. I think any
such modifications are outside the scope of this patch.

I am nearly finished preparing a patch to update.php which uses this
patch, but I can not post it until this patch is resolved.

------------------------------------------------------------------------

Sat, 23 Jul 2005 09:35:03 +0000 : stefan nagtegaal

Okay, I think I have to agree with Neill in here.. If this patch hits
the trunk, I'll make a new issue and rolls a patch for introducing new
functions which should improve code-wise consistency..

Let me introduce you to:

function theme_placeholder($message) {
  return "<em>$message</em>";
}
function theme_important($message) {
  return "<strong>$message</strong>";
}
function theme_code($message) {
  return "<code>$message";
}

function theme_text($message) {
  return "$message

";
}

Patches are soon to come in another issue, after this has hit the
trunk..

------------------------------------------------------------------------

Wed, 27 Jul 2005 00:40:00 +0000 : drumm

Attachment: http://drupal.org/files/issues/errorhandling_1.patch (8.75 KB)

Updated patch using theme_placeholder() where appropriate.

------------------------------------------------------------------------

Wed, 27 Jul 2005 01:59:01 +0000 : Steven

Commited to HEAD.

------------------------------------------------------------------------

Wed, 27 Jul 2005 22:44:26 +0000 : wiz

Attachment: http://drupal.org/files/issues/errorhandling_base.patch (825 bytes)

Basically works nicely, but the function theme_maintenance_page doesn't
set the BASE tag, which (at least in my setup with the site not at the
root directory) fails to find the CSS files. The patch adds the same
BASE tag as is present on normal pages (i.e., $base_url).