More info in case that's useful:

Error

• service.version: 1.44.0-wmf.1
• trace.id: d1a455ff-0881-40ad-b8a8-aa5b87a94b85
• Find trace.id in Logstash

[{reqId}] {exception_url}   TypeError: Argument 2 passed to Wikimedia\Minify\IndexMap::addEncodedMap() must be an instance of Wikimedia\Minify\IndexMapOffset, null given, called in /srv/mediawiki/php-1.44.0-wmf.1/includes/ResourceLoader/ResourceLoader.php

Notes

• Since rollout of 1.44.0-wmf.1 seeting this show up for both testwiki and cawiki (group0 and group1, respectively)

Change #1095611 had a related patch set uploaded (by Hokwelum; author: Hokwelum):

[mediawiki/libs/Minify@master] Do not minify sources with /*@nomin*/ pragma but add source content to sourceMap

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

Change #1092308 had a related patch set uploaded (by Hokwelum; author: Hokwelum):

[mediawiki/core@master] Fix: $generatedSize must be of type Wikimedia\Minify\IndexMapOffset, null given

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

Still noted in 1.44.0-wmf.12 (T382363).

Still noted in 1.44.0-wmf.16/17. Politely pinging in hopes of getting this resolved. I see that there are two open patches related to this ticket. Can anyone help wrap it up?

Adding PHP 8.1 support given that I've only seen the error occurring on kube-mw-web-next as of the last week.

In T378498#10603526, @Dreamy_Jazz wrote:

Adding PHP 8.1 support given that I've only seen the error occurring on kube-mw-web-next as of the last week.

It definitely occurs on PHP 7.4

Error

• mwversion: 1.44.0-wmf.19
• reqId: 6144c5cb-eb04-4dab-885e-86c0a3d0f1b5
• Find reqId in Logstash

[{reqId}] {exception_url}   TypeError: Argument 2 passed to Wikimedia\Minify\IndexMap::addEncodedMap() must be an instance of Wikimedia\Minify\IndexMapOffset, null given, called in /srv/mediawiki/php-1.44.0-wmf.19/includes/ResourceLoader/ResourceLoader.ph

Impact

Notes

Maybe I did the filters wrong then in logstash. Removing PHP 8.1 support

In T378498#10608157, @Dreamy_Jazz wrote:

Maybe I did the filters wrong then in logstash. Removing PHP 8.1 support

Indeed, it's happening on both 7.4 and 8.1, it's just that the TypeError exception text varies between PHP versions.

Change #1092308 had a related patch set uploaded (by Reedy; author: Hokwelum):

[mediawiki/core@master] Fix: $generatedSize must be of type Wikimedia\Minify\IndexMapOffset, null given

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

Error

• mwversion: 1.44.0-wmf.23
• reqId: cc443a70-40bd-47b2-81be-e9a25002a32e
• Find reqId in Logstash

[{reqId}] {exception_url}   TypeError: Wikimedia\Minify\IndexMap::addEncodedMap(): Argument #2 ($generatedSize) must be of type Wikimedia\Minify\IndexMapOffset, null given, called in /srv/mediawiki/php-1.44.0-wmf.23/includes/ResourceLoader/ResourceLoader.

Impact

Notes

@matmarex and I discussed this in #mediawiki-core on IRC yesterday (logs).

What we tried so far

The above patches took an approach that Hannah and I learned would not work, which is why we haven't proceeded them.

Basically, when we're building source maps (implemented in T47514: ResourceLoader: Implement support for Source Maps), we minify one file at a time, because source maps don't just translate past minification, they also translate past bundling. So each chunk also tells the source map which original URL it should map to (or pseudo-URL, for virtual files and non-file chunks such as WikiModule). And, after that chunk, the offset will continue cursoring through the actual minified bundle with the next chunk.

I could not find a straight-forward way to "skip" over a chunk that is not-minified in the first place (as would be with the @nomin instruction). At the very least, the avoid breaking source maps for the rest of the load.php response, the line/offset would need to be internally incremented past the non-minified chunk. This is non-trivial due to to some of those being private, but do-able with a minor refactor. However, the exact offset is still non-trivial in that case given handling of new lines and wrappers that are easily to miss without an active minifier state being passed around. Let's suppose we manage that. That still leaves us with a perfectly good file completely unmapped. And the way source maps work is that, if a sequence of tokens isn't mapped, it continues from the last mapped token. This means, given "a.js, b-nomin.js, c.js" in a bundled response, "b-nomin.js" would be attributed to non-existing past-end-of-file line numbers in "a.js".

One thing we tried is to insert a minimum viable chunk that correctly maps the first token in b-nomin.js, and then don't map anything until we start c.js correctly again. I feel this like this should work, but we were unable to come up with a minimal encoding that the browser would accept. If we spend more time on this, I would read the source map spec more closely a few more times, and perhaps use one of those "source map visualizer" tools to get a better understanding of how we'd represent something minimally there. The idea is that the way that browsers default to just assuming "the next line is the next line" without mapping information, is exactly what we want for unminified chunks. Except I couldn't find a way to trigger that cleanly.

Another thing we tried is to synthesize a source map chunk that defines the entire unminified file as a single very long token. This worked in so far that it no longer corrupts mapping of prior (a.js) and later files (c.js), but it didn't have the intended effect. This means that traces for anywhere in b-nomin.js were attributed to b-nomin.js:1:1, i.e. the start of the file.

The last thing we tried is to add a "non-minifying mode" in JavaScriptMinifier, i.e. where we do parse every token and whitespace sequence, but don't remove or change anything, just add it to the source map as-is. This can work in theory, but it requires getting it exactly right, because in production mode, we don't call the minifier, so if anything that we skip over or condendse, even if in a very minor cosmetic way, the source map is off. The patches above reflect this approach, and for the basic three-module bundle we tested with, the offset was still wrong by one or two.

Bartosz' idea

<matmarex> i wonder, do we still need @nomin at all? afair it was mostly a workaround for us lacking support for modern JS syntax. but we have that support now.

• https://codesearch.wmcloud.org/search/?q=FILTER_NOMIN&files=php&excludeFiles=test&repos=
• https://codesearch.wmcloud.org/search/?q=%40nomin&files=js%24&excludeFiles=test&repos=

Usage slipped into the AdvancedSearch extension, which copied some old now-changed code from core which in turn used to use NOMIN. Trivial to remove.

• It was there in core originally for user.tokens, as a way of disabling the APCU cache for minification, not so much minification itself. Because that user.tokens is just a plain JSON blob, there is nothing to minify anyway. The reason is that user.tokens are different one very pageview, so it exploded php-apcu with infinite entries for every randomly generated token, and it wasn't even useful because the next pageview even from the same user would generate a different user.tokens blob and thus not hit the cache, because the content-hash key would be different each time as well.
• This was solved a long time ago by excluding group=private from the minification cache (this code has moved around over the years, but now lives in ResourceLoader::getOneModuleResponse where ResourceLoader::filter is called with cache => false if $module->shouldEmbedModule is true, which in turn checks getGroup == private.

There was also use of NOMIN in the ContentTranslate extension since 2020, for a then-unsupported ES6 token (T255556). That has since been removed as well since we added support for ES6 natively in 2021 (T272882).

Lastly, with T368204: Consider removing new-line insertion from JavaScriptMinifier use of future/unsupported tokens is much less likely to affect payloads in the first place, so that also further removes the need for this feature.

So yeah, maybe it's time to pull the plug and remove this feature!
The Codesearch links above show 3 Wikibase files, which pass ESLint with es2017 (one of them fails es2015, but passes es2017), so that too doens't need @nomin anymore.

Change #1095611 abandoned by Hokwelum:

[mediawiki/libs/Minify@master] Do not minify sources with /*@nomin*/ pragma but add source content to sourceMap

Reason:

We are removing the @nomin pragma.. See task details

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

Change #1152379 had a related patch set uploaded (by Hokwelum; author: Hokwelum):

[mediawiki/extensions/AdvancedSearch@master] Remove usage of ResourceLoader::FILTER_NOMIN

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

Change #1152379 merged by jenkins-bot:

[mediawiki/extensions/AdvancedSearch@master] Remove usage of ResourceLoader::FILTER_NOMIN

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

Change #1153746 had a related patch set uploaded (by Hokwelum; author: Hokwelum):

[mediawiki/extensions/Wikibase@master] Remove @nomin pragma

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

Change #1092308 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Remove "@nomin" feature

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

Change #1153746 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Remove @nomin pragma

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