From 6776038c21f61e12aca399be260921db2e3ad7f5 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 12 Jun 2008 00:15:29 +0000 Subject: [PATCH] * (bug 14497) Throw visible errors in installer scripts when SQL files fail due to database permission or other error Database::sourceFile() was suppressing the standard error reporting, and returning any errors as a string value. While nice behavior in theory, several years ago, this doesn't work well in practice; most callers assume either that any error will be fatal and halt the script (no error checking) or that an error will return false and any boolean true is success. This caused false positive success, as error conditions returned as strings evaluated to true. Changed to letting the database object's existing erorr reporting behavior control whether it throws an exception or suppresses the error and returns it as a string -- default behavior will be to throw an exception, making 'can't run CREATE TABLE' errors actually visible to the user trying to install an extension. For good measure, failure to open the input file also throws an exception to ensure that it will produce a nice visible error instead of a hidden WTF. --- RELEASE-NOTES | 2 ++ includes/Database.php | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index cb1800b1cb..398dad12c9 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -362,6 +362,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN * We no longer just give up on a missing upload base directory; it's now created automatically if we have sufficient permissions! * (bug 14479) MediaWiki:upload-maxfilesize should have a div id wrapper +* (bug 14497) Throw visible errors in installer scripts when SQL files + fail due to database permission or other error === API changes in 1.13 === diff --git a/includes/Database.php b/includes/Database.php index f3f98e52e2..b6a8c8bde6 100644 --- a/includes/Database.php +++ b/includes/Database.php @@ -2116,7 +2116,7 @@ class Database { /** * Read and execute SQL commands from a file. - * Returns true on success, error string on failure + * Returns true on success, error string or exception on failure (depending on object's error ignore settings) * @param string $filename File name to open * @param callback $lineCallback Optional function called before reading each line * @param callback $resultCallback Optional function called for each MySQL result @@ -2124,7 +2124,7 @@ class Database { function sourceFile( $filename, $lineCallback = false, $resultCallback = false ) { $fp = fopen( $filename, 'r' ); if ( false === $fp ) { - return "Could not open \"{$filename}\".\n"; + throw new MWException( "Could not open \"{$filename}\".\n" ); } $error = $this->sourceStream( $fp, $lineCallback, $resultCallback ); fclose( $fp ); @@ -2133,7 +2133,7 @@ class Database { /** * Read and execute commands from an open file handle - * Returns true on success, error string on failure + * Returns true on success, error string or exception on failure (depending on object's error ignore settings) * @param string $fp File handle * @param callback $lineCallback Optional function called before reading each line * @param callback $resultCallback Optional function called for each MySQL result @@ -2176,7 +2176,7 @@ class Database { if ( $done ) { $cmd = str_replace(';;', ";", $cmd); $cmd = $this->replaceVars( $cmd ); - $res = $this->query( $cmd, __METHOD__, true ); + $res = $this->query( $cmd, __METHOD__ ); if ( $resultCallback ) { call_user_func( $resultCallback, $res ); } -- 2.20.1