Skip to content

Commit

Permalink
fix: proper catch-all daemon startup errors
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lidel committed Mar 14, 2022
1 parent cc06bcc commit daeaffc
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 17 deletions.
6 changes: 3 additions & 3 deletions assets/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:"
}
}
40 changes: 30 additions & 10 deletions src/daemon/daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/daemon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/daemon/migration-prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
Expand Down

0 comments on commit daeaffc

Please sign in to comment.