From 4257b26abc61ad24b3ffd3cdd2e2f798499d0742 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 19 May 2020 10:36:43 -0700
Subject: [PATCH] Treat PHP7 "Throwable" exceptions like other unhandled
 "Exception" cases in the worker queue

Summary: See PHI1745. Under PHP7, errors raised as Throwable miss this "generic exception" logic and don't increment their failure count. Instead, treat any "Throwable" we don't recognize like any "Exception" we don't recognize.

Test Plan:
  - Under PHP7, caused a worker task to raise a Throwable (e.g., call to undefined method, see D21270).
  - Ran `bin/worker execute --id ...`.
  - Before: worker failed, but did not increment failure count.
  - After: worker fails and increments failure count as it would for other types of unknown error.

Differential Revision: https://secure.phabricator.com/D21271
---
 .../daemon/workers/storage/PhabricatorWorkerActiveTask.php | 7 +++++++
 .../daemon/workers/storage/PhabricatorWorkerTask.php       | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
index 7139e39ac3..dfb2bf159d 100644
--- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
+++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php
@@ -134,6 +134,7 @@ final class PhabricatorWorkerActiveTask extends PhabricatorWorkerTask {
 
     $did_succeed = false;
     $worker = null;
+    $caught = null;
     try {
       $worker = $this->getWorkerInstance();
       $worker->setCurrentWorkerTask($this);
@@ -180,6 +181,12 @@ final class PhabricatorWorkerActiveTask extends PhabricatorWorkerTask {
 
       $result = $this;
     } catch (Exception $ex) {
+      $caught = $ex;
+    } catch (Throwable $ex) {
+      $caught = $ex;
+    }
+
+    if ($caught) {
       $this->setExecutionException($ex);
       $this->setFailureCount($this->getFailureCount() + 1);
       $this->setFailureTime(time());
diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php
index b0937c4e6d..2fc26503ea 100644
--- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php
+++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php
@@ -34,7 +34,7 @@ abstract class PhabricatorWorkerTask extends PhabricatorWorkerDAO {
     ) + parent::getConfiguration();
   }
 
-  final public function setExecutionException(Exception $execution_exception) {
+  final public function setExecutionException($execution_exception) {
     $this->executionException = $execution_exception;
     return $this;
   }