Page MenuHomePhabricator

Make permanent worker failures more user-friendly
ClosedPublic

Authored by epriestley on Jul 11 2016, 2:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 16, 9:43 PM
Unknown Object (File)
Wed, Apr 10, 4:02 AM
Unknown Object (File)
Mon, Apr 1, 3:18 AM
Unknown Object (File)
Mon, Apr 1, 3:18 AM
Unknown Object (File)
Mon, Apr 1, 3:18 AM
Unknown Object (File)
Mon, Apr 1, 3:17 AM
Unknown Object (File)
Mar 18 2024, 6:12 PM
Unknown Object (File)
Mar 18 2024, 5:00 PM
Subscribers
None

Details

Summary

Ref T11309. In that task, a user misunderstood two parts of this error:

  • They took "exception" to mean "unexpected failure", when it was intended to mean "rare circumstance".
  • They intereted the internal ID number of a commit to mean that Phabricator was malfunctioning.

Make the language of this condition more direct, explaining what the situation means in greater detail.

Additionally, we would previously re-throw this exception, which would make the daemon exit, wait a moment, and restart. This was normal and expected.

When unexpected failures occur, it's important do to this: it prevents a daemon failing in a loop from causing too many side effects (e.g., limit of 1 email per 5 seconds instead of thousands per second).

When expected, permanent failures occur, we do not need to do this: the task will not be retried. I just did it because it was slightly more consistent ("failures restart daemons") and we had few permanent failure types at the time.

We have more now, and restarting the daemons generates some additional logs which have the potential to confuse. Cycling the daemon also (intentionally) reduces the rate at which we process tasks, which can be bad for permanent failures like "deleted commit" because users can delete a huge number of commits and possibly clog up the queue with cycle-after-failure actions.

Test Plan

Tried to process a deleted commit, saw a new message:

2016-07-11 9:30:22 AM [STDE] <VERB> PhabricatorTaskmasterDaemon Task 1428658 was cancelled: Commit "R55:6c46b7d0fb82a859ca3f87a95dc8dcceef8088c9" (with internal ID "282161") is no longer reachable from any branch, tag, or ref in this repository, so it will not be imported. This usually means that the branch the commit was on was deleted or overwritten.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable