From daeaffc550f2fcef0af1ca08b30511f66721c7d4 Mon Sep 17 00:00:00 2001
From: Marcin Rataj <lidel@lidel.org>
Date: Mon, 14 Mar 2022 21:57:50 +0100
Subject: [PATCH] fix: proper catch-all daemon startup errors

this should engage isErrored only when ipfs daemon is truly errored:
- checks if pid is alive, and shows "startup error" window when process
  is dead
- removes false-negatives caused by migration code or other "soft"
  errors printed to the console
---
 assets/locales/en.json         |  6 ++---
 src/daemon/daemon.js           | 40 +++++++++++++++++++++++++---------
 src/daemon/index.js            |  4 ++--
 src/daemon/migration-prompt.js |  4 ++--
 4 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/assets/locales/en.json b/assets/locales/en.json
index de8a164ff..8cc7cbe53 100644
--- a/assets/locales/en.json
+++ b/assets/locales/en.json
@@ -231,8 +231,8 @@
     "message": "One moment! IPFS Desktop needs to run the latest data store migrations:",
     "closeAndContinue": "Continue in the background"
   },
-  "migrationFailedDialog": {
-    "title": "IPFS Desktop Migration Has Failed",
-    "message": "IPFS has encountered an error and migration could not be completed:"
+  "startupFailedDialog": {
+    "title": "IPFS Desktop Startup Has Failed",
+    "message": "IPFS node has encountered an error and startup could not be completed:"
   }
 }
diff --git a/src/daemon/daemon.js b/src/daemon/daemon.js
index d94fbe39b..37541071d 100644
--- a/src/daemon/daemon.js
+++ b/src/daemon/daemon.js
@@ -94,24 +94,34 @@ async function startIpfsWithLogs (ipfsd) {
   let isMigrating, isErrored, isFinished
   let logs = ''
 
+  const isSpawnedDaemonDead = (ipfsd) => {
+    if (typeof ipfsd.subprocess === 'undefined') throw new Error('undefined ipfsd.subprocess, unable to reason about startup errors')
+    if (ipfsd.subprocess === null) return false // not spawned yet or remote
+    if (ipfsd.subprocess?.failed) return true // explicit failure
+
+    // detect when spawned ipfsd process is gone/dead
+    // by inspecting its pid - it should be alive
+    const { pid } = ipfsd.subprocess
+    try {
+      // signal 0 throws if process is missing, noop otherwise
+      process.kill(pid, 0)
+      return false
+    } catch (e) {
+      return true
+    }
+  }
+
   const stopListening = listenToIpfsLogs(ipfsd, data => {
     logs += data.toString()
     const line = data.toLowerCase()
     isMigrating = isMigrating || line.includes('migration')
-    isErrored = isErrored || line.includes('error')
+    isErrored = isErrored || isSpawnedDaemonDead(ipfsd)
     isFinished = isFinished || line.includes('daemon is ready')
 
-    if (!isMigrating) {
+    if (!isMigrating && !isErrored) {
       return
     }
 
-    // Undo error state if retrying after HTTP failure
-    // https://github.com/ipfs/ipfs-desktop/issues/2003
-    if (isErrored && line.includes('fetching with ipfs') && !line.includes('error')) {
-      isErrored = false
-      if (migrationPrompt) migrationPrompt.loadWindow(logs, isErrored, isFinished)
-    }
-
     if (!migrationPrompt) {
       logger.info('[daemon] ipfs data store is migrating')
       migrationPrompt = showMigrationPrompt(logs, isErrored, isFinished)
@@ -134,10 +144,20 @@ async function startIpfsWithLogs (ipfsd) {
   } catch (e) {
     err = e
   } finally {
-    // stop monitoring daemon output - we only care about migration phase
+    // stop monitoring daemon output - we only care about startup phase
     stopListening()
+
+    // Show startup error using the same UI as migrations.
+    // This is catch-all that will show stdout/stderr of ipfs daemon
+    // that failed to start, allowing user to self-diagnose or report issue.
+    isErrored = isErrored || isSpawnedDaemonDead(ipfsd)
     if (isErrored) { // save daemon output to error.log
       logger.error(logs)
+      if (migrationPrompt) {
+        migrationPrompt.loadWindow(logs, isErrored, isFinished)
+      } else {
+        showMigrationPrompt(logs, isErrored, isFinished)
+      }
     }
   }
 
diff --git a/src/daemon/index.js b/src/daemon/index.js
index 052fb0ded..7e1c56bcb 100644
--- a/src/daemon/index.js
+++ b/src/daemon/index.js
@@ -53,8 +53,8 @@ module.exports = async function (ctx) {
 
     ipfsd = res.ipfsd
 
-    logger.info(`[daemon] PeerID is ${res.id}`)
-    logger.info(`[daemon] Repo is at ${ipfsd.path}`)
+    logger.info(`[daemon] IPFS_PATH: ${ipfsd.path}`)
+    logger.info(`[daemon] PeerID:    ${res.id}`)
 
     // Update the path if it was blank previously.
     // This way we use the default path when it is
diff --git a/src/daemon/migration-prompt.js b/src/daemon/migration-prompt.js
index 7ee199ab3..9784af9ba 100644
--- a/src/daemon/migration-prompt.js
+++ b/src/daemon/migration-prompt.js
@@ -64,8 +64,8 @@ const inProgressTemplate = (logs, id, done) => {
 }
 
 const errorTemplate = (logs) => {
-  const title = i18n.t('migrationFailedDialog.title')
-  const message = i18n.t('migrationFailedDialog.message')
+  const title = i18n.t('startupFailedDialog.title')
+  const message = i18n.t('startupFailedDialog.message')
   const buttons = [
     `<button class="default" onclick="javascript:window.close()">${i18n.t('close')}</button>`,
     `<button onclick="javascript:openIssue()">${i18n.t('reportTheError')}</button>`