Drush updatedb doesn't show failed updates

Please follow the advice here when posting support requests.

There is no logic to check for errors after the batch has been executed.

In Drupal 7, Drush launches an exception on failed updates. When doing it from update.php, Drupal calls watchdog_exception() so they get logged too. I have added a call to watchdog() so we do it as well.

In Drupal 6 does not throw an exeption and errors are not even logged when using update.php. Still, I added another watchdog() within _drush_log_update_sql().

The reason why I am looking at the watchdog is because updates are executed in a batch process, so we cannot get a return value from it. This has the downside that if dblog module is disabled, we do not knowt about the result of the update, that is why I added a check at the post update hook to warn about this.

Note that I could not use git format-patch for this patch because I have just done it for #1562764: Undefined variable: db_scheme dbtng.inc:109 when using drush_db_select in Drupal 6. Instead I created it using git show HEAD, which I have seen that it formatted the commit message properly.

Here are the steps to reproduce the issue:

1. Unpack the attached module (choose between the version for Drupal 6 or Drupal 7) into sites/all/modules/custom
2. drush en dummy -y
3. Open sites/all/modules/custom/dummy/dummy.install and rename function dummy_update_6000(&$sandbox) by function dummy_update_6001(&$sandbox), so it runs on the next database update.
4. drush updatedb -y -v

The last command should have shown an error as the database update is adding a field that already exists. update.php in Drupal core uses the $_SESSION variable to hand errors to the results page. As we do not have $_SESSION on Drush, I have used the watchdog table instead to look for errors. Drupal 7 calls watchdog when there are are errors during updates, but Drupal 6 does not. However, as Drush takes care of the drush_update_do_one() function, we can control this.

According to #1554732: Updb does not show update hook $ret value, This works in drush 4 without resorting to nastiness like querying watchdog table by timestamp. Lets see if we can get back to where we were with drush4

I ran into this as well when I botched a db_add_field. update.php told me what was wrong; Drush just kept offering the database update every time I ran drush updb.

I tried the D7 version of dummy module as per #3 and it properly reports an error for me. Am trying with latest drush5 from master branch, though I don't think this code has changed in a while. drush_set_error() gets called in drush_update_do_one() and thats why we properly error here. So, please reopen with more instructions about how to reproduce this.

~/htd/d7 (7.x *=)$ dr updb -y -v
Initialized Drupal 7.11-dev root directory at /Users/mweitzman/htd/d7                                                              [notice]
Initialized Drupal site default at sites/default                                                                                   [notice]
The following updates are pending:

block module : 
  7009 - 

dummy module : 
  7001 -   Implements hook_update_N(). 

Do you wish to run all pending updates? (y/n): y
/usr/local/bin/php /Users/mweitzman/c/h/drush/drush.php --php=/usr/local/bin/php  --backend=2 --verbose --yes                      [notice]
--config=.git/../drush/drushrc.php --root=/Users/mweitzman/htd/d7 --uri=http://default updatedb-batch-process 56 56 2>&1
Initialized Drupal 7.11-dev root directory at /Users/mweitzman/htd/d7                                                              [notice]
Initialized Drupal site default at sites/default                                                                                   [notice]
Executing dummy_update_7001                                                                                                        [notice]
Cannot add field <em class="placeholder">dummy</em>.<em class="placeholder">nid</em>: field already exists.                     [error]
Performing dummy_update_7001                                                                                                    [ok]

I decided to checkout drush 5.1 and it does indeed have the bad behavior described here. So now I am even more confident that this bug is already fixed in master branch and will be fixed in 5.2 (coming within a week).

I think this got fixed with #1547432: drush_invoke_process() should default to integrate errors

Yep, it makes total sense. Thanks.