Page MenuHomePhabricator

[Regression] MediaWiki should detect absent or outdated vendor
Closed, ResolvedPublic

Description

Right now doing git-pull gives me an blank HTTP 500 response with no information at all.

This is useless and unacceptable. Before the upcoming release, there must be a detection of sorts. Similar to what we do when PHP version isn't supported or when there are no skins installed, etc.

Going into the Apache error log gives me:

PHP Fatal error:  Class 'Psr\\Log\\AbstractLogger' not found in mediawiki/core/includes/debug/logger/legacy/Logger.php on line 40
PHP Stack trace:
PHP   1. {main}() mediawiki/core/index.php:0

Which isn't useful either.


See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=72700

Details

Reference
bz72777

Event Timeline

bzimport raised the priority of this task from to Normal.
bzimport set Reference to bz72777.
Krinkle created this task.Oct 30 2014, 7:45 PM
Reedy added a comment.Oct 30 2014, 7:46 PM

(In reply to Krinkle from comment #0)

Right now doing git-pull gives me an blank HTTP 500 response with no
information at all.

git pull where of what repo?

The place where we do the php version check is before the autoloader, so that won't work to fix the problem. Maybe we can put a check in the includes/debug/logger/legacy/Logger.php file itself that is outside of the class. That way when our autoloader first requires the file we could check to see if the PSR-3 classes are loaded/loadable or not. I'm not exactly sure how to trigger a better global warning however. I think about the best we could do is make an error_log() call that gives a more explanatory message.

gerritadmin wrote:

Change 170274 had a related patch set uploaded by BryanDavis:
Throw exception when \Psr\Log\LoggerInterface is unavailable

https://gerrit.wikimedia.org/r/170274

Reedy added a comment.Nov 4 2014, 1:28 AM
  • Bug 72932 has been marked as a duplicate of this bug. ***

I ran into the same problem. This should definitely generate some kind of meaningful error message. Also someone should update the documentation on mediawiki.org to explain that Composer is now required:
https://www.mediawiki.org/wiki/Download_from_Git#Prerequisites
I'll take a shot at it myself, but I can't guarantee it will be accurate.

(In reply to Ryan Kaldari from comment #5)

I ran into the same problem. This should definitely generate some kind of
meaningful error message. Also someone should update the documentation on
mediawiki.org to explain that Composer is now required:
https://www.mediawiki.org/wiki/Download_from_Git#Prerequisites
I'll take a shot at it myself, but I can't guarantee it will be accurate.

It was documented at https://www.mediawiki.org/wiki/Download_from_Git#Fetch_external_libraries, but I've stated in the Prerequisites that composer is recommended, but not required.

Legoktm: It seems to be required to me. If I don't install it, I just get a 500 server error (as Krinkle did).

Reedy added a comment.Nov 4 2014, 1:56 AM

You can just clone https://github.com/wikimedia/mediawiki-vendor instead. you don't have to use composer

Ah, nevermind, I see now. You can install the vendor components manually if you want to. That should be better explained in the prerequisites section.

Guess I should read the entire instructions and not just the first part :)

gerritadmin wrote:

Change 170274 merged by jenkins-bot:
die() with explanation when \Psr\Log\LoggerInterface is missing

https://gerrit.wikimedia.org/r/170274

We need a more general solution for this problem than the one off fix for PSR-3 that was applied.

I'm thinking that we could install an autolaoder via spl_autoload_register() *after* all other autoloaders to give a nice warning when a class has not been found yet.

I think we could also create a git hook that would compare composer.json and composer.lock when composer.json is seen to be updated by a pull and give the user a warning and instructions on how to proceed. The git install instructions could include details on how to install this hook and MediaWiki-Vagrant could do it automatically.

Krinkle updated the task description. (Show Details)Nov 24 2014, 4:02 PM
Krinkle set Security to None.

I think we could also create a git hook that would compare composer.json and composer.lock when composer.json is seen to be updated by a pull and give the user a warning and instructions on how to proceed. The git install instructions could include details on how to install this hook and MediaWiki-Vagrant could do it automatically.

I've been playing around with this hook:

1#!/usr/bin/env bash
2# Run composer if composer.json has changed
3
4git diff-tree -r --name-only --no-commit-id ORIG_HEAD HEAD |
5grep --quiet composer.json &&
6composer update --no-progress --optimize-autoloader

It seems to work for the most basic use case of a git pull that brings an updated composer.json. This hook does not fire when git checkout switches the contents of the composer.json file. That would require a different hook.

Another idea from irc:

< ori> bd808: how about: register_shutdown_function( function () { $error = error_get_last(); if ( /* error is a "class not found" fatal */ ) { /* check if composer is up-to-date */ } } );
< bd808> ori: oh that's an interesting idea

We need a more general solution for this problem than the one off fix for PSR-3 that was applied.

I'm thinking that we could install an autolaoder via spl_autoload_register() *after* all other autoloaders to give a nice warning when a class has not been found yet.

Using a special autoloader registered in Setup.php after extensions have finished initializing almost works to notify the user nicely of a missing class. Here's the code I tried:

// Install a last ditch autoloader that will warn the user nicely when a class
// is not found.
spl_autoload_register( function ( $clazz ) {
    $message = <<<TXT
Class '{$clazz}' not found.

MediaWiki or an installed extension requires this class but it is not embedded directly in MediaWiki's git repository and must be installed separately by the end user.

Please see <a href="https://www.mediawiki.org/wiki/Download_from_Git#Fetch_external_libraries">mediawiki.org</a> for help on installing the required components.
TXT;
    echo $message;
    throw new MWException( $message );
} );

When lessc is not found in $IP/vendor, this code emits this in the autoloader output:

/*
exception 'MWException' with message 'Class 'lessc' not found.

MediaWiki or an installed extension requires this class but it is not embedded directly in MediaWiki's git repository and must be installed separately by the end user.

Please see <a href="https://www.mediawiki.org/wiki/Download_from_Git#Fetch_external_libraries">mediawiki.org</a> for help on installing the required components.' in /vagrant/mediawiki/includes/Setup.php:693
Stack trace:
#0 /vagrant/mediawiki/includes/resourceloader/ResourceLoader.php(1497): {closure}()
#1 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(977): ResourceLoader::getLessCompiler()
#2 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(906): ResourceLoaderFileModule->getLessCompiler()
#3 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(877): ResourceLoaderFileModule->readStyleFile()
#4 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderFileModule.php(407): ResourceLoaderFileModule->readStyleFiles()
#5 /vagrant/mediawiki/includes/resourceloader/ResourceLoader.php(920): ResourceLoaderFileModule->getStyles()
#6 /vagrant/mediawiki/includes/resourceloader/ResourceLoader.php(647): ResourceLoader->makeModuleResponse()
#7 /vagrant/mediawiki/load.php(45): ResourceLoader->respond()
#8 /var/www/w/load.php(5): include()
#9 {main}
*/

The problem I ran into with this is that the SyntaxHighlight_GeSHi extension guards loading geshi.php with a if( !class_exists( 'GeSHi' ) ) condition. This (and any other class_exists(...) call) fires the autoloader to see if the named class can be loaded. This will hit the special autoloader and trigger the exception.

I think we could find a work around for the guard in SyntaxHighlight_GeSHi, but a quick git grep shows that we use class_exists() in many places in MediaWiki core. To deal with this, we would need to keep some sort of whitelist of classes that are expected to be missing and I'm sure that would be fragile to maintain.

bd808 moved this task from Untriaged to In Dev on the Librarization board.Dec 2 2014, 6:56 PM

Change 177724 had a related patch set uploaded (by BryanDavis):
Register a shutdown function to log fatal errors

https://gerrit.wikimedia.org/r/177724

Patch-For-Review

@bd808 Last week we discussed the option of ditching fatal handlers and early-aborts (like in Logger.php) in favour of validating composer.lock (fallback to vendor/composer.lock).

That way we'll also catch smaller differences that may not cause an exception. I don't feel comfortable running code (fatal or no fatal) with outdated dependencies. Changes in behaviour can cause all kinds of unexpected/irreversible side effects.

It'd be like how git reports outdated submodules. Requiring vendor be present and at the right version early on in MediaWiki initialisation. If worth it, we can cache it in a file. E.g. cache/vendor-check containing the hash or mtime of composer.json. And if not present, load composer.json and lock, and do the comparisons. If valid, update/create the cache file. If invalid, trigger error, print message and end script execution.

Or do we want to defer the responsibility of ensuring up-to-date dependencies to site maintainers instead?

@bd808 Last week we discussed the option of ditching fatal handlers and early-aborts (like in Logger.php) in favour of validating composer.lock (fallback to vendor/composer.lock).

That way we'll also catch smaller differences that may not cause an exception. I don't feel comfortable running code (fatal or no fatal) with outdated dependencies. Changes in behaviour can cause all kinds of unexpected/irreversible side effects.

It'd be like how git reports outdated submodules. Requiring vendor be present and at the right version early on in MediaWiki initialisation. If worth it, we can cache it in a file. E.g. cache/vendor-check containing the hash or mtime of composer.json. And if not present, load composer.json and lock, and do the comparisons. If valid, update/create the cache file. If invalid, trigger error, print message and end script execution.

The introduction of composer provides an interesting ability to do this sort of validation, but I'm a bit concerned about the potential cost of validating the installed library versions on each page view. To some extent I fear that actually introducing the validation would provide a false sense of security for both the developers and the site maintainers. We would not be able to verify that all installed extensions were updated nor that database migrations had been properly performed. Without verifying these things, all we can verify is that the components listed in $IP/composer.json are present in the first of $IP/composer.lock or $IP/vendor/composer.lock.

Or do we want to defer the responsibility of ensuring up-to-date dependencies to site maintainers instead?

Since we don't expect composer.json to be dynamically changed by application code, validating this on each page load seems quite heavy. Perhaps what could be done instead is to provide a maintenance script that would validate the vendor components. This would be somewhat similar to the existing update.php maintenance script (or even integrated into it?). This would provide the site maintainer tools to do things safely and could be documented in the appropriate places on wiki and email list to encourage its use.

Change 177724 merged by jenkins-bot:
Register a shutdown function to log fatal errors

https://gerrit.wikimedia.org/r/177724

What we have implemented so far for this:

I think that when we get the maintenance script merged we can call this {{done}} again. Any objections?

bd808 closed this task as Resolved.Dec 30 2014, 12:00 AM

I think we've given the end-user a reasonable warning via the debug log changes and a tool to use for validation in the maintenance script. If there is a strong disagreement I would welcome a new feature request or bug report describing additional steps we should take.

Krinkle reopened this task as Open.Jan 1 2015, 5:53 AM

Whatever hardcoded logic was added to class files (PSR) or other debug warnings, no longer works (or is bypassed). We probably added additional callers earlier on, thus causing script execution to end before that logic has a chance to log anything.

Re-opening as the issue resurfaced. Unpacking MediaWiki and serving it from Apache yields a blank page. Ran into this when setting up a new laptop recently and didn't have composer installed yet.

PHP Fatal error:  Class 'MWLogger' not found in includes/GlobalFunctions.php on line 1007
Stack trace:
  1. {main}() /index.php:0
  2. require() /index.php:43
  3. require_once() includes/WebStart.php:121
  4. wfDebug() includes/Setup.php:569
  5. AutoLoader::autoload() includes/Setup.php:1007
  6. require() includes/AutoLoader.php:90
  7. trigger_error() includes/debug/logger/Logger.php:28
  8. MWExceptionHandler::handleError() includes/debug/logger/Logger.php:28
  9. MWExceptionHandler::logError() includes/exception/MWExceptionHandler.php:218
 10. wfDebugLog() includes/exception/MWExceptionHandler.php:492
 11. MWExceptionHandler::handleFatalError() includes/exception/MWExceptionHandler.php:0
 12. MWExceptionHandler::logError() includes/exception/MWExceptionHandler.php:259
 13. wfDebugLog() includes/exception/MWExceptionHandler.php:492
 14. wfOutputHandler() includes/OutputHandler.php:0
 15. wfGzipHandler() includes/OutputHandler.php:58
 16. wfClientAcceptsGzip() includes/OutputHandler.php:122
 17. wfDebug() includes/GlobalFunctions.php:1940

PHP Fatal error: Class 'MWLogger' not found in includes/GlobalFunctions.php on line 1007

MWLogger is a class inside MediaWiki (as the prefix suggests), so it sounds like your MW copy is broken somehow, unrelated to composer.

MWLogger is a class inside MediaWiki (as the prefix suggests), so it sounds like your MW copy is broken somehow, unrelated to composer.

Ok, I take that back and can reproduce it by removing my vendor directory. I see the PSR-3 warning though. It appears that the trigger_error call is preventing the die (1) from being reached? I'm pretty confused.

bd808 moved this task from Done to Untriaged on the Librarization board.Jan 4 2015, 6:23 AM
bd808 moved this task from Done to Backlog on the MediaWiki-Core-Team board.

MWLogger is a class inside MediaWiki (as the prefix suggests), so it sounds like your MW copy is broken somehow, unrelated to composer.

Ok, I take that back and can reproduce it by removing my vendor directory. I see the PSR-3 warning though. It appears that the trigger_error call is preventing the die (1) from being reached? I'm pretty confused.

Trying to reproduce on my MediaWiki-Vagrant install:

  1. Removed PSR-3 from composer.json
  2. Ran composer update
  3. Loaded http://127.0.0.1:8080/wiki/Special:Version
  4. Get a 500 response that says "Class undefined: MWLogger" in my browser
  1. Deleted $IP/vendor
  2. Loaded http://127.0.0.1:8080/wiki/Special:Version
  3. Get a 500 response that says "Class undefined: MWLogger" in my browser
  1. vagrant roles enable zend
  2. vagrant provision
  3. Deleted $IP/vendor
  4. Loaded http://127.0.0.1:8080/wiki/Special:Version
  5. Get a 200 response that says:
MediaWiki requires the PSR-3 logging library to be present. This library is not embedded directly in MediaWiki's git repository and must be installed separately by the end user. Please see mediawiki.org for help on installing the required components.

and xdebug pretty error output.

  1. sudo rm /etc/php5/apache2/conf.d/20-xdebug.ini
  2. sudo service apache2 restart
  3. Loaded http://127.0.0.1:8080/wiki/Special:Version
  4. Get a 200 response that says:
MediaWiki requires the PSR-3 logging library to be present. This library is not embedded directly in MediaWiki's git repository and must be installed separately by the end user. Please see mediawiki.org for help on installing the required components.
Fatal error: Class 'MWLogger' not found in /vagrant/mediawiki/includes/GlobalFunctions.php on line 1185
(10.0.2.15)
Fatal error: Class 'MWLogger' not found in /vagrant/mediawiki/includes/GlobalFunctions.php on line 1185
(10.0.2.15)

I can't get it to break without displaying anything, but I get better output from PHP5 than I do from HHVM.

I'm getting exactly this error.

In the apache logs:

Class 'MWLogger' not found  in ... mediawiki/includes/GlobalFunctions.php on line 1185

and this in the browser

MediaWiki requires the PSR-3 logging library to be present. This library is not embedded directly in MediaWiki's git repository and m

Mine is (hopefully) a simple 1.25 install (git clone followed by git pull) to directory "wiki/" and
another git clone to "wiki/vendor", followed by git pull
The psr directory is visible inside "wiki/vendor" but somehow not being picked up?

Any hints welcome (it's been sometime (1.20) since I installed mediawiki)
I upgraded to 1.24 yesterday - that seemed to go fine.
The setup here has a "central" wiki install, with specifics (like LocalSettings) in their own "wiki" directories, and symlinks for all shared stuff back to the "central" wiki location.

Of course I forgot to symlink to the new "vendor" directory from the wiki instance.
Duh!
Writing the comment above helped me to realise my mistake.

update.php will also now fail if dependencies are missing or out of date (T87789 for the web installer).

Time to re-re-close this? I'm not sure what else I can reasonably do.

Legoktm closed this task as Resolved.Feb 6 2015, 7:04 PM
bd808 moved this task from Untriaged to Done on the Librarization board.Feb 9 2015, 3:33 AM
bd808 moved this task from Backlog to Done on the MediaWiki-Core-Team board.Feb 9 2015, 9:40 PM
bd808 moved this task from Done to Archive on the MediaWiki-Core-Team board.Feb 9 2015, 11:30 PM