Page MenuHomePhabricator

[INVESTIGATION] Audit edit errors to avoid logging non-critical errors as exceptional
Closed, ResolvedPublic

Description

Until recently we considered any unsuccessful edit an unexpected error (which caused T329233). After this incident we now handle prevented (via abuse filter, spam filter, rate limit) edits (patch) differently, but a look at MediaWikiEditEntity.php, which we use internally, reveals other kinds of unexceptional errors, e.g. edit conflicts.

We need to find out what other kinds of errors we should expect, and come up with a better permanent solution for them.

Time box: 8 hrs

Event Timeline

Ollie.Shotton_WMDE renamed this task from Audit edit errors to avoid logging non-critical errors as exceptional to [INVESTIGATION] Audit edit errors to avoid logging non-critical errors as exceptional.Feb 14 2023, 11:15 AM
Ollie.Shotton_WMDE updated the task description. (Show Details)
Ollie.Shotton_WMDE moved this task from Next to Sprint 23 on the Wikibase Product Platform (v1) board.

test from MediawikiEditEntityTest that result in a non-OK status:

  • testEditConflict -- returns non-OK status
  • testAttemptSaveWithLateConflict -- returns non-OK status
  • testCheckEditPermissions -- covered via MediaWiki REST framework
  • testAttemptSavePermissions -- covered via MediaWiki REST framework
  • testCheckLocalEntityTypes -- always local
  • testAttemptSaveRateLimit -- covered via ItemUpdatePrevented
  • testIsTokenOk -- $token is always false (i.e. not used)
  • testAttemptSaveUnresolvedRedirect -- covered via metadata redirect check

non-OK status updates in MediawikiEditEntity:

  • L677 -- not applicable, since Item is always local
  • L684 -- not applicable, due to if ( $token !== false ... )
  • L688 -- not applicable, since status is still good
  • L692 -- not applicable, since MediaWiki REST framework covers edit permission checks
  • L694 -- rate limits will result in non-OK status
  • L714 -- not applicable, since redirects are covered by our metadata check
  • L734 -- edit conflicts will result in non-OK status
  • L749, L785 -- EntityContentTooBigException will end up in our UnexpectedErrorHandler and cause a 'wikibase-error-entity-too-big' error message in the exception channel

Whether any additional third-party extensions can cause a non-OK status via editFilterHook, I did not cover in this investigation. For now it should be safe to assume those are indeed "unexpected errors" which will end up in our generic error handler.

As a result, in addition to the spam-blocklist, abusefilter and rate limits we need to cover failed save attempts due to:

  • edit conflicts,
  • edits that will exceed the item size limit.

LGTM, thanks for investigating and for the good notes @Silvan_WMDE! \o/