Contrib.social

Failure marker message does not contain exception message + backtrace if available

Open on Drupal.org →
Created on 19 April 2023, about 1 year ago
Updated 31 May 2023, about 1 year ago

Problem/Motivation

While working on #3351895-12: Add Drush command to allow running cron updates via console and by a separate user, for defense-in-depth → I noticed in manual testing I would get the error

"Automatic updates failed to apply, and the site is in an indeterminate state. Consider restoring the code and database from a backup."

My guess is this came from \Drupal\package_manager\StageBase::apply

If we get an error from $this->committer->commit($stage_dir, $active_dir, $paths_to_exclude, NULL, $timeout); we then throw new ApplyFailedException($this, (string) $this->getFailureMarkerMessage(), $throwable->getCode(), $throwable); so the error message ultimately comes from \Drupal\automatic_updates\UpdateStage::getFailureMarkerMessage

but this does not give me much information. It is likely the $throwable, the original exception contains more information I could use if I am not debugging but I think if I am not actively debugging that information is lost.

The reason we added \Drupal\package_manager\StageBase::getFailureMarkerMessage was because we may never return from $this->committer->commit() so having the file with the message tells us something. On the other I think if we do return with exception we could provide the message we get from the throwable. We could also write this information back to the marker file so that if it is detected later \Drupal\package_manager\FailureMarker::assertNotExists we can provide as much information as possible. Additionally if the failure causes the site to not load at all but will still return from $this->committer->commit() if we update the marker file with all the information we have the marker file could be very valuable from figuring out what the error is later.

Steps to reproduce

You could try by manually testing the drush command in 📌 Add Drush command to allow running cron updates via console and by a separate user, for defense-in-depth Fixed this may cause the error.

Proposed resolution

  1. Update \Drupal\package_manager\FailureMarker::write() except an option 3rd param that is a throwable

    If this is provided
    set the 'message' key in the file to equal something like $message . ' Caused by : ' . $throwable->getMessage().
    Also add a new key 'backtrace' => $throwable->getTraceAsString()

  2. In \Drupal\package_manager\StageBase::apply() if get exception from the committer service call
    $this->failureMarker->write($this, $this->getFailureMarkerMessage(), $throwable); before we throw the ApplyFailedException

    The original call before we try to call the committer service of $this->failureMarker->write($this, $this->getFailureMarkerMessage()); would just be for if we do not return at all(server is unplugged?)

Remaining tasks

User interface changes

API changes

Data model changes

📌 Task
Status

Fixed

Version

3.0

Component

Code

Created by

🇺🇸United States tedbow Ithaca, NY, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024