From 5342bb10734a9f6274d96df4f27e10340570634c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 16 Aug 2012 14:13:24 -0700
Subject: [PATCH] Don't fatal on daemon status updates from `phd`

Summary:
See D3126, T1667, T1658. Prior to D3126, `phd` did not use MySQL directly. Now that it does, there are at least two specific problems (see inline comment).

In the long term, we should probably break this dependency and use Conduit. However, we don't currently have access to the daemon log ID and getting it is a mess (the overseer generates it), and I think I want to rewrite how all this works at some point anyway (the daemon calls are currently completely unauthenticated, which is silly -- we should move them to an authenticated channel at some point, I think).

Test Plan: Ran `phd stop` with a bad MySQL config against a non-running daemon, didn't get a query error.

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1667, T1658

Differential Revision: https://secure.phabricator.com/D3314
---
 .../daemon/PhabricatorDaemonControl.php       | 13 ++-----
 .../control/PhabricatorDaemonReference.php    | 34 ++++++++++++++-----
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/src/infrastructure/daemon/PhabricatorDaemonControl.php b/src/infrastructure/daemon/PhabricatorDaemonControl.php
index bd2cf929a4..a0ca065d0e 100644
--- a/src/infrastructure/daemon/PhabricatorDaemonControl.php
+++ b/src/infrastructure/daemon/PhabricatorDaemonControl.php
@@ -53,12 +53,7 @@ final class PhabricatorDaemonControl {
     foreach ($daemons as $daemon) {
       $name = $daemon->getName();
       if (!$daemon->isRunning()) {
-        $daemon_log = $daemon->loadDaemonLog();
-        if ($daemon_log) {
-          $daemon_log->setStatus(PhabricatorDaemonLog::STATUS_DEAD);
-          $daemon_log->save();
-        }
-
+        $daemon->updateStatus(PhabricatorDaemonLog::STATUS_DEAD);
         $status = 2;
         $name = '<DEAD> '.$name;
       }
@@ -116,11 +111,7 @@ final class PhabricatorDaemonControl {
       if (!$daemon->isRunning()) {
         echo "Daemon is not running.\n";
         unset($running[$key]);
-        $daemon_log = $daemon->loadDaemonLog();
-        if ($daemon_log) {
-          $daemon_log->setStatus(PhabricatorDaemonLog::STATUS_EXITED);
-          $daemon_log->save();
-        }
+        $daemon->updateStatus(PhabricatorDaemonLog::STATUS_EXITED);
       } else {
         posix_kill($pid, SIGINT);
       }
diff --git a/src/infrastructure/daemon/control/PhabricatorDaemonReference.php b/src/infrastructure/daemon/control/PhabricatorDaemonReference.php
index 1af743ac35..c6f82e0a74 100644
--- a/src/infrastructure/daemon/control/PhabricatorDaemonReference.php
+++ b/src/infrastructure/daemon/control/PhabricatorDaemonReference.php
@@ -35,15 +35,33 @@ final class PhabricatorDaemonReference {
     return $ref;
   }
 
-  public function loadDaemonLog() {
-    if (!$this->daemonLog) {
-      $this->daemonLog = id(new PhabricatorDaemonLog())->loadOneWhere(
-        'daemon = %s AND pid = %d AND dateCreated = %d',
-        $this->name,
-        $this->pid,
-        $this->start);
+  public function updateStatus($new_status) {
+    try {
+      if (!$this->daemonLog) {
+        $this->daemonLog = id(new PhabricatorDaemonLog())->loadOneWhere(
+          'daemon = %s AND pid = %d AND dateCreated = %d',
+          $this->name,
+          $this->pid,
+          $this->start);
+      }
+
+      if ($this->daemonLog) {
+        $this->daemonLog
+          ->setStatus($new_status)
+          ->save();
+      }
+    } catch (AphrontQueryException $ex) {
+      // Ignore anything that goes wrong here. We anticipate at least two
+      // specific failure modes:
+      //
+      //   - Upgrade scripts which run `git pull`, then `phd stop`, then
+      //     `bin/storage upgrade` will fail when trying to update the `status`
+      //     column, as it does not exist yet.
+      //   - Daemons running on machines which do not have access to MySQL
+      //     (like an IRC bot) will not be able to load or save the log.
+      //
+      //
     }
-    return $this->daemonLog;
   }
 
   public function getPID() {