Fix TransactionSquasher crashes
Open, Needs TriagePublic
Actions

Assigned To

Authored By

	matmarex
	Mar 31 2023, 4:43 PM

Description

In T333612#8746101, @matmarex wrote:

Likely related to these errors: https://logstash.wikimedia.org/goto/250706a6d2eba3b5f62559b1d35bb31c

In T333612#8746277, @matmarex wrote:

A bit more details: the issue was caused by code we deployed this week which was supposed to examine the changes made before saving, and record some data for the Edit Check project. Unfortunately it apparently fails on some kinds of changes (not all edits, but a significant number). In the en.wp VPT discussion someone suggested in happens when editing templates, but I couldn't reproduce the problem yet. For now I just disabled this code, and we will investigate it next week. Sorry about this.

There are two kinds of errors logged:

"Unexpected prior attribute value"

at ve.dm.TransactionSquasher.changeElement
at ve.dm.TransactionSquasher.processRemove
at ve.dm.TransactionSquasher.squashIn
at ve.dm.TransactionSquasher.static.squash
at ve.dm.Change.squash
at mw.editcheck.doesAddedContentNeedReference

at ve.dm.TransactionSquasher.changeElement
at ve.dm.TransactionSquasher.processAttribute
at ve.dm.TransactionSquasher.squashIn
at ve.dm.TransactionSquasher.static.squash
at ve.dm.Change.squash
at mw.editcheck.doesAddedContentNeedReference

at ve.dm.TransactionSquasher.processAttribute
at ve.dm.TransactionSquasher.squashIn
at ve.dm.TransactionSquasher.static.squash
at ve.dm.Change.squash
at mw.editcheck.doesAddedContentNeedReference

"Remove does not match insert"

at ve.dm.TransactionSquasher.processRemove
at ve.dm.TransactionSquasher.squashIn
at ve.dm.TransactionSquasher.static.squash
at ve.dm.Change.squash
at mw.editcheck.doesAddedContentNeedReference

Details

Subject	Repo	Branch	Lines +/-
Edit check: Guard against completeHistory being empty	mediawiki/extensions/VisualEditor	master	+3 -0
Log TransactionSquasher errors, in case any still occur	mediawiki/extensions/VisualEditor	master	+2 -1
Update VE core submodule to master (da7624d69)	mediawiki/extensions/VisualEditor	master	+1 -1
Allow attribute values that are reference types to compare equal	VisualEditor/VisualEditor	master	+40 -13

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Open		None	T265163 Create a system to encode best practices into editing experiences
In Progress		Trizek-WMF	T331946 [RELEASE TICKET] Make Edit Check (references) available to all newcomers at all Wikipedias
Open		None	T331947 [RELEASE TICKET] Make Edit Check MVP available to all newcomers at Partner Wikipedias
Resolved		ppelberg	T338907 [RELEASE TICKET] Make Edit Check MVP available as Beta Feature (all wikis)
Resolved		None	T332646 [Deployment] Make Edit Check MVP (mobile + desktop) available for testing at test.wikipedia.org
Resolved		ppelberg	T328944 Build Edit Check MVP (references)
Resolved		None	T324733 Introduce a tag to identify edits that meet the Edit Check heuristic
Open		dchan	T333710 Fix TransactionSquasher crashes
Resolved	BUG REPORT	matmarex	T333612 Visual Edits do not save
Resolved	BUG REPORT	dchan	T334677 Internal copy/paste can cause an unbalanced transaction

Event Timeline

matmarex created this task.Mar 31 2023, 4:43 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 31 2023, 4:43 PM

matmarex added a subtask: T333612: Visual Edits do not save.Mar 31 2023, 4:44 PM

matmarex added a parent task: T324733: Introduce a tag to identify edits that meet the Edit Check heuristic .

matmarex updated the task description. (Show Details)Mar 31 2023, 4:47 PM

matmarex added a project: Editing-team (Kanban Board).

matmarex moved this task from To Triage to Triaged on the VisualEditor board.

Esanders added a subscriber: dchan.Mar 31 2023, 8:39 PM

matmarex closed subtask T333612: Visual Edits do not save as Resolved.Apr 3 2023, 6:00 PM

matmarex updated the task description. (Show Details)Apr 11 2023, 3:50 PM

Change 908230 had a related patch set uploaded (by Divec; author: Divec):

[VisualEditor/VisualEditor@master] Allow attribute values that are reference types to compare equal

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

gerritbot added a project: Patch-For-Review.Apr 12 2023, 1:27 PM

The patch set above fixes the "Unexpected prior attribute value" errors.

Could someone with logstash access provide me with an example change URL for which the "Remove does not match insert" error happened?

After investigating, I think the "Remove does not match insert" errors are caused by actual invalid transactions: see T334677 .

dchan claimed this task.Apr 22 2023, 3:15 AM

matmarex added a subtask: T334677: Internal copy/paste can cause an unbalanced transaction.Apr 22 2023, 8:08 AM

Change 908230 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Allow attribute values that are reference types to compare equal

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

Maintenance_bot removed a project: Patch-For-Review.Apr 22 2023, 8:11 AM

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

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (da7624d69)

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

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

[mediawiki/extensions/VisualEditor@master] Log TransactionSquasher errors, in case any still occur

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

Change 910809 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (da7624d69)

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

matmarex closed subtask T334677: Internal copy/paste can cause an unbalanced transaction as Resolved.Apr 22 2023, 8:43 AM

Change 910810 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Log TransactionSquasher errors, in case any still occur

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

Maintenance_bot removed a project: Patch-For-Review.Apr 27 2023, 3:30 PM

Check logs after deployment

matmarex moved this task from Blocked / Needs More Work to Ready to Be Worked On on the Editing-team (Kanban Board) board.May 8 2023, 5:21 PM

There have been a few new entries, 26 in total over the last week, with error messages:

"Cannot read properties of undefined (reading 'operations')" (6 times)
"Remove does not match insert" (20 times)

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.9; 2023-05-15).May 16 2023, 12:02 AM

matmarex mentioned this in T340145: Remove does not match insert.Jun 23 2023, 8:50 PM

matmarex merged a task: T340145: Remove does not match insert.

matmarex added a subscriber: Jdlrobson.

@dchan: can you tell if there is anything else to be done to decrease the occurrence of these errors?

There's more now -- 607 over the last 7 days. Mostly "Remove does not match insert", but occasional "Cannot read properties of undefined (reading 'operations')".

Change 949045 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] Edit check: Guard against completeHistory being empty

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

gerritbot added a project: Patch-For-Review.Aug 15 2023, 1:47 PM

The above patch should fix the Cannot read properties of undefined (reading 'operations') errors.

Change 949045 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Edit check: Guard against completeHistory being empty

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

Maintenance_bot removed a project: Patch-For-Review.Aug 16 2023, 3:30 PM

ppelberg moved this task from Ready to Be Worked On to Code Review on the Editing-team (Kanban Board) board.Aug 22 2023, 9:37 PM

ppelberg added a project: Goal.

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.25; 2023-09-05); removed MW-1.41-notes (1.41.0-wmf.9; 2023-05-15).Aug 29 2023, 6:10 PM