Page MenuHomePhabricator

Shut up filemtime() warnings in ResourceLoader (and possibly other warnings related to nonexistent files as well)
Closed, ResolvedPublic

Description

We don't check file existence prior to calling filemtime(), and possibly other functions operating on files as well. This causes PHP warnings to be thrown for nonexistent files. These are nicely caught and displayed in a comment so they don't mess up CSS/JS code, but they do still spam server logs, so we should avoid them.


Version: 1.17.x
Severity: enhancement

Details

Reference
bz26486

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:20 PM
bzimport set Reference to bz26486.

Since you gave this to yourself, Roan, did you get it done?

(In reply to comment #1)

Since you gave this to yourself, Roan, did you get it done?

Not yet, no, but it's not a blocker.

(In reply to comment #2)

Not yet, no, but it's not a blocker.

I wasn't worried that it is a blocker, but it had been a month and I just didn't want to see this disappear. Thanks for the reply :)

cnit wrote:

[Fri May 06 14:04:24 2011] [error] [client 10.1.1.119] PHP Warning: filemtime(): stat failed for /var/www/wiki/farm117/skins/common/password.css in /var/www/wiki/farm117/includes/resourceloader/ResourceLoaderFileModule.php on line 363,

[Fri May 06 14:04:24 2011] [error] [client 10.1.1.119] PHP Warning: filemtime(): stat failed for /var/www/wiki/farm117/skins/common/password.js in /var/www/wiki/farm117/includes/resourceloader/ResourceLoaderFileModule.php on line 363,

max( array_map( 'filemtime', $files ) ) is nice, however it really does not check for file existence.

What is password.css and password.js, by the way?

(In reply to comment #4)

What is password.css and password.js, by the way?

Something with password strength checking, don't quite remember. The feature was pulled out of the 1.17 branch but apparently the module declaration was left in. It was just being useless and spewing warnings so I killed it in r87534.

cnit wrote:

I've updated from /branches/REL1_17/phase3/ and now it gives me a more severe error (probably unrelated, but...)
[Sat May 07 22:26:39 2011] [error] [client 213.187.118.145] PHP Fatal error: Class 'CSSMin' not found in /var/www/wiki/farm117/includes/resourceloader/ResourceLoaderFileModule.php on line 504,

Something is really broken, either class is renamed or missing. I have php 5.3.6, it should be quite stable.

cnit wrote:

That wiki also has latest stable Semantic MediaWiki 1.5.6 and Extension:Poem from trunk, if that is relevant.

cnit wrote:

Updated again, now it is fixed. I didn't expect such untested revision, however.

  • Bug 29207 has been marked as a duplicate of this bug. ***

Fixed in r94433. There is one warning left, but that's due to a PHP bug; see the commit summary for more information.

In my install, I have deleted some of the older skins (modern, chick, and simple). I'm getting errors in my error log about main.css and print.css for these skins, even though I have listed them in $wgSkipSkins.

Will this fix address that scenario as well?

(In reply to comment #11)

In my install, I have deleted some of the older skins (modern, chick, and
simple). I'm getting errors in my error log about main.css and print.css for
these skins, even though I have listed them in $wgSkipSkins.

Will this fix address that scenario as well?

No, you should not remove those files. Adding them to $wgSkipSkins is enough to stop users from using them, removing them from the harddisk will and should throw errors.

Xcollector838 subscribed.

This issue still shows when starting to use onLogException hook. In the code below you can see the LocalisationCache keeps trying to get some .json file, even though it creates .cdb files. So this is also a bug for them I suppose. I'll probably have a new issue for this one but anyway, why not simply use file_exists() ? Is it because it'll cause two file system calls?

onLogException PHP Warning: filemtime(): stat failed for /cache/wikicache/l10nupdate-en.json

#0 [internal function]: MWExceptionHandler::handleError(2, 'filemtime(): st...', '/var/www/wiki/i...', 202, Array)
#1 /var/www/wiki/includes/cache/CacheDependency.php(202): filemtime('/cache/wikicach...')
#2 /var/www/wiki/includes/cache/localisation/LocalisationCache.php(431): FileDependency->isExpired()
#3 /var/www/wiki/includes/cache/localisation/LocalisationCache.php(462): LocalisationCache->isExpired('en')
#4 /var/www/wiki/includes/cache/localisation/LocalisationCache.php(338): LocalisationCache->initLanguage('en')
#5 /var/www/wiki/includes/cache/localisation/LocalisationCache.php(275): LocalisationCache->loadItem('en', 'rtl')
#6 /var/www/wiki/languages/Language.php(3027): LocalisationCache->getItem('en', 'rtl')
#7 /var/www/wiki/languages/Language.php(3035): Language->isRTL()
#8 /var/www/wiki/includes/resourceloader/ResourceLoaderContext.php(200): Language->getDir()
#9 /var/www/wiki/includes/resourceloader/ResourceLoaderFileModule.php(920): ResourceLoaderContext->getDirection()
#10 /var/www/wiki/includes/resourceloader/ResourceLoaderFileModule.php(396): ResourceLoaderFileModule->getFlip(Object(ResourceLoaderContext))
#11 /var/www/wiki/includes/resourceloader/ResourceLoaderModule.php(648): ResourceLoaderFileModule->getStyles(Object(ResourceLoaderContext))
#12 /var/www/wiki/includes/resourceloader/ResourceLoaderModule.php(595): ResourceLoaderModule->buildContent(Object(ResourceLoaderContext))
#13 /var/www/wiki/includes/resourceloader/ResourceLoader.php(1011): ResourceLoaderModule->getModuleContent(Object(ResourceLoaderContext))
#14 /var/www/wiki/includes/resourceloader/ResourceLoader.php(736): ResourceLoader->makeModuleResponse(Object(ResourceLoaderContext), Array, Array)
#15 /var/www/wiki/load.php(53): ResourceLoader->respond(Object(ResourceLoaderContext))
#16 {main}

@Xcollector838 Despite ResourceLoader showing in the stack trace, the exception you caught is not related to ResourceLoader nor this task. Re-closing as such.

Please file a new task about a filemtime() warning coming from LocalisationCache (tag MediaWiki-Internationalization and MediaWiki-libs-BagOStuff).