Currently, in D7 and D8 if a queue worker throws an exception it is uncaught. That means the entire PHP process dies. There is an upside to that: It means the item being processed will not be deleted from the queue and can simply picked up by another worker later, after it expires. That is, however, the only upside and easily outweighed by the whole "PHP breaks" downside. (Which has, incidentally, now cost me and my client several hours of swearing because our queue workers talk to outside systems, which is one of the most likely things to break and throw exceptions.)

It's also dead simple to fix, so I don't know why we haven't. The solution? Catch the exception, log it, don't delete.

The attached patch does so. It does not call releaseItem() on the assumption that if there's an exception you probably want to wait a bit before trying again; if a 3rd party service is down, for instance, you want to give it time to come back up before bombarding it with failing connection attempts. Instead, we simply allow the item to expire and get picked up on its own.

I'm pretty sure the same patch will apply to both D7 and D8, but I'm including a version rolled against D7 anyway for completeness. This should get committed to both versions.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lz1irq’s picture

Issue summary: View changes
FileSize
852 bytes

Applies cleanly to commit c47f5c70ef97e1288a72d4ded06ad436be863d3c.

Status: Needs review » Needs work

The last submitted patch, 1: 2021933-queue_exception-1.patch, failed testing.

Anonymous’s picture

yep, i like this patch, waiting_queue does something like this already.

all we need is a test.

Crell’s picture

Status: Needs work » Needs review
FileSize
3.67 KB

Tests it is.

IMO drupal_run_cron() is in desperate need of refactor, given that I had to make a whole new test module just for this, but that's for another issue.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

looks good to me.

nitpick - missing Star Wars/Star Trek quote or reference in error message:

+  throw new Exception('That is not supposed to happen.');
Crell’s picture

I considered that, but in this case it really is the Exception you're looking for. *waves hand*

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

Committed/pushed to 8.x, thanks!

Looks like this could be backported to 7.x.

joachim’s picture

Status: Patch (to be ported) » Needs review
FileSize
3.06 KB
Crell’s picture

Status: Needs review » Reviewed & tested by the community

Boy that looks familiar... :-)

David_Rothstein’s picture

"testFooBar" is an interesting function name, so I changed it to be the same as the Drupal 8 one :) I also removed the D8-specific stuff from elsewhere in the patch. These are minor changes so I'll commit this as long as it passes/fails as expected.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +7.28 release notes

Well, the patch passed tests, but so did the version that was supposed to fail. I guess that's good enough for now, especially since this looks like the same test that's in Drupal 8 (we can investigate any problems there first, then backport).

Committed to 7.x - thanks!

  • Commit 07a910e on 7.x by David_Rothstein:
    Issue #2021933 by Crell, David_Rothstein, joachim, lz1irq: Catch...
David_Rothstein’s picture

Title: Catch exceptions from queue workers » Catch exceptions from queue workers (follow: tests don't work)
Version: 7.x-dev » 8.x-dev
Status: Fixed » Needs review
FileSize
923 bytes

Here's a quick test to see if Drupal 8 has the same problem. If the testbot passes this, then the tests here need work (although this could be moved to a new issue as well).

David_Rothstein’s picture

Title: Catch exceptions from queue workers (follow: tests don't work) » Catch exceptions from queue workers (followup: tests don't work)

Typo fix.

Status: Needs review » Needs work

The last submitted patch, 13: 2021933-followup-tests-12.patch, failed testing.

David_Rothstein’s picture

OK, somehow Drupal 8 is detecting the exception when Drupal 7 isn't (could have something to do with how the test system calls cron.php in each case).

But really the tests should be fixed in Drupal 8 too since they should really have an assertion that is expected to fail (rather than just relying on an exception). I.e. if the tests managed to reach this line, they'd actually pass in Drupal 8 regardless of whether the exception was caught (as described in the original issue, the failing item is still in the queue before the patch also):

    // The item should be left in the queue.
    $this->assertEqual($queue->numberOfItems(), 1, 'Failing item still in the queue after throwing an exception.');
joachim’s picture

Would something along the same lines as this do the trick:

      // Check that the failed rollback was logged.
      $records = db_query("SELECT wid FROM {watchdog} WHERE message LIKE 'Explicit rollback failed%'")->fetchAll();
      $this->assertTrue(count($records) > 0, 'Transactions not supported, and rollback error logged to watchdog.');

(from core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockCreationTest.php)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • catch committed a521147 on 8.3.x
    Issue #2021933 by Crell, lz1irq: Catch exceptions from queue workers.
    

  • catch committed a521147 on 8.3.x
    Issue #2021933 by Crell, lz1irq: Catch exceptions from queue workers.
    

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • catch committed a521147 on 8.4.x
    Issue #2021933 by Crell, lz1irq: Catch exceptions from queue workers.
    

  • catch committed a521147 on 8.4.x
    Issue #2021933 by Crell, lz1irq: Catch exceptions from queue workers.
    

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

The last comment here is by me, and yet I have no idea what this issue is about any more.

> Catch exceptions from queue workers

I'm pretty sure the queue runner does catch exceptions.

Crell’s picture

Assigned: Crell » Unassigned

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Status: Needs work » Fixed
Issue tags: +Bug Smash Initiative

This was committed to both Drupal 8 and Drupal 7.

If test rest needed should happen in a follow-up task.

quietone’s picture

Version: 8.9.x-dev » 8.0.x-dev

Changing version to the version of the Drupal 8 commit.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.