From 8c58b129181bfd7df4783338b96494775e62eeb1 Mon Sep 17 00:00:00 2001
From: Wraithan McCarroll <wraithan@particle.io>
Date: Tue, 29 Oct 2019 17:54:38 -0700
Subject: [PATCH] add timeout to  command to catch if device isn't listening

---
 src/cmd/serial.js             | 40 +++++++++++++++++++++++------------
 src/cmd/serial.test.js        | 24 ++++++++++++++++++++-
 test/__mocks__/serial.mock.js |  2 +-
 3 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/src/cmd/serial.js b/src/cmd/serial.js
index f0a3aa87a..7f3ce515d 100644
--- a/src/cmd/serial.js
+++ b/src/cmd/serial.js
@@ -23,7 +23,6 @@ const ensureError = require('../lib/utilities').ensureError;
 const cmd = path.basename(process.argv[1]);
 const arrow = chalk.green('>');
 const alert = chalk.yellow('!');
-const timeoutError = 'Serial timed out';
 
 function protip(){
 	const args = Array.prototype.slice.call(arguments);
@@ -31,6 +30,8 @@ function protip(){
 	console.log.apply(null, args);
 }
 
+const TIMEOUT_ERROR_MESSAGE = 'Serial timed out';
+
 // An LTE device may take up to 18 seconds to power up the modem
 const MODULE_INFO_COMMAND_TIMEOUT = 20000;
 const IDENTIFY_COMMAND_TIMEOUT = 20000;
@@ -616,7 +617,7 @@ module.exports = class SerialCommand {
 				return !!matches;
 			})
 			.catch((err) => {
-				if (err !== timeoutError){
+				if (err !== TIMEOUT_ERROR_MESSAGE){
 					throw err;
 				}
 				return false;
@@ -897,7 +898,7 @@ module.exports = class SerialCommand {
 			}
 
 			function startTimeout(to){
-				self._serialTimeout = setTimeout(() => reject('Serial timed out'), to);
+				self._serialTimeout = setTimeout(() => reject(TIMEOUT_ERROR_MESSAGE), to);
 			}
 
 			function resetTimeout(){
@@ -919,8 +920,22 @@ module.exports = class SerialCommand {
 		return promise.finally(cleanUpFn);
 	}
 
+	/**
+	 * This is a wrapper function created so _serialWifiConfig can return the
+	 * true promise state, but the wrapper can still act like it handled any
+	 * failures gracefully as it acted before testing was attempted.
+	 */
+	serialWifiConfig(...args) {
+		return this._serialWifiConfig.apply(this, args)
+			.then(() => {
+				console.log('Done! Your device should now restart.');
+			}, (err) => {
+				log.error('Something went wrong:', err);
+			});
+	}
+
 	/* eslint-disable max-statements */
-	serialWifiConfig(device, opts = {}){
+	_serialWifiConfig(device, opts = {}){
 		if (!device){
 			return Promise.reject('No serial port available');
 		}
@@ -1204,15 +1219,18 @@ module.exports = class SerialCommand {
 				serialTrigger.start(true);
 				serialPort.write('w');
 				serialPort.drain();
+
+				// In case device is not in listening mode.
+				startTimeout(5000, 'Serial timed out while initially listening to device, please ensure device is in listening mode with particle usb start-listening');
 			});
 
 			function serialClosedEarly(){
 				reject('Serial port closed early');
 			}
 
-			function startTimeout(to){
+			function startTimeout(to, message = TIMEOUT_ERROR_MESSAGE){
 				self._serialTimeout = setTimeout(() => {
-					reject('Serial timed out');
+					reject(message);
 				}, to);
 			}
 
@@ -1275,13 +1293,7 @@ module.exports = class SerialCommand {
 			}
 		});
 
-		return promise
-			.then(() => {
-				console.log('Done! Your device should now restart.');
-			}, (err) => {
-				log.error('Something went wrong:', err);
-			})
-			.finally(cleanUpFn);
+		return promise.finally(cleanUpFn);
 	}
 	/* eslint-enable max-statements */
 
@@ -1307,7 +1319,7 @@ module.exports = class SerialCommand {
 			serialPort.pipe(parser);
 
 			const failTimer = setTimeout(() => {
-				reject(timeoutError);
+				reject(TIMEOUT_ERROR_MESSAGE);
 			}, failDelay);
 
 			parser.on('data', (data) => {
diff --git a/src/cmd/serial.test.js b/src/cmd/serial.test.js
index 2ae8ae568..3a428c191 100644
--- a/src/cmd/serial.test.js
+++ b/src/cmd/serial.test.js
@@ -1,15 +1,23 @@
 const MockSerial = require('../../test/__mocks__/serial.mock');
-const { expect } = require('../../test/setup');
+const { expect, sinon } = require('../../test/setup');
 const SerialCommand = require('./serial');
 
 
 describe('Serial Command', () => {
 	let serial;
+	let clock;
 
 	beforeEach(() => {
 		serial = new SerialCommand({ params: {} });
 	});
 
+	afterEach(() => {
+		if (clock !== undefined) {
+			clock.restore();
+			clock = undefined;
+		}
+	});
+
 	describe('supportsClaimCode', () => {
 		it('can check if a device supports claiming', () => {
 			var device = { port: 'vintage' };
@@ -53,5 +61,19 @@ describe('Serial Command', () => {
 			});
 		});
 	});
+
+	describe('serialWifiConfig', () => {
+		it('can reject with timeout after 5000ms if not getting any serial data', () => {
+			clock = sinon.useFakeTimers();
+			const device = { port: 'baltimore' };
+			const mockSerial = new MockSerial();
+			serial.serialPort = mockSerial;
+			const wifiPromise = serial._serialWifiConfig(device);
+			process.nextTick(() => {
+				clock.tick(5010);
+			});
+			return expect(wifiPromise).to.be.rejected;
+		});
+	});
 });
 
diff --git a/test/__mocks__/serial.mock.js b/test/__mocks__/serial.mock.js
index 380da1385..f7343af1d 100644
--- a/test/__mocks__/serial.mock.js
+++ b/test/__mocks__/serial.mock.js
@@ -15,7 +15,7 @@ MockSerial.prototype.write = function write(chunk) {
 	this.data += chunk;
 };
 
-MockSerial.prototype.drain = function drain(cb) {
+MockSerial.prototype.drain = function drain(cb = () => {}) {
 	this.emit('drain');
 	process.nextTick(cb);
 };