fix: If subprocess failed with fatal error, it should not prevent Map…

…eo from closing.
digidem · Sep 21, 2020 · 2242126 · 2242126
1 parent 257f8f4
commit 2242126
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 37 deletions.
diff --git a/src/main/index.js b/src/main/index.js
@@ -19,13 +19,17 @@ class Main extends events.EventEmitter {
     this.mapeo = new rabbit.Client()
     this.mapPrinter = new rabbit.Client()
 
-    this._findSocket('mapeo', (socket) => {
-      this.mapeoSocket = socket
-      if (this.mapPrinterSocket) this._ready()
-    })
-    this._findSocket('mapPrinter', (socket) => {
-      this.mapPrinterSocket = socket
-      if (this.mapeoSocket) this._ready()
+    this.pid.cleanup((err) => {
+      if (err) logger.debug('No stale processes to clean up')
+      else logger.debug('Successfully removed any stale processes')
+      this._findSocket('mapeo', (socket) => {
+        this.mapeoSocket = socket
+        if (this.mapPrinterSocket) this._ready()
+      })
+      this._findSocket('mapPrinter', (socket) => {
+        this.mapPrinterSocket = socket
+        if (this.mapeoSocket) this._ready()
+      })
     })
   }
 
@@ -54,49 +58,48 @@ class Main extends events.EventEmitter {
     if (!this.connected) this._connect()
     this.pid.create({
       socketName: this.mapeoSocket,
-      filepath: path.join(__dirname, 'background', 'mapeo-core', 'index.js')
+      filepath: path.join(__dirname, '..', 'background', 'mapeo-core', 'index.js')
     }, (err, process) => {
       if (err) logger.error('Failed to start Mapeo Core', err)
-      throw new Error('Failed to start Mapeo Core')
     })
   }
 
   startMapeoHTTPServers (opts, cb) {
     if (!this.connected) this._connect()
-    this.pid.cleanup((err) => {
-      if (err) logger.debug('No stale processes to clean up')
-      else logger.debug('Successfully removed any stale processes')
-      logger.debug('waiting for mapeo listen')
-      this.mapeo.send('listen', opts, (err, osmServerPort) => {
+    logger.debug('waiting for mapeo listen')
+    this.mapeo.send('listen', opts, (err, osmServerPort) => {
+      if (err) return cb(err)
+      logger.debug('got osmServerPort', osmServerPort)
+      this.mapPrinter.send('listen', opts, (err, mapPrinterPort) => {
         if (err) return cb(err)
-        logger.debug('got osmServerPort', osmServerPort)
-        this.mapPrinter.send('listen', opts, (err, mapPrinterPort) => {
-          if (err) return cb(err)
-          logger.debug('got mapPrinterPort', osmServerPort)
-          return cb(null, { osmServerPort, mapPrinterPort })
-        })
+        logger.debug('got mapPrinterPort', osmServerPort)
+        return cb(null, { osmServerPort, mapPrinterPort })
       })
     })
   }
 
   close (cb) {
+    var _close = () => {
+      this.pid.cleanup((err) => {
+        if (err) {
+          this.isDev
+            ? logger.debug('Nothing to clean up')
+            : logger.error('Failed to clean up a child process', err)
+        }
+        logger.debug('Successfully removed any stale processes')
+        cb()
+      })
+    }
+
+    logger.info('pid process', this.pid.process)
+    if (!this.pid.process) return _close()
+
     this.mapeo.send('get-replicating-peers', null, (err, length) => {
       if (err) logger.error('get-replicating-peers on close', err)
 
-      this.mapPrinter.send('close', null, () => {
-        this.mapeo.send('close', null, () => {
-          logger.debug('IPC closed')
-
-          this.pid.cleanup((err) => {
-            if (err) {
-              this.isDev
-                ? logger.debug('Nothing to clean up')
-                : logger.error('Failed to clean up a child process', err)
-            }
-            logger.debug('Successfully removed any stale processes')
-            cb()
-          })
-        })
+      this.mapeo.send('close', null, () => {
+        logger.debug('IPC closed')
+        _close()
       })
     })
   }

diff --git a/src/main/menu.js b/src/main/menu.js
@@ -1,10 +1,10 @@
 const { shell, dialog, app, Menu } = require('electron')
 
-const userConfig = require('./user-config')
 const updater = require('./auto-updater')
 const i18n = require('./i18n')
 const logger = require('../logger')
 const config = require('../../config')
+const userConfig = require('./user-config')
 
 const t = i18n.t
 

diff --git a/src/pid-manager.js b/src/pid-manager.js
@@ -9,9 +9,11 @@ class Worker {
   constructor (userDataPath) {
     this.userDataPath = userDataPath
     this.loc = path.join(userDataPath, 'pid')
+    this.process = null
   }
 
   create ({socketName, filepath}, cb) {
+    logger.debug('Creating subprocess', socketName, filepath)
     this.cleanup((err) => {
       if (err) logger.debug('Not fatal', err)
       logger.debug('Starting background process')
@@ -20,7 +22,17 @@ class Worker {
         socketName,
         this.userDataPath
       ])
-      return cb(null, this.process)
+
+      this.process.on('error', () => {
+        logger.error('Node process error', err)
+        this.error = err
+        this.process = null
+      })
+      this.process.on('exit', (code) => {
+        logger.debug('Node process exited with code', code)
+        this.process = null
+      })
+      return cb(null, process)
     })
   }
 
@@ -35,7 +47,7 @@ class Worker {
   pid (cb) {
     // Write the current pid of this process to the pid file.
     var done = () => {
-      logger.info('writing pid', process.pid)
+      logger.debug('writing pid', process.pid)
       fs.writeFile(this.loc, process.pid.toString(), cb)
     }
     if (this._exists()) this.cleanup(done)