Deprecate $wgShowSQLErrors and $wgShowDBErrorBacktrace and make nonfunctional

Clarify and simplify exception output by deprecating
$wgShowSQLErrors and wgShowDBErrorBacktrace.
$wgShowExceptionDetails will now control most related output.
$wgShowHostnames will now solely control output of
MWExceptionRenderer::reportOutageHTML.

Bug: T165768
Change-Id: Idead2c11c499463dfa6293c3d4b33be3bde92e1a

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index 7ac5479..486645a 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -279,6 +279,10 @@ because of Phabricator reports.
   'log-show-hide-[type]' format. Instead use 'logeventslist-[type]-log'.
 * Global functions  wfArrayFilter() and wfArrayFilterByKey() are deprecated.
   use array_filter() directly.
+* The $wgShowSQLErrors global is deprecated and nonfunctional.
+  Set $wgShowExceptionDetails and/or $wgShowHostnames instead.
+* The $wgShowDBErrorBacktrace global is deprecated and nonfunctional.
+  Set $wgShowExceptionDetails instead.
 
 === Other changes in 1.32 ===
 * (T198811) The following tables have had their UNIQUE indexes turned into

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 84131f0..13d5368 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -6296,14 +6296,17 @@ $wgSpecialVersionShowHooks = false;
  * Whether to show "we're sorry, but there has been a database error" pages.
  * Displaying errors aids in debugging, but may display information useful
  * to an attacker.
+ *
+ * @deprecated and nonfunctional since 1.32: set $wgShowExceptionDetails and/or
+ * $wgShowHostnames instead.
  */
 $wgShowSQLErrors = false;
 
 /**
- * If set to true, uncaught exceptions will print a complete stack trace
- * to output. This should only be used for debugging, as it may reveal
- * private information in function parameters due to PHP's backtrace
- * formatting.
+ * If set to true, uncaught exceptions will print the exception message and a
+ * complete stack trace to output. This should only be used for debugging, as it
+ * may reveal private information in function parameters due to PHP's backtrace
+ * formatting.  If set to false, only the exception's class will be shown.
  */
 $wgShowExceptionDetails = false;
 
@@ -6314,6 +6317,8 @@ $wgShowExceptionDetails = false;
  * reported in the normal manner. $wgShowExceptionDetails applies in other cases,
  * including those in which an uncaught exception is thrown from within the
  * exception handler.
+ *
+ * @deprecated and nonfunctional since 1.32: set $wgShowExceptionDetails instead.
  */
 $wgShowDBErrorBacktrace = false;
 

diff --git a/includes/DevelopmentSettings.php b/includes/DevelopmentSettings.php
index 96ed56b..8b32de4 100644 (file)
--- a/includes/DevelopmentSettings.php
+++ b/includes/DevelopmentSettings.php
@@ -24,18 +24,16 @@ ini_set( 'display_errors', 1 );
 /**
  * Debugging: MediaWiki
  */
-global $wgDevelopmentWarnings, $wgShowDBErrorBacktrace, $wgShowExceptionDetails,
-       $wgShowSQLErrors, $wgDebugRawPage,
-       $wgDebugComments, $wgDebugDumpSql, $wgDebugTimestamps,
+global $wgDevelopmentWarnings, $wgShowExceptionDetails, $wgShowHostnames,
+       $wgDebugRawPage, $wgDebugComments, $wgDebugDumpSql, $wgDebugTimestamps,
        $wgCommandLineMode, $wgDebugLogFile, $wgDBerrorLog, $wgDebugLogGroups;
 
 // Use of wfWarn() should cause tests to fail
 $wgDevelopmentWarnings = true;
 
 // Enable showing of errors
-$wgShowDBErrorBacktrace = true;
 $wgShowExceptionDetails = true;
-$wgShowSQLErrors = true;
+$wgShowHostnames = true;
 $wgDebugRawPage = true; // T49960
 
 // Enable log files

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 84708e7..e5e384c 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -24,8 +24,6 @@
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use Wikimedia\Timestamp\TimestampException;
-use Wikimedia\Rdbms\DBQueryError;
-use Wikimedia\Rdbms\DBError;
 
 /**
  * This is the main API class, used for both external and internal processing.
@@ -1038,9 +1036,7 @@ class ApiMain extends ApiBase {
                        $config = $this->getConfig();
                        $class = preg_replace( '#^Wikimedia\\\Rdbms\\\#', '', get_class( $e ) );
                        $code = 'internal_api_error_' . $class;
-                       if ( ( $e instanceof DBQueryError ) && !$config->get( 'ShowSQLErrors' ) ) {
-                               $params = [ 'apierror-databaseerror', WebRequest::getRequestId() ];
-                       } else {
+                       if ( $config->get( 'ShowExceptionDetails' ) ) {
                                if ( $e instanceof ILocalizedException ) {
                                        $msg = $e->getMessageObject();
                                } elseif ( $e instanceof MessageSpecifier ) {
@@ -1049,7 +1045,10 @@ class ApiMain extends ApiBase {
                                        $msg = wfEscapeWikiText( $e->getMessage() );
                                }
                                $params = [ 'apierror-exceptioncaught', WebRequest::getRequestId(), $msg ];
+                       } else {
+                               $params = [ 'apierror-exceptioncaughttype', WebRequest::getRequestId(), get_class( $e ) ];
                        }
+
                        $messages[] = ApiMessage::create( $params, $code );
                }
                return $messages;
@@ -1113,9 +1112,7 @@ class ApiMain extends ApiBase {
                                )
                        );
                } else {
-                       if ( $config->get( 'ShowExceptionDetails' ) &&
-                               ( !$e instanceof DBError || $config->get( 'ShowDBErrorBacktrace' ) )
-                       ) {
+                       if ( $config->get( 'ShowExceptionDetails' ) ) {
                                $result->addContentValue(
                                        $path,
                                        'trace',

diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json
index 74efd82..d7cdc6c 100644 (file)
--- a/includes/api/i18n/en.json
+++ b/includes/api/i18n/en.json
@@ -1712,12 +1712,12 @@
        "apierror-copyuploadbadurl": "Upload not allowed from this URL.",
        "apierror-create-titleexists": "Existing titles can't be protected with <kbd>create</kbd>.",
        "apierror-csp-report": "Error processing CSP report: $1.",
-       "apierror-databaseerror": "[$1] Database query error.",
        "apierror-deletedrevs-param-not-1-2": "The <var>$1</var> parameter cannot be used in modes 1 or 2.",
        "apierror-deletedrevs-param-not-3": "The <var>$1</var> parameter cannot be used in mode 3.",
        "apierror-emptynewsection": "Creating empty new sections is not possible.",
        "apierror-emptypage": "Creating new, empty pages is not allowed.",
        "apierror-exceptioncaught": "[$1] Exception caught: $2",
+       "apierror-exceptioncaughttype": "[$1] Caught exception of type $2",
        "apierror-filedoesnotexist": "File does not exist.",
        "apierror-fileexists-sharedrepo-perm": "The target file exists on a shared repository. Use the <var>ignorewarnings</var> parameter to override it.",
        "apierror-filenopath": "Cannot get local file path.",

diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json
index 2b4a587..dd8e529 100644 (file)
--- a/includes/api/i18n/qqq.json
+++ b/includes/api/i18n/qqq.json
@@ -1600,12 +1600,12 @@
        "apierror-copyuploadbadurl": "{{doc-apierror}}",
        "apierror-create-titleexists": "{{doc-apierror}}",
        "apierror-csp-report": "{{doc-apierror}}\n\nParameters:\n* $1 - Error code, e.g. \"toobig\".",
-       "apierror-databaseerror": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception log ID code. This is meaningless to the end user, but can be used by people with access to the logs to easily find the logged error.",
        "apierror-deletedrevs-param-not-1-2": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n\nSee also:\n* {{msg-mw|apihelp-query+deletedrevs-extended-description}}",
        "apierror-deletedrevs-param-not-3": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n\nSee also:\n* {{msg-mw|apihelp-query+deletedrevs-extended-description}}",
        "apierror-emptynewsection": "{{doc-apierror}}",
        "apierror-emptypage": "{{doc-apierror}}",
        "apierror-exceptioncaught": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception log ID code. This is meaningless to the end user, but can be used by people with access to the logs to easily find the logged error.\n* $2 - Exception message, which may end with punctuation. Probably in English.",
+       "apierror-exceptioncaughttype": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception log ID code. This is meaningless to the end user, but can be used by people with access to the logs to easily find the logged error.\n* $2 - Exception type (for example, DBError).",
        "apierror-filedoesnotexist": "{{doc-apierror}}",
        "apierror-fileexists-sharedrepo-perm": "{{doc-apierror}}",
        "apierror-filenopath": "{{doc-apierror}}",

diff --git a/includes/exception/MWExceptionRenderer.php b/includes/exception/MWExceptionRenderer.php
index 88b28df..49cf71e 100644 (file)
--- a/includes/exception/MWExceptionRenderer.php
+++ b/includes/exception/MWExceptionRenderer.php
@@ -19,7 +19,6 @@
  */
 
 use Wikimedia\Rdbms\DBConnectionError;
-use Wikimedia\Rdbms\DBError;
 use Wikimedia\Rdbms\DBReadOnlyError;
 use Wikimedia\Rdbms\DBExpectedError;
 
@@ -37,7 +36,7 @@ class MWExceptionRenderer {
         * @param Exception|Throwable|null $eNew New exception from attempting to show the first
         */
        public static function output( $e, $mode, $eNew = null ) {
-               global $wgMimeType;
+               global $wgMimeType, $wgShowExceptionDetails;
 
                if ( defined( 'MW_API' ) ) {
                        // Unhandled API exception, we can't be sure that format printer is alive
@@ -58,7 +57,7 @@ class MWExceptionRenderer {
                        self::header( "Content-Type: $wgMimeType; charset=utf-8" );
                        if ( $eNew ) {
                                $message = "MediaWiki internal error.\n\n";
-                               if ( self::showBackTrace( $e ) ) {
+                               if ( $wgShowExceptionDetails ) {
                                        $message .= 'Original exception: ' .
                                                MWExceptionHandler::getLogMessage( $e ) .
                                                "\nBacktrace:\n" . MWExceptionHandler::getRedactedTraceAsString( $e ) .
@@ -73,7 +72,7 @@ class MWExceptionRenderer {
                                }
                                $message .= "\n";
                        } else {
-                               if ( self::showBackTrace( $e ) ) {
+                               if ( $wgShowExceptionDetails ) {
                                        $message = MWExceptionHandler::getLogMessage( $e ) .
                                                "\nBacktrace:\n" .
                                                MWExceptionHandler::getRedactedTraceAsString( $e ) . "\n";
@@ -162,7 +161,9 @@ class MWExceptionRenderer {
         * @return string Html to output
         */
        public static function getHTML( $e ) {
-               if ( self::showBackTrace( $e ) ) {
+               global $wgShowExceptionDetails;
+
+               if ( $wgShowExceptionDetails ) {
                        $html = "<div class=\"errorbox mw-content-ltr\"><p>" .
                                nl2br( htmlspecialchars( MWExceptionHandler::getLogMessage( $e ) ) ) .
                                '</p><p>Backtrace:</p><p>' .
@@ -209,7 +210,9 @@ class MWExceptionRenderer {
         * @return string
         */
        private static function getText( $e ) {
-               if ( self::showBackTrace( $e ) ) {
+               global $wgShowExceptionDetails;
+
+               if ( $wgShowExceptionDetails ) {
                        return MWExceptionHandler::getLogMessage( $e ) .
                                "\nBacktrace:\n" .
                                MWExceptionHandler::getRedactedTraceAsString( $e ) . "\n";
@@ -218,34 +221,13 @@ class MWExceptionRenderer {
                }
        }
 
-       /**
-        * @param Exception|Throwable $e
-        * @return bool
-        */
-       private static function showBackTrace( $e ) {
-               global $wgShowExceptionDetails, $wgShowDBErrorBacktrace;
-
-               return (
-                       $wgShowExceptionDetails &&
-                       ( !( $e instanceof DBError ) || $wgShowDBErrorBacktrace )
-               );
-       }
-
        /**
         * @param Exception|Throwable $e
         * @return string
         */
        private static function getShowBacktraceError( $e ) {
-               global $wgShowExceptionDetails, $wgShowDBErrorBacktrace;
-               $vars = [];
-               if ( !$wgShowExceptionDetails ) {
-                       $vars[] = '$wgShowExceptionDetails = true;';
-               }
-               if ( $e instanceof DBError && !$wgShowDBErrorBacktrace ) {
-                       $vars[] = '$wgShowDBErrorBacktrace = true;';
-               }
-               $vars = implode( ' and ', $vars );
-               return "Set $vars at the bottom of LocalSettings.php to show detailed debugging information.";
+               $var = '$wgShowExceptionDetails = true;';
+               return "Set $var at the bottom of LocalSettings.php to show detailed debugging information.";
        }
 
        /**
@@ -294,7 +276,7 @@ class MWExceptionRenderer {
         * @param Exception|Throwable $e
         */
        private static function reportOutageHTML( $e ) {
-               global $wgShowDBErrorBacktrace, $wgShowHostnames, $wgShowSQLErrors, $wgSitename;
+               global $wgShowExceptionDetails, $wgShowHostnames, $wgSitename;
 
                $sorry = htmlspecialchars( self::msg(
                        'dberr-problems',
@@ -305,7 +287,7 @@ class MWExceptionRenderer {
                        'Try waiting a few minutes and reloading.'
                ) );
 
-               if ( $wgShowHostnames || $wgShowSQLErrors ) {
+               if ( $wgShowHostnames ) {
                        $info = str_replace(
                                '$1',
                                Html::element( 'span', [ 'dir' => 'ltr' ], $e->getMessage() ),
@@ -327,7 +309,7 @@ class MWExceptionRenderer {
                                '<style>body { font-family: sans-serif; margin: 0; padding: 0.5em 2em; }</style>' .
                                "</head><body><h1>$sorry</h1><p>$again</p><p><small>$info</small></p>";
 
-               if ( $wgShowDBErrorBacktrace ) {
+               if ( $wgShowExceptionDetails ) {
                        $html .= '<p>Backtrace:</p><pre>' .
                                htmlspecialchars( $e->getTraceAsString() ) . '</pre>';
                }

diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php
index 3905ba0..e7ee95c 100644 (file)
--- a/includes/installer/Installer.php
+++ b/includes/installer/Installer.php
@@ -1725,13 +1725,10 @@ abstract class Installer {
                $GLOBALS['wgLanguageConverterCacheType'] = CACHE_NONE;
                // Debug-friendly
                $GLOBALS['wgShowExceptionDetails'] = true;
+               $GLOBALS['wgShowHostnames'] = true;
                // Don't break forms
                $GLOBALS['wgExternalLinkTarget'] = '_blank';
 
-               // Extended debugging
-               $GLOBALS['wgShowSQLErrors'] = true;
-               $GLOBALS['wgShowDBErrorBacktrace'] = true;
-
                // Allow multiple ob_flush() calls
                $GLOBALS['wgDisableOutputCompression'] = true;
 

diff --git a/tests/phpunit/includes/ExtraParserTest.php b/tests/phpunit/includes/ExtraParserTest.php
index 75ebd31..164c83c 100644 (file)
--- a/tests/phpunit/includes/ExtraParserTest.php
+++ b/tests/phpunit/includes/ExtraParserTest.php
@@ -17,7 +17,7 @@ class ExtraParserTest extends MediaWikiTestCase {
 
                $contLang = Language::factory( 'en' );
                $this->setMwGlobals( [
-                       'wgShowDBErrorBacktrace' => true,
+                       'wgShowExceptionDetails' => true,
                        'wgCleanSignatures' => true,
                ] );
                $this->setUserLang( 'en' );

diff --git a/tests/phpunit/includes/api/ApiMainTest.php b/tests/phpunit/includes/api/ApiMainTest.php
index b7869e6..20d758e 100644 (file)
--- a/tests/phpunit/includes/api/ApiMainTest.php
+++ b/tests/phpunit/includes/api/ApiMainTest.php
@@ -967,8 +967,7 @@ class ApiMainTest extends ApiTestCase {
                $context->setLanguage( 'en' );
                $context->setConfig( new MultiConfig( [
                        new HashConfig( [
-                               'ShowHostnames' => true, 'ShowSQLErrors' => false,
-                               'ShowExceptionDetails' => true, 'ShowDBErrorBacktrace' => true,
+                               'ShowHostnames' => true, 'ShowExceptionDetails' => true,
                        ] ),
                        $context->getConfig()
                ] ) );
@@ -1052,7 +1051,8 @@ class ApiMainTest extends ApiTestCase {
                                                [ 'code' => 'existing-error', 'text' => 'existing error', 'module' => 'main' ],
                                                [
                                                        'code' => 'internal_api_error_DBQueryError',
-                                                       'text' => "[$reqId] Database query error.",
+                                                       'text' => "[$reqId] Exception caught: A database query error has occurred. " .
+                                                               "This may indicate a bug in the software.",
                                                ]
                                        ],
                                        'trace' => $dbtrace,