For some reason this did not show up on our Growth Logstash dashboard until I removed the "Our channels and traces"-filter. Seems we may have to adjust that.

In T388725#10646832, @Michael wrote:

For some reason this did not show up on our Growth Logstash dashboard until I removed the "Our channels and traces"-filter. Seems we may have to adjust that.

I've changed the "Our channels and traces"-filter to also match on traces with Echo in them.

The error seems to originate from this unserialize call:

		try {
			$this->extra = $row->event_extra ? unserialize( $row->event_extra ) : [];
		} catch ( Exception $e ) {
			// T73489: unserializing can fail for old notifications
			LoggerFactory::getInstance( 'Echo' )->warning(
				'Failed to unserialize event {id}',
				[
					'id' => $row->event_id
				]
			);
			return false;
		}

The comment points to T73489: Echo: Special:Notifications Exception from line of : DateTimeZone::__construct(): Unknown or bad timezone (+00:00), but that looks unrelated at first glance.

Do we have bad data in the database somehow? Or did it somehow turn bad after the fact?

A StackOverflow question seems at the possibility that a class name or namespace might have changed: https://stackoverflow.com/questions/73447771/warning-class-php-incomplete-class-has-no-unserializer

➡️ getting more info about the specific rows might be helpful

Confusingly, the logging from the above Exception handler does not seem to take place?

Did we recently change how unserialization is performed? Maybe by upgrading a vendor library or something? Typically, such a warning should not be logged, and only a __PHP_Incomplete_Class should be created.

At least, that would explain why the exception handler is no triggered: the unserialization "completes" by creating a __PHP_Incomplete_Class.

That being said, have that class there is still not what we want. We should log that and figure out what is going on.

Looking into my local table for this, I'm seeing a bit of data from Flow that looks very suspicious. However, I would expect that to produce a different error?

Change #1129220 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/Echo@master] Add logging to help figure unserialization issues

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

"__PHP_Incomplete_Class: Created by unserialize() when trying to unserialize an undefined class…" (https://www.php.net/manual/en/class.php-incomplete-class.php)

First guess is that this is caused by removing some non-namespaced alias while we have serialized instances of that non-namespaced class somewhere in the Echo tables.

This error was really confusing me, so I tried to find out what's going on.

There doesn't seem to be any documentation in the internet about what Class <class> has no unserializer really means, so I went to read the PHP source code. I learned here: https://github.com/php/php-src/blob/fb07c62f2cba5941f02a1b5b02580f3aa40c7c69/ext/standard/var_unserializer.re#L774 that this happens when the serialized class instance belongs to a class that implements Serializable (https://www.php.net/manual/en/class.serializable.php), which are serialized in a different format, but the currently existing class with that name does not implement it. __PHP_Incomplete_Class naturally doesn't implement it, so this warning happens, but this seems like a PHP bug to emit it for this case, since the real problem is that the class does not exist at all, not that it has no unserializer… And it's also worth noting that there's no warning for when the serialized class instance belongs to a class that doesn't exist; that just silently creates a __PHP_Incomplete_Class instance. So we might have those floating around our systems with no logs about it.

Here's how to reproduce the problem locally, using maintenance/shell.php:

> use MediaWiki\Extension\Notifications\Model\Event;
> class MyTempClass implements Serializable {
  public function serialize() {
    return serialize("hello");
  }
  public function unserialize($data) {
  }
}

> Event::create( [ 'type' => 'welcome', 'agent' => User::newFromName('Matma Rex'), 'extra'=>['x'=>new MyTempClass] ] )
= MediaWiki\Extension\Notifications\Model\Event {#6275}

You will now see these warnings whenever the notification is shown.

if you do the same thing with a class that is not Serializable:

> class MyTempClass2 {}
> Event::create( [ 'type' => 'welcome', 'agent' => User::newFromName('Matma Rex'), 'extra'=>['x'=>new MyTempClass2] ] )
= MediaWiki\Extension\Notifications\Model\Event {#4945}

… you will not see any warnings, but you probably should.

Reading the docs, I found that we can set a callback that is called when a serialized class instance belongs to a class that doesn't exist: https://www.php.net/manual/en/var.configuration.php#ini.unserialize-callback-func. We could probably use this to log a warning of our own, including for the case where the missing class doesn't implement Serializable (so it doesn't cause *this* warning).

I tried adding this in my LocalSettings.php:

ini_set('unserialize_callback_func', 'wfUnserializeMissingClass');
function wfUnserializeMissingClass($className) {
  MediaWiki\Logger\LoggerFactory::getInstance('unserialize')->error( 'Missing class {class}', [ 'class' => $className ] );
}

…and it works, I now get errors logged with the class name for both cases (as well as an extra PHP Warning: unserialize(): Function wfUnserializeMissingClass() hasn't defined the class it was called for from PHP, but I think that's fine). We might want to do something like this in MediaWiki.

Change #1129220 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Add logging to help figure unserialization issues

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

Change #1129336 had a related patch set uploaded (by Bartosz Dziewoński; author: Michael Große):

[mediawiki/extensions/Echo@wmf/1.44.0-wmf.20] Add logging to help figure unserialization issues

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

@matmarex Thank you for digging up that rabbit hole! I can confirm that with this insight, I can reproduce that warning on 3v4l as well.

I'll see if I can get that change backported tomorrow, so that we know what we are dealing with and how urgent this is.

Change #1129362 had a related patch set uploaded (by Bartosz Dziewoński; author: Michael Große):

[mediawiki/extensions/Echo@wmf/1.44.0-wmf.21] Add logging to help figure unserialization issues

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

Change #1129362 merged by jenkins-bot:

[mediawiki/extensions/Echo@wmf/1.44.0-wmf.21] Add logging to help figure unserialization issues

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

Change #1129336 merged by jenkins-bot:

[mediawiki/extensions/Echo@wmf/1.44.0-wmf.20] Add logging to help figure unserialization issues

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

Mentioned in SAL (#wikimedia-operations) [2025-03-20T08:34:41Z] <tgr@deploy2002> Started scap sync-world: Backport for [[gerrit:1129336|Add logging to help figure unserialization issues (T388725)]], [[gerrit:1129362|Add logging to help figure unserialization issues (T388725)]]

Mentioned in SAL (#wikimedia-operations) [2025-03-20T08:41:14Z] <tgr@deploy2002> matmarex, tgr: Backport for [[gerrit:1129336|Add logging to help figure unserialization issues (T388725)]], [[gerrit:1129362|Add logging to help figure unserialization issues (T388725)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2025-03-20T08:50:46Z] <tgr@deploy2002> Finished scap sync-world: Backport for [[gerrit:1129336|Add logging to help figure unserialization issues (T388725)]], [[gerrit:1129362|Add logging to help figure unserialization issues (T388725)]] (duration: 16m 05s)

Looking at the first warnings I see in logstash, this seems to have been caused by Re-apply "Drop all 49 remaining class_aliases from MediaWiki 1.40" for T166010: The Great Namespaceization Effort. In particular, CommentStoreComment and RawMessage classes seem to be serialized into the extra data of events of type edit-user-talk.

So, if I look at the 4 warning that I see so far

commonswiki: 2 different revisions from 2019
ruwiki: the same revision twice, it is also from 2019

I think this underlines how we should really do either T383948: Echo notification expiration should be time-based, not hardcoded 2000, and/or T325703: Switch Echo serialization format from PHP to JSON and migrate the existing notifications somehow.

And here is the context for why CommentStoreComment got serialized into these events in the first place: Don't add CommentStoreComment as plaintext params and related T223741: Invalid message parameter CommentStoreComment from EventPresentationModel.php.

So this is not urgent in the sense that nothing is broken now that wasn't already broken before, and that only affects a few notifications from ~6 years ago that are still around.

But those are annoying. Can we remove them?

The RemoveInvalidNotification maintenance script seems to have been defacto a one-off.

Now we need to figure out what to actually do here.

I would just restore the class aliases, once we have a fairly complete list, until we can find the time to make one of these two Echo changes you mentioned, even if that is a while from now. It doesn't really cost us anything to have them.

There are also a few entries where the missing class is Flow\Model\UUID, because Flow has been undeployed from the wiki in question. They do not emit the PHP Warning: Class __PHP_Incomplete_Class has no unserializer, because that class is not Serializable, but they emit the new warning we added. That seems like a separate problem, I'll file some tasks.

Change #1130182 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] Restore deprecated aliases for CommentStoreComment and RawMessage

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

I also filed https://github.com/php/php-src/issues/18128 about the confusing warning.

Change #1130182 merged by jenkins-bot:

[mediawiki/core@master] Restore deprecated aliases for CommentStoreComment and RawMessage

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

Change #1130596 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@wmf/1.44.0-wmf.21] Restore deprecated aliases for CommentStoreComment and RawMessage

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

Change #1130596 merged by jenkins-bot:

[mediawiki/core@wmf/1.44.0-wmf.21] Restore deprecated aliases for CommentStoreComment and RawMessage

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

Mentioned in SAL (#wikimedia-operations) [2025-03-25T13:33:57Z] <phuedx@deploy1003> Started scap sync-world: Backport for [[gerrit:1130596|Restore deprecated aliases for CommentStoreComment and RawMessage (T388725)]]

Mentioned in SAL (#wikimedia-operations) [2025-03-25T13:40:33Z] <phuedx@deploy1003> phuedx, matmarex: Backport for [[gerrit:1130596|Restore deprecated aliases for CommentStoreComment and RawMessage (T388725)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2025-03-25T13:49:36Z] <phuedx@deploy1003> Finished scap sync-world: Backport for [[gerrit:1130596|Restore deprecated aliases for CommentStoreComment and RawMessage (T388725)]] (duration: 15m 39s)

Thank you for having taken care of all of this! 🙏