Page MenuHomePhabricator

Handle most errors
Open, HighPublic

Description

CreateWiki currently has lots of fail points. While it isn't fun to write hundreds of lines of error handling code to cover all of these things, we should work on doing that. Currently, CreateWiki will spew an error out to a user (or even a white screen) in the case of an error, with no alert to staff.

We should change this so that staff are informed of any/all errors, CreateWiki can proceed in various errors (and alert staff to manually fix the issue), and users don't get long technical errors that aren't friendly.

jack added a comment.Jul 3 2016, 9:51 AM

"Hundreds of lines of error handling code" is hopefully somewhat of an overstatement. ;-) T64 and the like can be worked around with a try-catch loop, for example. Right now in CreateWikiBackend::initDB(), line 479, we blindly source the files listed in $wgCreateWikiSQLFiles (regardless of their existence or the lack of thereof!) against the newly created DB with DatabaseBase::sourceFile(), which seems to throw a MWException if it can't open the file. So maybe something like this would work:

		// Load SQL files (at least MediaWiki core, also for any extensions, etc.)
		if( is_array( $wgCreateWikiSQLFiles ) ) {
			foreach( $wgCreateWikiSQLFiles as $sqlfile ) {
				try {
					$dbw->sourceFile( $sqlfile );
				} catch ( MWException $e ) {
					// Don't halt processing, just skip over the problematic
					// (possibly nonexistent, etc.) file(s)
					// [somehow notify staff about this problem here]
					continue;
				}
			}
		}

It's true that CreateWiki's error handling/logging is far from ideal. Right now there are four calls to error_log (and two somewhat redundant wfDebugLog calls), but it goes without saying that such error messages can and will get easily lost in the huge error log file. I guess we'd want a method for emailing a specified address (either technical@sw or maybe a brand new CW-specific list or something else?) and call that instead of error_log in the places where we have such calls currently.

What's the trickiest part (IMHO) is actually identifying the pain points. T64, for example, is quite an edge case, but we know it happened and it caused issues to some of our users, even though we swiftly fixed it. We also know from prior experience that the foreach loop above, which generates the necessary tables in the new database, is slow, but addressing that isn't easy.