Last modified: 2013-10-14 17:53:56 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T57442, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 55442 - ResourceLoader: Exceptions from lessphp cause problems (Exception object is not MWException)
ResourceLoader: Exceptions from lessphp cause problems (Exception object is n...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.22.0
All All
: High major (vote)
: 1.22.0 release
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-07 20:20 UTC by Huji
Modified: 2013-10-14 17:53 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Huji 2013-10-07 20:20:17 UTC
On multiple locations, ResourceLoader.php has a call for $e->logException() where $e is of type Exception.

The logException() function is *NOT* a method for Exception class. It is a method for MWException class. Whenever an exception happens, the user will get the following error message:

  Call to undefined method Exception::logException() in /var/www/wiki/includes/resourceloader/ResourceLoader.php on line 835 


All these references have to be fixed.
Comment 1 Krinkle 2013-10-07 20:22:04 UTC
Indeed, the type hint is incorrect.

What kind of exception are you getting though that isn't an instance of MWException? What is the error message?
Comment 2 Huji 2013-10-07 22:06:50 UTC
I got an exception from the lessc.inc.php when it tried to parse Bootstrap 3. The PHP less included in MW is apparently unable to parse Bootstrap 3 (can parse Boostrap 2.x though), for reasons I couldn't figure out yet.

Regardless, it led me to identify bug 55442.
Comment 3 Krinkle 2013-10-07 22:09:47 UTC
As for the error parsing Bootstrap 3, see https://github.com/leafo/lessphp/issues/481.

As those errors are not of type MWException, fixing the type hint isn't going to do it. We should either abstract the less parsing in a try/catch and re-throw as MWException or generalise this code to not depend on MWException.

Adding Ori to CC.
Comment 4 Krinkle 2013-10-07 22:12:09 UTC
Blocking 1.22 release. Like other exceptions, ResourceLoader should properly catch them all and output as comment and log it, as to not break load.php responses.
Comment 5 Ori Livneh 2013-10-07 23:51:03 UTC
> I got an exception from the lessc.inc.php when it tried to parse Bootstrap 3.

What was the exception?
Comment 6 Huji 2013-10-08 02:44:42 UTC
The message is what I posted above (see Description). I can try to reproduce the bug and dig more, but I'm sure it is due to lessphp not being able to parse Bootstrap 3.
Comment 7 Ori Livneh 2013-10-08 03:44:09 UTC
(In reply to comment #6)
> The message is what I posted above (see Description). I can try to reproduce
> the bug and dig more, but I'm sure it is due to lessphp not being able to
> parse
> Bootstrap 3.

More details and possibly steps to reproduce would be helpful.

Maybe you can replace line 835 with

    if ( !method_exists( $e, 'logException' ) ) debug_print_backtrace();
Comment 8 Gerrit Notification Bot 2013-10-08 21:49:14 UTC
Change 88645 had a related patch set uploaded by Krinkle:
resourceloader: Rethrow LESS error and let makeModuleResponse handle it

https://gerrit.wikimedia.org/r/88645
Comment 9 Huji 2013-10-08 23:58:16 UTC
Ori, I can't reproduce the bug. I did a git pull before trying to reproduce the bug (not so smart, huh?) and because I was using the HEAD revision of MW, lessphp and Bootstrap at the time, going back in time is cumbersome.

If you have any advice about reproducing the bug, please reach me directly by email (so as not to clutter this bug report).
Comment 10 Krinkle 2013-10-14 05:11:39 UTC
Fixed.

Change-Id: Iab98e3a7a9b78d8602e69e0571b35cf107a96b72
Comment 11 Gerrit Notification Bot 2013-10-14 17:53:56 UTC
Change 88645 merged by jenkins-bot:
resourceloader: Don't catch LESS error in ResourceLoaderFileModule

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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links