This is a delete action, which does not find the deleted revision any more.

This happens via DomainEvents (after T392205). The latest revision of the deleted event is a cached record and try to refresh itself to avoid steal data, but domain events send after the deletion happens and nothing can be refreshed in that case, the row is deleted.

It seems that MediaWiki-Core-Revision-backend is not responsible here, adding other tags.

Hm, I'm a bit lost at this point in the stack trace:

from	/srv/mediawiki/php-1.45.0-wmf.11/includes/Revision/RevisionStoreCacheRecord.php(114)
	
#0	/srv/mediawiki/php-1.45.0-wmf.11/includes/Revision/RevisionStoreCacheRecord.php(78)	MediaWiki\Revision\RevisionStoreCacheRecord->loadFreshRow()

#1	/srv/mediawiki/php-1.45.0-wmf.11/includes/Revision/RevisionRecord.php(489)	MediaWiki\Revision\RevisionStoreCacheRecord->getVisibility()

Why is getVisibility here causing a load from the database?

The latest revision of the deleted event is a cached record and try to refresh itself to avoid steal data,

Is this the answer? Post transaction (e.g. DeferredUpdates etc.) callbacks do not have access to the original RevisionRecord that was deleted?

Hm, I think I see. After page delete and RevisionDelete (visibility change), attempts to access any potentially sensitive bits of the RevisionRecord will always cause a re-lookup from the database to avoid exposing e.g. comment or content fields. Is this right?

Okay, in this case then, I think we can just avoid attempting to set things like revision comment and visibility settings if we can't access them.

I'm not exactly sure how to detect this without catching the RevisionAccessException though.

Given a RevisionRecord (or specifically in this case, a RevisionStoreCacheRecord), how can I detect that I will not be able to access the comment or visibility settings (via RevisionRecord->isDeleted(...) ) without causing a RevisionAccessException?

cc @daniel

@Ottomata I can't think of a clean solution for this situation off the top of my head. Two thoughts:

• Maybe the exception shouldn't be an exception. RevisionRecords are "conceptually immutable" and should relect the state of the database at the moment the object was created (though lazy loading applies). So if RevisionStoreCacheRecord fails to find the row, perhaps it should simply use the "old" values. That will result in the expected behavior more often than throwing an exception would. I think.
• If you catch the exception and then call getVisibility() again, you should get the old cached data. RevisionStoreCacheRecord only tries the callback once and gives up after that. That wasn't intended for the error case - the idea was to avoid redundant queries on the "happy path". But it should work... at least for now... So "just try once again if it fails the first time" could provide a workable solution. Though it should have a big fat comment pointing to this ticket :)

If you catch the exception and then call getVisibility() again, you should get the old cached data.

Oh, wow. Okay, will do.

Change #1191152 had a related patch set uploaded (by Ottomata; author: Ottomata):

[mediawiki/extensions/EventBus@master] RevisionEntitySerializer - retry call to getVisibility to avoid RevisionAccessException

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

Change #1201113 had a related patch set uploaded (by Ottomata; author: Ottomata):

[mediawiki/core@master] WIP - RevisionStoreCacheRecord - add test about accessing data after failing to load from db

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

@daniel, moving the convo from the patch to this ticket.

I tried adding a test to MW core for this: WIP - RevisionStoreCacheRecord - add test about accessing data after failing to load from db (1201113)

It fails!

In the patch review, I wrote:

The callback provided to RevisionStoreCachedRecord is expected to lookup and return fresh $rev_deleted and UserIdentity. However, in this case, the callback fails looking it up and returns [null, null] for these, which is what triggers the RevisionAccessException in loadFreshRow.

So, if we catch the RevisionAccessException, we will be able to access most parts of the RevisionRecord, but not the $rev_deleted (visibility) bits or the revision's editor UserIdentity.

This means it is not possible to access the Revision's $rev_deleted or User from a RevisionStoreCachedRecord if they cannot be loaded from the db. The RevisionStoreCachedRecord delays even setting those properties until they are accessed, so they aren't actually (ever) 'cached' in memory.

Perhaps...RevisionRecords provided to DomainEvent handlers should not use RevisionStoreCachedRecord?

Or, I understand why you might want User to always be fresh / not accessible if the user is not visible in $rev_deleted. But, why should the $rev_deleted be not accessible? The actual visibility settings of a Revision are not privacy sensitive?

Could RevisionStoreCachedRecord cache $rev_deleted along with other revision properties, but not User?

Change #1238426 had a related patch set uploaded (by Ottomata; author: Ottomata):

[mediawiki/extensions/EventBus@master] RevisionEntitySerializer - avoid RevisionAccessException on page delete

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

Change #1238426 abandoned by Ottomata:

[mediawiki/extensions/EventBus@master] RevisionEntitySerializer - avoid RevisionAccessException on page delete

Reason:

I2861ff3d6e9cac72e222c8fae638bda71a6c2a95 was the original change. this was mistakenly submitted as new Change-Id.

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

So, if we catch the RevisionAccessException, we will be able to access most parts of the RevisionRecord, but not the $rev_deleted (visibility) bits or the revision's editor UserIdentity.

This is wrong. I guess busy from Nov 3 2025 me couldn't read code. Daniel's suggestion works just fine.

"just try once again if it fails the first time" could provide a workable solution.

• okay! RevisionEntitySerializer - avoid RevisionAccessException on page delete (1191152)

• Updated docs and added a MW core test too: RevisionStoreCacheRecordTest - test intentional access of stale data after failing to load fresh (1201113)

Change #1245037 had a related patch set uploaded (by Aaron Schulz; author: Aaron Schulz):

[mediawiki/core@master] page: make WikiPage::loadFromRow() clear mLastRevision from object cache

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

@aaron sweet! Sooooo...what will that do in relation to this bug? :D

Change #1201113 merged by jenkins-bot:

[mediawiki/core@master] RevisionStoreCacheRecordTest - test intentional access of stale data after failing to load fresh

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

Change #1245037 merged by jenkins-bot:

[mediawiki/core@master] page: make WikiPage::loadFromRow() clear revision fields in more cases

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

In T400380#11659513, @Ottomata wrote:

@aaron sweet! Sooooo...what will that do in relation to this bug? :D

If we avoid the RevisionStoreCacheRecord class, which has the lazy-loading, then the bug should not happen in DomainEvent handlers anymore since they can't trigger the lazy-load with the regular RevisionStoreRecord class.

Ah yes, that would be best! TY

The error has disappeared!

I'm going to call this done and abandon my EventBus workaround patch! Thank you @aaron!

Change #1191152 abandoned by Ottomata:

[mediawiki/extensions/EventBus@master] RevisionEntitySerializer - avoid RevisionAccessException on page delete

Reason:

Was fixed upstream in MW core.

https://phabricator.wikimedia.org/T400380#11908367

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