Problem/Motivation
I discovered this while working on #3338666: Add functional test that proves there is reasonable UX whenever a stage event subscriber has an exception. If a PostDestroyEvent subscriber throws an exception, the batch processor redirects you to the UpdateReady form. The first thing that form tries to do is claim the stage, but since the stage has already been destroyed, you get an exception saying "This operation was already canceled." That's no good.
Proposed resolution
If the subscribers of either PreDestroyEvent or PostDestroyEvent throw an exception, redirect to the finish page, which will report the error, and also notify the user that the update completed successfully (or needs to run update.php). In the case of PreDestroyEvent, if the user goes back to the start page and tries to start a new update, they won't be allowed to do it because the previous stage still exists...but at least they'll have the ability to delete it.
Issue fork automatic_updates-3354003
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
phenaproximaI've discovered how we can fix this.
In BatchProcessor::clean(), we should specifically deal with StageEventException carrying PostDestroyEvent. If we catch that, we know the update finished successfully, so rather than recording and re-throwing the exception, we should ensure its message is added to the messenger, then return a RedirectResponse to the updater form (/admin/reports/updates).
Comment #3
tedbowJust clarifying that we need a "Proposed resolution" in the summary before we work on this
Comment #5
phenaproximaComment #6
phenaproximaComment #7
phenaproximaIssue summary is accurate and reflects what I implemented here. I'm happy with it; tests should be passing! I think this is ready for review; assigning to Wim for that.
Comment #8
wim leersZero remarks, looks perfect!
Comment #10
phenaproximaExcellent. Nice to have found this with our new test, and fixed it with the same test!
Comment #12
tedbow