Dont treat suspending of a queue as erroneous

This is about the core Cron runner, which is especially noisy:

          catch (SuspendQueueException $e) {
            // If the worker indicates there is a problem with the whole queue,
            // release the item and skip to the next queue.
            $queue->releaseItem($item);

            watchdog_exception('cron', $e);

Right now the runner treats SuspendQueueException as erroneous, creating a watchdog exception. #2667294: Allow for peaceful requeueing simply changed the logic so RequeueException is caught, which prevented the default \Exception catch from logging as an exception.

I think the change here should remove the watchdog_exception call entirely, and at best log a normal ~debug-level log entry.

I think the change here should remove the watchdog_exception call entirely, and at best log a normal ~debug-level log entry.

That seems reasonable to me.

• Changed to throwing SuspendQueueException no longer logs an error level log and backtrace: logs debug level log and no backtrace/exception.
• Replaced "Broken/Problems/Crash" terms with "Suspend"
• Added logging tests to new and existing cron queue tests.
• Separated cron exception tests for isolation and to test logging reliably; removed use of deprecated functions.
• Turned test queue worker IDs into constants similar to language negotiation plugins; improves DX.

Lets try this.

Ninja CS fixes

Need to refresh some memory on expectations around this make sure it wouldn't break anyone's expectations but this seems to make sense. I think this may be similar to how drush is handling it already.

One thought skimming the patch, the "broken" test seems like it was probably valid just using the wrong exception. Is there another test that covers the handling of broken queues throwing exceptions or was it really only testing the suspend exception?

Is there another test that covers the handling of broken queues throwing exceptions

\Drupal\Tests\system\Kernel\System\CronQueueTest::testUncaughtExceptions makes use of a queue worker (\Drupal\cron_queue_test\Plugin\QueueWorker\CronQueueTestException) dedicated to throwing the generic \Exception.

The // Get the queue to test the specific SuspendQueueException. comment seems to indicate this was just for Suspend.

I think this may be similar to how drush is handling it already.

Right, Drush's current behavior in \Drush\Drupal\Commands\core\QueueCommands is to terminate by throwing an exception, which may end up in stdout. Nothing is logged.

Reroll of #16 against 9.3.x

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Appears the MR has test failures.

Have not yet reviewed.

I'm not sure what the old logs were before (with $this->anything() coming first) but now we just have our log and then the cron run completed notice.

Thanks.

Tagged for a change record to announce this new behavior.

The code changes look good to me so +1 there.

https://www.drupal.org/node/3343512 drafted.

perfect and adding the example really helped!

Makes sense and the changes look good, more of a task than a feature.

Committed ccae3c0 and pushed to 10.1.x. Thanks!

Published change record.