While working on https://gerrit.wikimedia.org/r/773946 (tracked via T277883) we believe we found a potential bug.

In FlaggedRevs::parseStableRevisionPooled() is a PoolCounterWorkViaCallback with doWork, doCachedWork, and fallback functions. The code that executes these functions can be seen in PoolCounterWork::execute(). Fallback is called 2 times there. In both cases the result of the fallback is used as-is when it is anything but false. The current implementation never return false. Instead an empty Status object is returned, which is going to be ignored in FlaggablePageView::showStableVersion() and will show an missing-article error message instead.

That's the wrong error.

See, the fallback is just that, a fallback in case everything else failed. All the fallback does is to try for the last time to find some possibly dirty result in the parser cache to prevent the user being hit by the original error. The fact that the fallback might not be able do this is not the error. It's an expected state that a cache doesn't contain everything. The original error is either that the PoolCounter timed out or the queue was full. This is what's interesting and should be shown.

The correct code path ends with the more correct view-pool-error message instead of missing-article.

A similar issue exists in the doCachedWork function, but that a crazy, probably entirely unreachable edge-case, and doesn't seem like it's easy to fix or even worth it.