fix: double pre start (#1437)

* fix: double pre start call License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: initialize event only after repo initializes License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: call callback License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: error message regex License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: typo on intial state name and other fixes License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: simplify License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: remove unused preStart and preStarted state changers License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io> * fix: error message regex License: MIT Signed-off-by: Alan Shaw <alan@tableflip.io>
ipfs · Jul 15, 2018 · e6ad63e · e6ad63e
1 parent b9e6495
commit e6ad63e
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 118 deletions.
diff --git a/src/cli/commands/daemon.js b/src/cli/commands/daemon.js
@@ -35,7 +35,7 @@ module.exports = {
     httpAPI = new HttpAPI(process.env.IPFS_PATH, null, argv)
 
     httpAPI.start((err) => {
-      if (err && err.code === 'ENOENT' && err.message.match(/Uninitalized repo/i)) {
+      if (err && err.code === 'ENOENT' && err.message.match(/uninitialized/i)) {
         print('Error: no initialized ipfs repo found in ' + repoPath)
         print('please run: jsipfs init')
         process.exit(1)

diff --git a/src/core/boot.js b/src/core/boot.js
@@ -1,8 +1,6 @@
 'use strict'
 
 const waterfall = require('async/waterfall')
-const series = require('async/series')
-const extend = require('deep-extend')
 const RepoErrors = require('ipfs-repo').errors
 
 // Boot an IPFS node depending on the options set
@@ -11,110 +9,79 @@ module.exports = (self) => {
   const options = self._options
   const doInit = options.init
   const doStart = options.start
-  const config = options.config
-  const setConfig = config && typeof config === 'object'
-  const repoOpen = !self._repo.closed
 
-  const customInitOptions = typeof options.init === 'object' ? options.init : {}
-  const initOptions = Object.assign({ bits: 2048, pass: self._options.pass }, customInitOptions)
-
-  // Checks if a repo exists, and if so opens it
-  // Will return callback with a bool indicating the existence
-  // of the repo
-  const maybeOpenRepo = (cb) => {
-    // nothing to do
-    if (repoOpen) {
-      return cb(null, true)
-    }
+  // Do the actual boot sequence
+  waterfall([
+    // Checks if a repo exists, and if so opens it
+    // Will return callback with a bool indicating the existence
+    // of the repo
+    (cb) => {
+      // nothing to do
+      if (!self._repo.closed) {
+        return cb(null, true)
+      }
 
-    series([
-      (cb) => self._repo.open(cb),
-      (cb) => self.pin._load(cb),
-      (cb) => self.preStart(cb),
-      (cb) => {
-        self.log('initialized')
-        self.state.initialized()
+      self._repo.open((err, res) => {
+        if (isRepoUninitializedError(err)) return cb(null, false)
+        if (err) return cb(err)
         cb(null, true)
+      })
+    },
+    (repoOpened, cb) => {
+      // Init with existing initialized, opened, repo
+      if (repoOpened) {
+        return self.init({ repo: self._repo }, (err) => cb(err))
       }
-    ], (err, res) => {
-      if (err) {
-        // If the error is that no repo exists,
-        // which happens when the version file is not found
-        // we just want to signal that no repo exist, not
-        // fail the whole process.
 
-        // Use standardized errors as much as possible
-        if (err.code === RepoErrors.ERR_REPO_NOT_INITIALIZED) {
-          return cb(null, false)
-        }
-
-        // TODO: As error codes continue to be standardized, this logic can be phase out;
-        // it is here to maintain compatability
-        if (err.message.match(/not found/) || // indexeddb
-          err.message.match(/ENOENT/) || // fs
-          err.message.match(/No value/) // memory
-        ) {
-          return cb(null, false)
-        }
-        return cb(err)
+      if (doInit) {
+        const initOptions = Object.assign(
+          { bits: 2048, pass: self._options.pass },
+          typeof options.init === 'object' ? options.init : {}
+        )
+        return self.init(initOptions, (err) => cb(err))
       }
-      cb(null, res)
-    })
-  }
 
-  const done = (err) => {
+      cb()
+    },
+    (cb) => {
+      // No problem, we don't have to start the node
+      if (!doStart) {
+        return cb()
+      }
+      self.start(cb)
+    }
+  ], (err) => {
     if (err) {
       return self.emit('error', err)
     }
-    self.log('boot:done')
+    self.log('booted')
     self.emit('ready')
-  }
-
-  const tasks = []
-
-  // check if there as a repo and if so open it
-  maybeOpenRepo((err, hasRepo) => {
-    if (err) {
-      return done(err)
-    }
+  })
+}
 
-    // No repo, but need should init one
-    if (doInit && !hasRepo) {
-      tasks.push((cb) => self.init(initOptions, cb))
-      // we know we will have a repo for all follwing tasks
-      // if the above succeeds
-      hasRepo = true
-    }
+function isRepoUninitializedError (err) {
+  if (!err) {
+    return false
+  }
 
-    // Need to set config
-    if (setConfig) {
-      if (!hasRepo) {
-        console.log('WARNING, trying to set config on uninitialized repo, maybe forgot to set "init: true"')
-      } else {
-        tasks.push((cb) => {
-          waterfall([
-            (cb) => self.config.get(cb),
-            (config, cb) => {
-              extend(config, options.config)
+  // If the error is that no repo exists,
+  // which happens when the version file is not found
+  // we just want to signal that no repo exist, not
+  // fail the whole process.
 
-              self.config.replace(config, cb)
-            }
-          ], cb)
-        })
-      }
-    }
+  // Use standardized errors as much as possible
+  if (err.code === RepoErrors.ERR_REPO_NOT_INITIALIZED) {
+    return true
+  }
 
-    // Need to start up the node
-    if (doStart) {
-      if (!hasRepo) {
-        console.log('WARNING, trying to start ipfs node on uninitialized repo, maybe forgot to set "init: true"')
-        return done(new Error('Uninitalized repo'))
-      } else {
-        tasks.push((cb) => self.start(cb))
-      }
-    }
+  // TODO: As error codes continue to be standardized, this logic can be phase out;
+  // it is here to maintain compatability
+  if (err.message.match(/not found/) || // indexeddb
+    err.message.match(/ENOENT/) || // fs
+    err.message.match(/No value/) // memory
+  ) {
+    return true
+  }
 
-    // Do the actual boot sequence
-    series(tasks, done)
-  })
+  return false
 }
diff --git a/src/core/components/init.js b/src/core/components/init.js
@@ -22,18 +22,31 @@ module.exports = function init (self) {
         return callback(err)
       }
 
-      self.state.initialized()
-      self.emit('init')
-      callback(null, res)
+      self.preStart((err) => {
+        if (err) {
+          self.emit('error', err)
+          return callback(err)
+        }
+
+        self.state.initialized()
+        self.emit('init')
+        callback(null, res)
+      })
     }
 
-    if (self.state.state() !== 'uninitalized') {
+    if (self.state.state() !== 'uninitialized') {
       return done(new Error('Not able to init from state: ' + self.state.state()))
     }
 
     self.state.init()
     self.log('init')
 
+    // An initialized, open repo was passed, use this one!
+    if (opts.repo) {
+      self._repo = opts.repo
+      return done(null, true)
+    }
+
     opts.emptyRepo = opts.emptyRepo || false
     opts.bits = Number(opts.bits) || 2048
     opts.log = opts.log || function () {}

diff --git a/src/core/components/pre-start.js b/src/core/components/pre-start.js
@@ -5,6 +5,7 @@ const PeerInfo = require('peer-info')
 const multiaddr = require('multiaddr')
 const waterfall = require('async/waterfall')
 const Keychain = require('libp2p-keychain')
+const extend = require('deep-extend')
 const NoKeychain = require('./no-keychain')
 /*
  * Load stuff from Repo into memory
@@ -16,6 +17,21 @@ module.exports = function preStart (self) {
     const pass = self._options.pass
     waterfall([
       (cb) => self._repo.config.get(cb),
+      (config, cb) => {
+        if (!self._options.config) {
+          return cb(null, config)
+        }
+
+        extend(config, self._options.config)
+
+        self.config.replace(config, (err) => {
+          if (err) {
+            return cb(err)
+          }
+
+          cb(null, config)
+        })
+      },
       (config, cb) => {
         // Create keychain configuration, if needed.
         if (config.Keychain) {
@@ -78,7 +94,8 @@ module.exports = function preStart (self) {
         }
 
         cb()
-      }
+      },
+      (cb) => self.pin._load(cb)
     ], callback)
   }
 }
diff --git a/src/core/components/start.js b/src/core/components/start.js
@@ -7,8 +7,6 @@ const promisify = require('promisify-es6')
 
 module.exports = (self) => {
   return promisify((callback) => {
-    callback = callback || function noop () {}
-
     const done = (err) => {
       if (err) {
         setImmediate(() => self.emit('error', err))
@@ -21,32 +19,31 @@ module.exports = (self) => {
     }
 
     if (self.state.state() !== 'stopped') {
-      return done(new Error('Not able to start from state: ' + self.state.state()))
+      return done(new Error(`Not able to start from state: ${self.state.state()}`))
     }
 
     self.log('starting')
     self.state.start()
 
     series([
       (cb) => {
+        // The repo may be closed if previously stopped
         self._repo.closed
           ? self._repo.open(cb)
           : cb()
       },
-      (cb) => self.preStart(cb),
-      (cb) => self.libp2p.start(cb)
-    ], (err) => {
-      if (err) { return done(err) }
-
-      self._bitswap = new Bitswap(
-        self._libp2pNode,
-        self._repo.blocks,
-        { statsEnabled: true }
-      )
-
-      self._bitswap.start()
-      self._blockService.setExchange(self._bitswap)
-      done()
-    })
+      (cb) => self.libp2p.start(cb),
+      (cb) => {
+        self._bitswap = new Bitswap(
+          self._libp2pNode,
+          self._repo.blocks,
+          { statsEnabled: true }
+        )
+
+        self._bitswap.start()
+        self._blockService.setExchange(self._bitswap)
+        cb()
+      }
+    ], done)
   })
 }
diff --git a/src/core/state.js b/src/core/state.js
@@ -7,8 +7,8 @@ log.error = debug('jsipfs:state:error')
 const fsm = require('fsm-event')
 
 module.exports = (self) => {
-  const s = fsm('uninitalized', {
-    uninitalized: {
+  const s = fsm('uninitialized', {
+    uninitialized: {
       init: 'initializing',
       initialized: 'stopped'
     },