r97144 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97143‎ | r97144 | r97145 >
Date:11:12, 15 September 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Merge r82361 from 1.17wmf1 to trunk. This shuts up "Non-string key given" exceptions in cases where a numeric key manages to become an integer somehow. This could probably be fixed in a better way but I wouldn't know how.
Modified paths:
  • /trunk/phase3/includes/cache/MessageCache.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/cache/MessageCache.php
——@@ -584,6 +584,11 @@
585585 function get( $key, $useDB = true, $langcode = true, $isFullKey = false ) {
586586 global $wgLanguageCode, $wgContLang;
587587
 588+ if ( is_int( $key ) ) {
 589+ // "Non-string key given" exception sometimes happens for numerical strings that become ints somewhere on their way here
 590+ $key = strval( $key );
 591+ }
 592+
588593 if ( !is_string( $key ) ) {
589594 throw new MWException( 'Non-string key given' );
590595 }
Property changes on: trunk/phase3/includes/cache/MessageCache.php
___________________________________________________________________
Added: svn:mergeinfo
591596 Merged /branches/new-installer/phase3/includes/cache/MessageCache.php:r43664-66004
592597 Merged /branches/wmf/1.17wmf1/includes/MessageCache.php:r82361
593598 Merged /branches/wmf-deployment/includes/cache/MessageCache.php:r53381
594599 Merged /branches/REL1_15/phase3/includes/cache/MessageCache.php:r51646
595600 Merged /branches/sqlite/includes/cache/MessageCache.php:r58211-58321

Follow-up revisions

RevisionCommit summaryAuthorDate
r97173Merged revisions 97087,97091-97092,97094,97096-97098,97100-97101,97103,97136,...dantman16:19, 15 September 2011
r97276REL1_18 MFT r97144, r97146, r97192reedy14:41, 16 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r823611.17wmf1: Live hack to shut up "Non-string key given" exceptions for numeric ...catrope23:07, 17 February 2011

Comments

#Comment by Nikerabbit (talk | contribs)   12:24, 15 September 2011

How about failing fast with an exception?

#Comment by Catrope (talk | contribs)   12:25, 15 September 2011

Well, that's what the existing behavior was. It was quite intrusive on the cluster when we deployed 1.17.

#Comment by Nikerabbit (talk | contribs)   12:25, 15 September 2011

So did you find the cause?

#Comment by Catrope (talk | contribs)   12:27, 15 September 2011

No. If we had looked at the backtraces back then, we might have.

How about we put in a logging thing that logs backtraces for these things but doesn't throw user-visible error messages?

#Comment by Nikerabbit (talk | contribs)   12:38, 15 September 2011

That's fine with me. I haven't seen these issues myself. It concerns me if we are hiding likely bugs.

Status & tagging log