Page MenuHomePhabricator

Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error

MediaWiki version: 1.36.0-wmf.10

message
Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]

Impact

Notes

Details

Request ID
47883ce5-9b70-479b-ae84-5ef16b70715b
Request URL
https://www.mediawiki.org/w/index.php?title=Reading/Web/Projects/Mobile_Page_Issues&action=submit
Stack Trace
exception.trace
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-1.36.0-wmf.10/includes/debug/MWDebug.php(329): trigger_error(string, integer)
#2 /srv/mediawiki/php-1.36.0-wmf.10/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-1.36.0-wmf.10/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.10/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-1.36.0-wmf.10/languages/Language.php(3341): wfDeprecated(string, string)
#6 /srv/mediawiki/php-1.36.0-wmf.10/languages/Language.php(3271): Language->commafy(string)
#7 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(1176): Language->formatNum(string)
#8 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(1140): Message->extractParam(array, string)
#9 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(872): Message->replaceParameters(string, string, string)
#10 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(929): Message->toString(string)
#11 /srv/mediawiki/php-1.36.0-wmf.10/includes/EditPage.php(3952): Message->parse()
#12 /srv/mediawiki/php-1.36.0-wmf.10/includes/EditPage.php(3248): EditPage::getPreviewLimitReport(ParserOutput)
#13 /srv/mediawiki/php-1.36.0-wmf.10/includes/EditPage.php(699): EditPage->showEditForm()
#14 /srv/mediawiki/php-1.36.0-wmf.10/includes/actions/EditAction.php(71): EditPage->edit()
#15 /srv/mediawiki/php-1.36.0-wmf.10/includes/actions/SubmitAction.php(38): EditAction->show()
#16 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(527): SubmitAction->show()
#17 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(313): MediaWiki->performAction(Article, Title)
#18 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(940): MediaWiki->performRequest()
#19 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(543): MediaWiki->main()
#20 /srv/mediawiki/php-1.36.0-wmf.10/index.php(53): MediaWiki->run()
#21 /srv/mediawiki/php-1.36.0-wmf.10/index.php(46): wfIndexMain()
#22 /srv/mediawiki/w/index.php(3): require(string)
#23 {main}

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 629219 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Disable deprecated warning in Language::commafy() for non numeric string

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

Change 629188 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string

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

Change 629188 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string

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

The culprit here seems to be Scribunto. Eg:

https://codesearch.wmcloud.org/search/?q=scribunto-limitreport&i=nope&files=&repos= shows that we have scribunto-limitreport-timeusage-value defined. But that should be fine because https://gerrit.wikimedia.org/g/mediawiki/extensions/Scribunto/ /2bec230e3d088b8b0218c618ca261c1c6d226497/includes/engines/LuaSandbox/LuaSandboxEngine.php#63 shows that the parameters are numeric.

On the other hand, scribunto-limitreport-logs is definitely non-numeric, and the message key is defined so it will attempt to format it. Patch forthcoming.

(This isn't the case in core: cachereport-origin and limitreport-timingprofile are both non-numeric, but neither of those message keys is defined so EditPage won't try to format them.)

Change 629229 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] EditPage: Don't apply numeric formatting unless a value message is defined

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

Change 629203 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@wmf/1.36.0-wmf.10] Revert "Disable deprecated warning in Language::commafy() for non numeric string"

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

Change 629203 merged by Urbanecm:
[mediawiki/core@wmf/1.36.0-wmf.10] Revert "Disable deprecated warning in Language::commafy() for non numeric string"

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

thcipriani triaged this task as Unbreak Now! priority.Sep 23 2020, 3:32 PM
thcipriani subscribed.

train blockers are UBNs

Change 629426 had a related patch set uploaded (by Ahmon Dancy; owner: Reedy):
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string

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

Change 629219 merged by jenkins-bot:
[mediawiki/core@master] Disable deprecated warning in Language::commafy() for non numeric string

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

Change 629426 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string

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

dancy lowered the priority of this task from Unbreak Now! to High.Sep 23 2020, 6:47 PM
dancy removed a project: Wikimedia-production-error.
dancy subscribed.

The commented out deprecation warning has been deployed to production so I removed the Wikimedia-production-error tag. I changed the priority from UBN to High.

We can either leave this task open for dealing with Scribunto, or a separate task can be created for that and this one can be closed.

Change 629229 merged by jenkins-bot:
[mediawiki/core@master] EditPage: Don't apply numeric formatting unless a value message is defined

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

Change 635374 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Re-enable deprecated warning in Language::commafy() for non-numeric string

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

Change 635374 merged by jenkins-bot:
[mediawiki/core@master] Re-enable deprecation warning in Language::commafy() for non-numeric string

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

Change 635374 merged by jenkins-bot:
[mediawiki/core@master] Re-enable deprecation warning in Language::commafy() for non-numeric string

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

This is now sending deprecation warnings on the beta cluster. 4 seen in the last 24 hours: https://logstash-beta.wmflabs.org/goto/bf153a71f0c6f9d43bb9c20a112cccf4

Look to be media file related

[X4@PCKwQBHcAADooZKUAAABV] /wiki/File:A_solid_ghost_hunting_breakfast_2013-06-05_10-55.jpg   ErrorException from line 3263 of /srv/mediawiki/php-master/languages/Language.php: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-master/includes/debug/MWDebug.php(329): trigger_error(string, integer)
#2 /srv/mediawiki/php-master/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-master/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-master/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-master/languages/Language.php(3333): wfDeprecated(string, string)
#6 /srv/mediawiki/php-master/languages/Language.php(3263): Language->commafy(string)
#7 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(1278): Language->formatNum(string)
#8 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(992): FormatMetadata->formatNum(string)
#9 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(92): FormatMetadata->makeFormattedData(array)
#10 /srv/mediawiki/php-master/includes/media/MediaHandler.php(549): FormatMetadata::getFormattedData(array, RequestContext)
#11 /srv/mediawiki/php-master/includes/media/ExifBitmapHandler.php(140): MediaHandler->formatMetadataHelper(array, RequestContext)
#12 /srv/mediawiki/php-master/includes/filerepo/file/File.php(1919): ExifBitmapHandler->formatMetadata(ForeignDBFile, RequestContext)
#13 /srv/mediawiki/php-master/includes/page/ImagePage.php(131): File->formatMetadata(RequestContext)
#14 /srv/mediawiki/php-master/includes/actions/ViewAction.php(74): ImagePage->view()
#15 /srv/mediawiki/php-master/includes/MediaWiki.php(530): ViewAction->show()
#16 /srv/mediawiki/php-master/includes/MediaWiki.php(316): MediaWiki->performAction(ImagePage, Title)
#17 /srv/mediawiki/php-master/includes/MediaWiki.php(943): MediaWiki->performRequest()
#18 /srv/mediawiki/php-master/includes/MediaWiki.php(546): MediaWiki->main()
#19 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#20 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#21 /srv/mediawiki/w/index.php(3): require(string)
#22 {main}

and 3 are MediaWiki-extensions-PdfHandler related

[X4@QLawQBHcAADooZZgAAABM] /wiki/File:A_Basic_Guide_to_Open_Educational_Resources.pdf?page=2   ErrorException from line 3263 of /srv/mediawiki/php-master/languages/Language.php: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-master/includes/debug/MWDebug.php(329): trigger_error(string, integer)
#2 /srv/mediawiki/php-master/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-master/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-master/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-master/languages/Language.php(3333): wfDeprecated(string, string)
#6 /srv/mediawiki/php-master/languages/Language.php(3263): Language->commafy(string)
#7 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(1278): Language->formatNum(string)
#8 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(992): FormatMetadata->formatNum(string)
#9 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(92): FormatMetadata->makeFormattedData(array)
#10 /srv/mediawiki/php-master/includes/media/MediaHandler.php(549): FormatMetadata::getFormattedData(array, RequestContext)
#11 /srv/mediawiki/php-master/extensions/PdfHandler/includes/PdfHandler.php(367): MediaHandler->formatMetadataHelper(array, RequestContext)
#12 /srv/mediawiki/php-master/includes/filerepo/file/File.php(1919): PdfHandler->formatMetadata(ForeignAPIFile, RequestContext)
#13 /srv/mediawiki/php-master/includes/page/ImagePage.php(131): File->formatMetadata(RequestContext)
#14 /srv/mediawiki/php-master/includes/actions/ViewAction.php(74): ImagePage->view()
#15 /srv/mediawiki/php-master/includes/MediaWiki.php(530): ViewAction->show()
#16 /srv/mediawiki/php-master/includes/MediaWiki.php(316): MediaWiki->performAction(ImagePage, Title)
#17 /srv/mediawiki/php-master/includes/MediaWiki.php(943): MediaWiki->performRequest()
#18 /srv/mediawiki/php-master/includes/MediaWiki.php(546): MediaWiki->main()
#19 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#20 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#21 /srv/mediawiki/w/index.php(3): require(string)
#22 {main}

Got one on my dev wiki too previewing a page... Possibly a customised messsage, but maybe not

Deprecated: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum in /var/www/wiki/mediawiki/core/languages/Language.php at line 3263] in /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php on line 329
1 0.0001 383656 {main}(  ) /var/www/wiki/mediawiki/core/index.php': 0
2 0.0245 3684864 wfIndexMain(  ) /var/www/wiki/mediawiki/core/index.php': 46
3 0.0246 3687008 MediaWiki->run(  ) /var/www/wiki/mediawiki/core/index.php': 53
4 0.0247 3687264 MediaWiki->main(  ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 546
5 0.0295 4366936 MediaWiki->performRequest(  ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 943
6 0.0300 4379872 MediaWiki->performAction(  ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 316
7 0.0302 4380256 SubmitAction->show(  ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 530
8 0.0314 4523968 SubmitAction->show(  ) /var/www/wiki/mediawiki/core/includes/actions/SubmitAction.php': 38
9 0.0319 4563400 EditPage->edit(  ) /var/www/wiki/mediawiki/core/includes/actions/EditAction.php': 71
10 0.0438 5103840 EditPage->showEditForm(  ) /var/www/wiki/mediawiki/core/includes/EditPage.php': 701
11 0.6170 8945768 EditPage::getPreviewLimitReport(  ) /var/www/wiki/mediawiki/core/includes/EditPage.php': 3259
12 0.6441 9019128 Message->parse(  ) /var/www/wiki/mediawiki/core/includes/EditPage.php': 3971
13 0.6441 9019128 Message->toString(  ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 929
14 0.6441 9019128 Message->replaceParameters(  ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 872
15 0.6441 9019128 Message->extractParam(  ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 1140
16 0.6441 9019504 Language->formatNum(  ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 1176
17 0.6441 9019504 Language->commafy(  ) /var/www/wiki/mediawiki/core/languages/Language.php': 3263
18 0.6441 9019504 wfDeprecated(  ) /var/www/wiki/mediawiki/core/languages/Language.php': 3333
19 0.6441 9019504 MWDebug::deprecated(  ) /var/www/wiki/mediawiki/core/includes/GlobalFunctions.php': 1029
20 0.6441 9019616 MWDebug::deprecatedMsg(  ) /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php': 234
21 0.6441 9148120 MWDebug::sendRawDeprecated(  ) /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php': 303
22 0.6441 9148120 <a href='http://www.php.net/function.trigger-error' target='_new'>trigger_error</a>
(  ) /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php': 329
Deprecated: Use of Language::formatNum with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum in /var/www/wiki/mediawiki/core/languages/Language.php at line 3267] in /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php on line 329
Call Stack
#	Time	Memory	Function	Location
1	0.0001	384296	{main}( )	.../index.php:0
2	0.0515	3687360	wfIndexMain( )	.../index.php:46
3	0.0515	3689504	MediaWiki->run( )	.../index.php:53
4	0.0516	3689760	MediaWiki->main( )	.../MediaWiki.php:546
5	0.0570	4369752	MediaWiki->performRequest( )	.../MediaWiki.php:943
6	0.0576	4382688	MediaWiki->performAction( )	.../MediaWiki.php:316
7	0.0579	4383232	SubmitAction->show( )	.../MediaWiki.php:530
8	0.0590	4526944	SubmitAction->show( )	.../SubmitAction.php:38
9	0.0597	4566376	EditPage->edit( )	.../EditAction.php:71
10	0.0728	5112768	EditPage->showEditForm( )	.../EditPage.php:702
11	0.6202	8961576	EditPage::getPreviewLimitReport( )	.../EditPage.php:3282
12	0.6487	9027384	Message->parse( )	.../EditPage.php:3994
13	0.6487	9027384	Message->toString( )	.../Message.php:929
14	0.6487	9027384	Message->replaceParameters( )	.../Message.php:872
15	0.6487	9027384	Message->extractParam( )	.../Message.php:1140
16	0.6487	9027760	Language->formatNum( )	.../Message.php:1176
17	0.6487	9027760	Language->formatNumInternal( )	.../Language.php:3267
18	0.6487	9027760	wfDeprecated( )	.../Language.php:3289
19	0.6487	9027760	MWDebug::deprecated( )	.../GlobalFunctions.php:1029
20	0.6487	9027872	MWDebug::deprecatedMsg( )	.../MWDebug.php:234
21	0.6488	9156376	MWDebug::sendRawDeprecated( )	.../MWDebug.php:303
22	0.6488	9156376	trigger_error ( )	.../MWDebug.php:329

I'm guessing EditPage::getPreviewLimitReport( ) is the key there
It's being passed a string of 5.86 MB

<div class="preview-limit-report-wrapper"><table class="preview-limit-report wikitable"><tbody><tr><th>CPU time usage</th><td>0.222 seconds</td></tr><tr><th>Real time usage</th><td>0.467 seconds</td></tr><tr><th>Preprocessor visited node count</th><td>1,527/1,000,000</td></tr><tr><th>Post-expand include size</th><td>11,392/2,097,152 bytes</td></tr><tr><th>Template argument size</th><td>188/2,097,152 bytes</td></tr><tr><th>Highest expansion depth</th><td>5/40</td></tr><tr><th>Expensive parser function count</th><td>0/100</td></tr><tr><th>Unstrip recursion depth</th><td>0/20</td></tr><tr><th>Unstrip post-expand size</th><td>1,279/5,000,000 bytes</td></tr><tr><th>Lua time usage</th><td>0.030/7 seconds</td></tr><tr><th>Lua virtual size</th><td>5.86 MB/50 MB</td></tr><tr><th>Lua estimated memory usage</th><td>0 bytes</td></tr>

Which pretty printed...

<div class="preview-limit-report-wrapper"><table class="preview-limit-report wikitable">
CPU time usage 0.222 seconds
Real time usage 0.467 seconds
Preprocessor visited node count 1,527/1,000,000
Post-expand include size 11,392/2,097,152 bytes
Template argument size 188/2,097,152 bytes
Highest expansion depth 5/40
Expensive parser function count 0/100
Unstrip recursion depth 0/20
Unstrip post-expand size 1,279/5,000,000 bytes
Lua time usage 0.030/7 seconds
Lua virtual size 5.86 MB/50 MB
Lua estimated memory usage 0 bytes

Looks like it's Scribunto

From LuaStandaloneEngine... I guess both of these are the problem. formatSize

			case 'scribunto-limitreport-virtmemusage':
				$value = array_map( [ $lang, 'formatSize' ], $value );
				break;
			case 'scribunto-limitreport-estmemusage':
				$value = $lang->formatSize( $value );
				break;

and

			$output->setLimitReportData( 'scribunto-limitreport-virtmemusage',
				[
					$status['vsize'],
					$this->options['memoryLimit']
				]
			);
			$output->setLimitReportData( 'scribunto-limitreport-estmemusage',
				$status['vsize'] - $this->initialStatus['vsize']
			);

So either the params want passing in as a number, and then the dynamic units dropping (just hard code bytes like the other values)...

Looking at EditPage

		foreach ( $output->getLimitReportData() as $key => $value ) {
			if ( Hooks::runner()->onParserLimitReportFormat( $key, $value, $limitReport, true, true ) ) {
				$keyMsg = wfMessage( $key );
				$valueMsg = wfMessage( [ "$key-value-html", "$key-value" ] );
				if ( !$valueMsg->exists() ) {
					// This is formatted raw, not as localized number.
					// If you want the parameter formatted as a number,
					// define the `$key-value` message.
					$valueMsg = ( new RawMessage( '$1' ) )->params( $value );
				} else {
					// If you define the `$key-value` or `$key-value-html`
					// message then the argument *must* be numeric.
					$valueMsg = $valueMsg->numParams( $value );
				}
				if ( !$keyMsg->isDisabled() && !$valueMsg->isDisabled() ) {
					$limitReport .= Html::openElement( 'tr' ) .
						Html::rawElement( 'th', null, $keyMsg->parse() ) .
						Html::rawElement( 'td', null, $valueMsg->parse() ) .
						Html::closeElement( 'tr' );
				}
			}
		}

And Scribuntos en.json

	"scribunto-limitreport-virtmemusage": "Lua virtual size",
	"scribunto-limitreport-virtmemusage-value": "$1/$2",
	"scribunto-limitreport-estmemusage": "Lua estimated memory usage",
	"scribunto-limitreport-estmemusage-value": "$1",
	"scribunto-limitreport-memusage": "Lua memory usage",
	"scribunto-limitreport-memusage-value": "$1/$2",

Change 635998 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/Scribunto@master] Format Scribunto memory usage only in bytes for the NewPP limit report

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

With the patch above..

Lua virtual size: 6152192/52428800 bytes
Lua estimated memory usage: 0 bytes

Rather than

Lua virtual size 5.86 MB/50 MB
Lua estimated memory usage 0 bytes

Considering the rest of the report uses bytes, I don't think this is an issue?

<!-- 
NewPP limit report
Parsed by ubuntu64‐web‐esxi
Cached time: 20201023110630
Cache expiry: 86400
Dynamic content: false
Complications: [vary‐revision‐sha1]
CPU time usage: 0.218 seconds
Real time usage: 0.492 seconds
Preprocessor visited node count: 1527/1000000
Post‐expand include size: 11392/2097152 bytes
Template argument size: 188/2097152 bytes
Highest expansion depth: 5/40
Expensive parser function count: 0/100
Unstrip recursion depth: 0/20
Unstrip post‐expand size: 1279/5000000 bytes
Lua time usage: 0.030/7 seconds
Lua virtual size: 6152192/52428800 bytes
Lua estimated memory usage: 0 bytes
-->

Which is only one of the usages, helpfully.

So the messages need changing for the NewPP report... Removing the formatSize calls is not helpful as it removes the pretty formatting, but also prints them as a raw number on that HTML output. Splitting my patch

No, I was right the first time.

Basically, the formatSize result in it being formatted twice. Once in Scribunto code, once in EditPage::getPreviewLimitReport() which causes the problem.

The hook does pass localise = true, so we want to keep the formatting...

Ok, PS4 now uses the hook handlers in Scribunto for onParserLimitReportFormat to do all the formatting, and then is just dealt with as "raw" in the output in EditPage

Keeps the possibility for localisation if needed... Keeps it as pretty formatted numbers. I dunno if that's actually wanted in prefixed bytes, but that's how it was done in b58ee1da94b9666092e01127a4d58ae01c4e5494...

Change 636005 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/Scribunto@master] Format Scribunto Lua Preview Limit Report memory numbers in bytes

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

So two options

Keep the output the same, formatting it with units (other code doesn't do this). Ignore this commit summary it's out of date

Change 635998 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/Scribunto@master] Format Scribunto memory usage only in bytes for the NewPP limit report

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

Or just simplify the code and output everything in byte(s) (with plural support!)

Change 636005 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/Scribunto@master] Format Scribunto Lua Preview Limit Report memory numbers in bytes

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

I don't know if/how much extra value having it output in bytes with prefixes (mega, kilo etc) is if the others aren't... It's definitely inconsistent and reduced hacks/workarounds in the code

Thanks for looking into this. Just to throw it out there, another option is to undefine the -value message, which will ensure the output is reported literally and not formatted. This would work for scribunto-limitreport-estmemusage-value which currently has the value $1. For scribunto-limitreport-virtmemusage-value and scribunto-limitreport-memusage-value you could do the $1/$2 formatting when those values are defined, and the undefine the i18n message to prevent the formatting from being applied twice.

This would look something like https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/ /636046

EDIT: nevermind, I see this is very similar to what you did in 635998

Change 636046 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/Scribunto@master] Don't double-format memusage in limit report

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

Change 636046 abandoned by C. Scott Ananian:
[mediawiki/extensions/Scribunto@master] Don't double-format memusage in limit report

Reason:
Abandoned in favor of If8a71419d656530859552abaddeed66d5a9ddc4b

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

Change 636063 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Don't try to formatNum() non-numeric media metadata

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

Look to be media file related
and 3 are MediaWiki-extensions-PdfHandler related

Patch in 636063 for that, although it's kind of a big hammer approach. I suspect there is double-formatting going on here as well, and it would be nice to have a finer-grained approach.

Do we want to make this a blocker for wmf.16? (No train next week, but high-priority because deprecation spam isn't meant to exist in prod. ;-))

Change 636063 merged by jenkins-bot:
[mediawiki/core@master] Don't try to formatNum() non-numeric media metadata

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

Change 635998 merged by jenkins-bot:
[mediawiki/extensions/Scribunto@master] Fix double formatting of memory units

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

Look to be media file related

[X4@PCKwQBHcAADooZKUAAABV] /wiki/File:A_solid_ghost_hunting_breakfast_2013-06-05_10-55.jpg   ErrorException from line 3263 of /srv/mediawiki/php-master/languages/Language.php: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-master/includes/debug/MWDebug.php(329): trigger_error(string, integer)
#2 /srv/mediawiki/php-master/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-master/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-master/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-master/languages/Language.php(3333): wfDeprecated(string, string)
#6 /srv/mediawiki/php-master/languages/Language.php(3263): Language->commafy(string)
#7 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(1278): Language->formatNum(string)
#8 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(992): FormatMetadata->formatNum(string)
#9 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(92): FormatMetadata->makeFormattedData(array)
#10 /srv/mediawiki/php-master/includes/media/MediaHandler.php(549): FormatMetadata::getFormattedData(array, RequestContext)
#11 /srv/mediawiki/php-master/includes/media/ExifBitmapHandler.php(140): MediaHandler->formatMetadataHelper(array, RequestContext)
#12 /srv/mediawiki/php-master/includes/filerepo/file/File.php(1919): ExifBitmapHandler->formatMetadata(ForeignDBFile, RequestContext)
#13 /srv/mediawiki/php-master/includes/page/ImagePage.php(131): File->formatMetadata(RequestContext)
#14 /srv/mediawiki/php-master/includes/actions/ViewAction.php(74): ImagePage->view()
#15 /srv/mediawiki/php-master/includes/MediaWiki.php(530): ViewAction->show()
#16 /srv/mediawiki/php-master/includes/MediaWiki.php(316): MediaWiki->performAction(ImagePage, Title)
#17 /srv/mediawiki/php-master/includes/MediaWiki.php(943): MediaWiki->performRequest()
#18 /srv/mediawiki/php-master/includes/MediaWiki.php(546): MediaWiki->main()
#19 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#20 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#21 /srv/mediawiki/w/index.php(3): require(string)
#22 {main}

So imported https://upload.beta.wmflabs.org/wikipedia/commons/c/c0/A_solid_ghost_hunting_breakfast_2013-06-05_10-55.jpg

<small>/var/www/wiki/mediawiki/core/includes/media/FormatMetadata.php:1280:</small><small>string</small> <font color='#cc0000'>'&#0;&#0;&#0;&#0;'</font> <i>(length=4)</i>

And if we use ctype_print it's "false".... But its value is \u0000\u0000\u0000\u0000

It's from a "tag" of FlashPixVersion. FlashpixVersion is handled, FlashPixVersion is not. It's case sensitive...

Exif.php has a different casing already (unhelpfully)

'FlashPixVersion' => self::UNDEFINED, # Supported Flashpix version #p32

From a look online, FlashpixVersion looks reasonably well used, but that doesn't mean FlashPixVersion is actually incorrect...

And it may just be a device in the wild implementing it incorrectly (and as such should be commented as such)

Change 636119 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] media: Handle FlashpixVersion in FormatMetadata::makeFormattedData()

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

Change 636120 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] [DNM] Add some more debugging for badly formatted numbers which probably come from unknown exif tags

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

and 3 are MediaWiki-extensions-PdfHandler related

[X4@QLawQBHcAADooZZgAAABM] /wiki/File:A_Basic_Guide_to_Open_Educational_Resources.pdf?page=2   ErrorException from line 3263 of /srv/mediawiki/php-master/languages/Language.php: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array)
#1 /srv/mediawiki/php-master/includes/debug/MWDebug.php(329): trigger_error(string, integer)
#2 /srv/mediawiki/php-master/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string)
#3 /srv/mediawiki/php-master/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer)
#4 /srv/mediawiki/php-master/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer)
#5 /srv/mediawiki/php-master/languages/Language.php(3333): wfDeprecated(string, string)
#6 /srv/mediawiki/php-master/languages/Language.php(3263): Language->commafy(string)
#7 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(1278): Language->formatNum(string)
#8 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(992): FormatMetadata->formatNum(string)
#9 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(92): FormatMetadata->makeFormattedData(array)
#10 /srv/mediawiki/php-master/includes/media/MediaHandler.php(549): FormatMetadata::getFormattedData(array, RequestContext)
#11 /srv/mediawiki/php-master/extensions/PdfHandler/includes/PdfHandler.php(367): MediaHandler->formatMetadataHelper(array, RequestContext)
#12 /srv/mediawiki/php-master/includes/filerepo/file/File.php(1919): PdfHandler->formatMetadata(ForeignAPIFile, RequestContext)
#13 /srv/mediawiki/php-master/includes/page/ImagePage.php(131): File->formatMetadata(RequestContext)
#14 /srv/mediawiki/php-master/includes/actions/ViewAction.php(74): ImagePage->view()
#15 /srv/mediawiki/php-master/includes/MediaWiki.php(530): ViewAction->show()
#16 /srv/mediawiki/php-master/includes/MediaWiki.php(316): MediaWiki->performAction(ImagePage, Title)
#17 /srv/mediawiki/php-master/includes/MediaWiki.php(943): MediaWiki->performRequest()
#18 /srv/mediawiki/php-master/includes/MediaWiki.php(546): MediaWiki->main()
#19 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#20 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#21 /srv/mediawiki/w/index.php(3): require(string)
#22 {main}

Grabbing the PDF from https://en.wikipedia.beta.wmflabs.org/wiki/File:A_Basic_Guide_to_Open_Educational_Resources.pdf / https://upload.wikimedia.org/wikipedia/commons/b/b4/A_Basic_Guide_to_Open_Educational_Resources.pdf (or, well, actually just using it via instant commons)

There seems to be an exif header of pdf-Encrypted and a value of no.

This is added by MediaWiki-extensions-PdfHandler, but then MW borks because it's not numeric, and no real way to identify it's not supposed to be. Everything else seems to be standard and known about in MW... Or numeric

				case 'Encrypted':
					// @todo: The value isn't i18n-ised. The appropriate
					// place to do that is in FormatMetadata.php
					// should add a hook a there.
					// For reference, if encrypted this fields value looks like:
					// "yes (print:yes copy:no change:no addNotes:no)"
					$items['pdf-Encrypted'] = $val;

Added in rEPHD7663952c3451: Display metadata from PDF files on image description page.

So how should we be dealing with these? adding a "hack" to MW core... Or implementing some sort of hook for the default statement of switch ( $tag ) in FormatMetadata::makeFormattedData() and allowing things to offer up ways to format these...? I wonder if a task was ever filed about that.

Funnily, looking at the comment above...

				// These last two (version and encryption) I was unsure
				// if we should include in the table, since they aren't
				// all that useful to editors. I leaned on the side
				// of including. However not including if file
				// is optimized/linearized since that is really useless
				// to an editor.
				case 'PDF version':
					$items['pdf-Version'] = $val;
					break;

Potentially, we could just remove these and they'll go away from the exif output.

As per the question, are they actually useful?

I imagine "encrypted"-ness isn't much use on commons et al. If it's encrypted, we probably don't want it. On a private wiki? Maybe it's more useful. But of course, if it is encrypted, it's not going to thumb etc

Change 636120 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] [DNM] Add some more debugging for badly formatted numbers which probably come from unknown exif tags

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

Something like this would be potentially useful... Maybe rather than the deprecation here and in Language::commafy we should be disabling the deprecation again, and adding some better structured logging so we can play whack-a-mole for a while, and then when those logs are cleaner, re-enable the deprecation. At least in this case in FormatMetadata (more so than Language::commafy with it's many callers all over the place), we can easily add a parameter for referencing the exif tag (as done in the DNM patch atm; which also fudges the wfDeprecated text which is kinda nasty. Hence DNM). And as the function is private, easily remove it again a little down the way without causing any breaking changes

Or in the case of formatNum in FormatMetadata, maybe passing in non numbers is fine to this private method... Just returning them verbatim if they're not numerical and therefore can't be formatted.

Certainly, it's going to need something like the aformentioned hook to allow the extra stuff that MediaWiki-extensions-PdfHandler
adds in to be able to deal with formatting them. Or passing in like a Message object, and then not calling formatNum() with it passed (and we're in the switch ( $tag ) default condition if $val instanceof Message
Otherwise we're shooting fairly blind with these deprecation warnings, which in WMF prod, could be fairly prolific

Change 636219 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_35] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()

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

Change 636220 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_31] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()

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

Change 636220 merged by jenkins-bot:
[mediawiki/core@REL1_31] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()

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

Change 636219 merged by jenkins-bot:
[mediawiki/core@REL1_35] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()

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

Change 636119 merged by jenkins-bot:
[mediawiki/core@master] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()

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

Change 636120 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] [DNM] Add some more debugging for badly formatted numbers which probably come from unknown exif tags

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

Something like this would be potentially useful... Maybe rather than the deprecation here and in Language::commafy we should be disabling the deprecation again, and adding some better structured logging so we can play whack-a-mole for a while, and then when those logs are cleaner, re-enable the deprecation. At least in this case in FormatMetadata (more so than Language::commafy with it's many callers all over the place), we can easily add a parameter for referencing the exif tag (as done in the DNM patch atm; which also fudges the wfDeprecated text which is kinda nasty. Hence DNM). And as the function is private, easily remove it again a little down the way without causing any breaking changes

Or in the case of formatNum in FormatMetadata, maybe passing in non numbers is fine to this private method... Just returning them verbatim if they're not numerical and therefore can't be formatted.

Certainly, it's going to need something like the aformentioned hook to allow the extra stuff that MediaWiki-extensions-PdfHandler
adds in to be able to deal with formatting them. Or passing in like a Message object, and then not calling formatNum() with it passed (and we're in the switch ( $tag ) default condition if $val instanceof Message
Otherwise we're shooting fairly blind with these deprecation warnings, which in WMF prod, could be fairly prolific

It looks to me like there's already an endpoint which is *intended* to allow the particular *Handler class to do the proper formatting, since the comments all discuss how the main point of these methods is to turn opaque magic numbers into human-readable strings. But in any case https://gerrit.wikimedia.org/r/636063 separated out the media-related issue from the others; the wfDeprecated there could easily be downgraded to a debug log. I *think* the main commafy/formatNum path is actually fixed, though, so I'd rather not turn that deprecation warning off. We should probably split off a new task for the whack-a-mole based on media metadata, which should now be generating a deprecation warning in FormatMetadata::formatNum() rather than one in commafy.

This is now sending deprecation warnings on the beta cluster. 4 seen in the last 24 hours: https://logstash-beta.wmflabs.org/goto/bf153a71f0c6f9d43bb9c20a112cccf4

None of the Language::formatNum() warnings seen since then, but we're still seeing FormatMetadata-related warnings now: https://logstash-beta.wmflabs.org/goto/5438534501b787a1d15cf41809637a35

Looks like PDF, which @Reedy investigated in T263592#6575833 but I don't think has been patched yet.

I'm going to fork off T266677: Use of FormatMetadata::formatNum with non-numeric value was deprecated in MediaWiki 1.36. [Called from FormatMetadata::makeFormattedData] for these media-related cases. @Jdforrester-WMF, that's the task which might be a production blocker, if any. We'll use this task for any other bad uses of Language::formatNum() which slip through, hopefully there aren't any and this bug can be closed after we confirm this doesn't show up in the logs again when the train rolls next week.

Change 636005 merged by jenkins-bot:
[mediawiki/extensions/Scribunto@master] Format Scribunto Lua Preview Limit Report memory numbers in bytes

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

Change 636120 abandoned by Reedy:

[mediawiki/core@master] [DNM] Add some more debugging for badly formatted numbers which probably come from unknown exif tags

Reason:

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

Language::commafy was dropped from MW 1.40.