Merge pull request #1158 from OpenBazaar/hot-fixes

Hot fixes

Author: jjeffryes

js/languages/en_US.json

@@ -35,7 +35,6 @@
     "zcashBinaryLabel": "Zcash Binary",
     "fullNodeLink": "here",
     "zcashBinaryHelper": "A Zcash full node is required. Download a Zcash full node %{fullNodeLink}. Once installed, enter the path to your zcashd binary above.",
-    "zcashBinaryPlaceholder": "/path/to/zcashd",
     "btnBrowse": "Browse",
     "btnNext": "Next"
   },
@@ -1655,7 +1654,7 @@
     "invalidTorProxy": "The value does not appear to be in the right format. It should be in the format ip-address:port, e.g. 127.0.0.1:9150. The port must be a number between 0 and 65535.",
     "provideWalletCurrency": "Please select a wallet currency.",
     "provideZcashBinaryPath": "Please provide a value.",
-    "invalidZcashBinaryPath": "The provided path must be to be a valid exectuable file."
+    "invalidZcashBinaryPath": "The provided path must be to be a file on your system."
   },
   "shippingAddressModelErrors": {
     "provideName": "Please provide a name.",

js/models/ServerConfig.js

@@ -3,7 +3,6 @@ import { remote } from 'electron';
 import LocalStorageSync from '../utils/backboneLocalStorage';
 import is from 'is_js';
 import { getCurrencyByCode as getCryptoCurByCode } from '../data/cryptoCurrencies';
-import { fileModeToPermissions } from '../utils';
 import app from '../app';
 import BaseModel from './BaseModel';
 
@@ -165,7 +164,7 @@ export default class extends BaseModel {
           // pass
         }
 
-        if (!fsStat || !fsStat.isFile() || !fileModeToPermissions(fsStat).execute.owner) {
+        if (!fsStat || !fsStat.isFile()) {
           addError('zcashBinaryPath',
             app.polyglot.t('serverConfigModelErrors.invalidZcashBinaryPath'));
         }

js/start.js

@@ -37,14 +37,15 @@ import './utils/exchangeRateSyncer';
 import './utils/listingData';
 import { launchDebugLogModal, launchSettingsModal } from './utils/modalManager';
 import listingDeleteHandler from './startup/listingDelete';
-import { fixLinuxZoomIssue, handleLinks } from './startup';
+import { fixLinuxZoomIssue, handleLinks, handleServerShutdownRequests } from './startup';
 import ConnectionManagement from './views/modals/connectionManagement/ConnectionManagement';
 import Onboarding from './views/modals/onboarding/Onboarding';
 import WalletSetup from './views/modals/WalletSetup';
 import SearchProvidersCol from './collections/search/SearchProviders';
 import defaultSearchProviders from './data/defaultSearchProviders';
 
 fixLinuxZoomIssue();
+handleServerShutdownRequests();
 
 app.localSettings = new LocalSettings({ id: 1 });
 app.localSettings.fetch().fail(() => app.localSettings.save());
@@ -314,7 +315,7 @@ function fetchStartupData() {
     .fail((jqXhr) => {
       const curConn = getCurrentConnection();
 
-      if (!curConn || curConn.status === 'disconnected') {
+      if (!jqXhr || !curConn || curConn.status !== 'connected') {
         // the connection management modal should be up with relevant info
         console.error('The startup data fetches failed. Looks like the connection to the ' +
           'server was lost.');
@@ -884,12 +885,12 @@ serverConnectEvents.on('connected', (connectedEvent) => {
 ipcRenderer.on('close-attempt', (e) => {
   const localServer = remote.getGlobal('localServer');
 
-  if (localServer.isRunning) {
+  if (localServer && localServer.isRunning) {
     localServer.once('exit', () => e.sender.send('close-confirmed'));
     localServer.stop();
   }
 
-  if (localServer.isStopping) {
+  if (localServer && localServer.isStopping) {
     app.pageNav.navigable = false;
     openSimpleMessage(
       app.polyglot.t('localServerShutdownDialog.title'),

js/startup/index.js

@@ -1,9 +1,11 @@
 // Putting start-up related one offs here that are too small for their own module and
 // aren't appropriate to be in any existing module
 
-import { screen, shell } from 'electron';
+import { screen, shell, ipcRenderer } from 'electron';
+import { platform } from 'os';
 import $ from 'jquery';
 import { getBody } from '../utils/selectors';
+import { getCurrentConnection } from '../utils/serverConnect';
 import app from '../app';
 import Backbone from 'backbone';
 import TorExternalLinkWarning from '../views/modals/TorExternalLinkWarning';
@@ -74,3 +76,39 @@ export function handleLinks() {
     e.preventDefault();
   });
 }
+
+
+/**
+ * This function will accept requests from the main process to shutdown the OB server daemon.
+ * This should only be called on the bundled app on windows. For Linux and OSX, the localServer
+ * module is able to shut down the daemon via OS signals.
+ */
+export function handleServerShutdownRequests() {
+  ipcRenderer.on('server-shutdown', () => {
+    if (platform() !== 'win32') {
+      ipcRenderer.send('server-shutdown-fail',
+        { reason: 'Not on windows. Use childProcess.kill instead.' });
+      return;
+    }
+
+    const curConn = getCurrentConnection();
+
+    if (!curConn || curConn.status !== 'connected') {
+      ipcRenderer.send('server-shutdown-fail',
+        { reason: 'No server connection' });
+      return;
+    }
+
+    try {
+      $.post(app.getServerUrl('ob/shutdown/'))
+        .fail(xhr => ipcRenderer.send('server-shutdown-fail', {
+          xhr,
+          reason: xhr && xhr.responseJSON && xhr.responseJSON.reason || '',
+        }));
+    } catch (e) {
+      ipcRenderer.send('server-shutdown-fail',
+        { reason: e.toString() });
+      return;
+    }
+  });
+}

js/templates/modals/connectionManagement/configurationForm.html

@@ -98,7 +98,7 @@ <h2 class="h3 clrT txUnl txCtr"><%= ob.title %></h2>
         <div class="col9">
           <% if (ob.errors.zcashBinaryPath) print(ob.formErrorTmpl({ errors: ob.errors.zcashBinaryPath })) %>
           <div class="flex gutterH rowSm">
-            <input class="clrBr clrP js-inputZcashBinaryPath" type="text" name="zcashBinaryPath" placeholder="<%= ob.polyT('walletSetup.zcashBinaryPlaceholder') %>" value="<%= ob.zcashBinaryPath %>" data-field-builtin />
+            <input class="clrBr clrP js-inputZcashBinaryPath" type="text" name="zcashBinaryPath" placeholder="<%= ob.zcashBinaryPlaceholder %>" value="<%= ob.zcashBinaryPath %>" data-field-builtin />
             <button class="btn clrP clrBr form js-browseZcashBinary"><%= ob.polyT('walletSetup.btnBrowse') %></button>
           </div>
           <div class="clrT2 tx6">

js/utils/index.js

@@ -202,35 +202,3 @@ export function deparam(queryStr = '') {
 
   return parsed;
 }
-
-// https://github.com/mmalecki/mode-to-permissions/blob/master/lib/mode-to-permissions.js
-export function fileModeToPermissions(fileMode) {
-  let mode = fileMode;
-
-  if (typeof mode === 'object') {
-    // Accept `fs.Stats`.
-    mode = mode.mode;
-  }
-
-  const owner = mode >> 6;
-  const group = (mode << 3) >> 6;
-  const others = (mode << 6) >> 6;
-
-  return {
-    read: {
-      owner: !!(owner & 4),
-      group: !!(group & 4),
-      others: !!(others & 4),
-    },
-    write: {
-      owner: !!(owner & 2),
-      group: !!(group & 2),
-      others: !!(others & 2),
-    },
-    execute: {
-      owner: !!(owner & 1),
-      group: !!(group & 1),
-      others: !!(others & 1),
-    },
-  };
-}

js/utils/localServer.js

@@ -1,3 +1,4 @@
+import { ipcMain } from 'electron';
 import _ from 'underscore';
 import { EOL, platform } from 'os';
 import { Events } from 'backbone';
@@ -18,15 +19,32 @@ export default class LocalServer {
       throw new Error('Please provide an error log path.');
     }
 
+    if (!options.getMainWindow || typeof options.getMainWindow !== 'function') {
+      throw new Error('Please provide a function that, if available, returns a mainWindow ' +
+        'instance.');
+    }
+
     _.extend(this, Events);
     this.serverPath = options.serverPath;
     this.serverFilename = options.serverFilename;
     this.errorLogPath = options.errorLogPath;
+    this._getMainWindow = options.getMainWindow;
     this._isRunning = false;
     this._isStopping = false;
     this._debugLog = '';
     this._lastStartCommandLineArgs = [];
     this.startAfterStop = (...args) => this.start(...args);
+
+    ipcMain.on('server-shutdown-fail', (e, data = {}) => {
+      if (this.isStopping && this.serverSubProcess) {
+        const reasonInsert = data.reason ? ` (${data.reason})` : '';
+        const logMsg = `The server shutdown via api request failed${reasonInsert}. ` +
+          'Will forcibly shutdown.';
+
+        this.log(logMsg);
+        this._forceKill();
+      }
+    });
   }
 
   get isRunning() {
@@ -48,8 +66,8 @@ export default class LocalServer {
   }
 
   start(commandLineArgs = []) {
-    if (this.pendingStop) {
-      this.pendingStop.once('exit', () => this.startAfterStop(commandLineArgs));
+    if (this.isStopping) {
+      this.serverSubProcess.once('exit', () => this.startAfterStop(commandLineArgs));
       const debugInfo = 'Attempt to start server while an existing one' +
         ' is the process of shutting down. Will start after shut down is complete.';
       this.log(debugInfo);
@@ -132,28 +150,46 @@ export default class LocalServer {
     return;
   }
 
+  _forceKill() {
+    if (platform() !== 'win32') {
+      throw new Error('For non windows OSs, use childProcess.kill and pass in a signal.');
+    }
+
+    if (!this.isStopping) {
+      throw new Error('A force kill should only be attempted if you tried stopping via this.stop ' +
+        'and it failed.');
+    }
+
+    if (this.serverSubProcess) {
+      this.log('Forcibly shutting down the server via taskkill.');
+      childProcess.spawn('taskkill', ['/pid', this.serverSubProcess.pid, '/f', '/t']);
+    }
+  }
+
   stop() {
     if (!this.isRunning) return;
 
-    if (this.pendingStop) {
-      this.pendingStop.removeListener('exit', this.startAfterStop);
+    if (this.isStopping) {
+      this.serverSubProcess.removeListener('exit', this.startAfterStop);
       return;
     }
 
     this._isStopping = true;
-    this.pendingStop = this.serverSubProcess;
-    this.pendingStop.once('exit', () => {
-      this.pendingStop = null;
-      this._isStopping = false;
-    });
+    this.serverSubProcess.once('exit', () => (this._isStopping = false));
 
     this.log('Shutting down server');
     console.log('Shutting down server');
 
     if (platform() === 'darwin' || platform() === 'linux') {
       this.serverSubProcess.kill('SIGINT');
     } else {
-      this.childProcess.spawn('taskkill', ['/pid', this.serverSubProcess.pid, '/f', '/t']);
+      const mw = this._getMainWindow();
+
+      if (mw) {
+        mw.webContents.send('server-shutdown');
+      } else {
+        this._forceKill();
+      }
     }
   }
 

js/utils/serverConnect.js

@@ -325,24 +325,50 @@ export default function connect(server, options = {}) {
     if (connectAttempt) connectAttempt.cancel();
   };
 
-  // If we're not connecting to the local bundled server or it's running for a different coin,
-  // then let's ensure it's stopped.
+  if (server.get('builtIn') && !localServer) {
+    // This should never happen to normal users. The only way it would is if you are a dev
+    // and mucking with localStorage and / or fudging the source for the app to masquerade
+    // as a bundled app.
+    throw new Error('A configuration for a built-in server should only be used on ' +
+      ' the bundled app.');
+  }
+
+  const curLocalServerCoin = serverStartArgsToCoin();
+  let curLocalServerZecBinaryPath = null;
+
+  if (localServer) {
+    if (curLocalServerCoin === 'ZEC') {
+      const zecIndex = localServer.lastStartCommandLineArgs
+        .indexOf(serverCurStartArgMap.ZEC);
+
+      if (zecIndex !== -1 &&
+        typeof localServer.lastStartCommandLineArgs[zecIndex + 1] === 'string') {
+        curLocalServerZecBinaryPath = localServer.lastStartCommandLineArgs[zecIndex + 1];
+      }
+    }
+  }
+
+  // If we're not connecting to the local bundled server or it's running with incompatible
+  // command line arguments, let's ensure it's stopped.
   if (localServer && localServer.isRunning &&
-    (!server.get('builtIn') || serverCurrency !== serverStartArgsToCoin())) {
+    (
+      !server.get('builtIn') || serverCurrency !== serverStartArgsToCoin() ||
+      (serverCurrency === 'ZEC' && server.get('zcashBinaryPath') !== curLocalServerZecBinaryPath)
+    )) {
     deferred.notify({ status: 'stopping-local-server' });
     localServer.stop();
 
-    if (server.get('builtIn') && serverCurrency !== serverStartArgsToCoin() &&
-      options.attempts === undefined && options.minAttemptSpacing === undefined &&
+    if (server.get('builtIn') && options.attempts === undefined &&
+      options.minAttemptSpacing === undefined &&
       options.maxAttemptTime === undefined) {
       // If we need to wait for a bundled server to shut down before starting a new instance and
       // the user did not pass in override options regarding connection timeouts, we'll bump up the
       // defaults to give the shutting down server more time (it may take up to a few minutes).
       opts = {
-        attempts: 36, // works out to 3 minutes total
+        ...opts,
+        attempts: 60, // works out to 5 minutes total
         minAttemptSpacing: 5000,
         maxAttemptTime: 5000,
-        ...opts,
       };
     }
   }
@@ -504,14 +530,6 @@ export default function connect(server, options = {}) {
         }
       };
 
-      if (server.get('builtIn') && !localServer) {
-        // This should never happen to normal users. The only way it would is if you are a dev
-        // and mucking with localStorage and / or fudging the source for the app to masquerade
-        // as a bundled app.
-        throw new Error('A configuration for a built-in server should only be used on ' +
-          ' the bundled app.');
-      }
-
       if (server.get('useTor')) {
         innerConnectNotify('setting-tor-proxy');
         innerLog(`Activating a proxy at socks5://${server.get('torProxy')}`);

js/views/modals/WalletSetup.js

@@ -36,8 +36,10 @@ export default class extends BaseModal {
   }
 
   onClickBrowseZcashBinary() {
-    remote.dialog.showOpenDialog({ properties: ['openFile', 'openDirectory'] }, e => {
-      this.getCachedEl('.js-inputZcashBinaryPath').val(e[0] || '');
+    remote.dialog.showOpenDialog({ properties: ['openFile'] }, e => {
+      if (e) {
+        this.getCachedEl('.js-inputZcashBinaryPath').val(e[0] || '');
+      }
     });
   }