rMWb02bfac06b04: Adding a bunch of hooks from wikiHow into DifferenceEngine looks suspicious. Specifically, the bit where DifferenceEngine::getDiffBody() no longer exits early when there's no old revision unless the new 'DifferenceEngineShowEmptyOldContent' returns false.

In local testing, I can receive a similar fatal with index.php?diff=prev&oldid=2 (where "2" is the first revision of a page), and restoring the early-exit behavior fixes it.

Change 297702 had a related patch set uploaded (by Thcipriani):
Revert "Show parser output for diffs unless extension aborts"

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

Change 297701 had a related patch set uploaded (by Mattflaschen):
Revert "Adding a bunch of hooks from wikiHow into DifferenceEngine"

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

Change 297702 merged by jenkins-bot:
Revert "Show parser output for diffs unless extension aborts"

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

Change 297704 had a related patch set uploaded (by MaxSem):
Revert "Show parser output for diffs unless extension aborts"

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

Change 297701 merged by jenkins-bot:
Revert "Adding a bunch of hooks from wikiHow into DifferenceEngine"

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

Change 297704 merged by jenkins-bot:
Revert "Show parser output for diffs unless extension aborts"

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

Change 297705 had a related patch set uploaded (by MaxSem):
Revert "Adding a bunch of hooks from wikiHow into DifferenceEngine"

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

Change 297705 merged by jenkins-bot:
Revert "Adding a bunch of hooks from wikiHow into DifferenceEngine"

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

Mentioned in SAL [2016-07-06T22:18:28Z] <maxsem@tin> Synchronized php-1.28.0-wmf.9/includes/diff: T139526 (duration: 00m 38s)

@Anomie What was/is/would be needed to trigger the error(s) and reproduce the issue? The actual fatal error mentioned in this ticket's description is somewhat vague, and I wasn't able to reproduce it locally by visiting a URL like index.php?diff=prev&oldid=2 when using a slightly modified version of rMWb02bfac06b04, and neither was @Isarra when she tested the original, unmodified rMWb02bfac06b04.

I would really be interested in fixing whatever issues popped up so that we can restore these hooks ASAP.

See T139435.

The "2" must be the revision id of the first revision of some page, and you need to make sure that nothing hooks 'DifferenceEngineShowEmptyOldContent' (or that whatever does hook it returns true).

So Thanks or Echo are returning true on that, or something? Is that what's going on?

Nope, you've changed code to not abort by default even though some stuff might be not initialized.

Err, not you, of course :)

Wait, if that's a new hook, how is this affecting these extensions at all? Or was it just a simple oversight with related stuff, and the stuff @Anomie noted resolves it?

Also, to clarify - is this why it was reverted, and is the problem in 139435 because of this too, or is that something else? We would very much like to see these hooks available in core, but what all went wrong is a little confusing and we also definitely don't want to go repeating that.

Change 298026 had a related patch set uploaded (by Jack Phoenix):
Adding a bunch of hooks from wikiHow into DifferenceEngine, 2nd try

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

@Anomie @MaxSem Could you take a look at the above patch and try it out, please? The difference to the initial patch, which broke things, is literally two lines. I've tested this newer patch out under MW 1.28alpha (03dfe2c) with Echo (7879f0c) and Thanks (6cd5790) installed, and I was unable to reproduce anything even remotely like T139435. Any and all help would be super. :)

In T139526#2439912, @Isarra wrote:

Wait, if that's a new hook, how is this affecting these extensions at all? Or was it just a simple oversight with related stuff, and the stuff @Anomie noted resolves it?

Because the default behavior (when no extension used the hook) was wrong for a page-creation diff, and caused an exception.

In T139526#2439915, @Isarra wrote:

Also, to clarify - is this why it was reverted, and is the problem in 139435 because of this too, or is that something else? We would very much like to see these hooks available in core, but what all went wrong is a little confusing and we also definitely don't want to go repeating that.

There are two known problems with the original:

• The current task (T139435: edit-thank for new page creation results in a notification that redirects to the Error page. is a symptom of this, hence why I merged it in). All page creation diffs were broken AFAICT.
• T139433: Page content is no longer showing with diffs

I haven't reviewed (yet) (but DifferenceEngineRenderRevisionAddParserOutput looks good). Please make sure you test both those scenarios before +2'ing.

Change 298026 merged by jenkins-bot:
Adding a bunch of hooks from wikiHow into DifferenceEngine, 2nd try

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