From f2c9cd0c624c450ac751dc347a78b172debd0c1b Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Tue, 3 Aug 2021 16:08:16 +0200
Subject: [PATCH] Don't try stopping if instance is already stopped (#401)

Ignore stop if instance status is already stopped. Fixes instance status
getting stuck at stopping if Factorio exits during startup due to the
stopped status having already been sent at the time stop is invoked.

Fixes #400
---
 packages/slave/slave.js       | 11 +++++------
 test/integration/clusterio.js | 11 +++++++++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/packages/slave/slave.js b/packages/slave/slave.js
index 2767349d..948df556 100755
--- a/packages/slave/slave.js
+++ b/packages/slave/slave.js
@@ -168,8 +168,8 @@ class Instance extends libLink.Link {
 			maxConcurrentCommands: this.config.get("factorio.max_concurrent_commands"),
 		};
 
+		// Valid statuses are stopped, starting, running, stopping, creating_save and exporting_data.
 		this._status = "stopped";
-		this._running = false;
 		this._loadedSave = null;
 		this.server = new libFactorio.FactorioServer(
 			factorioDir, this._dir, serverOptions
@@ -276,7 +276,6 @@ class Instance extends libLink.Link {
 	}
 
 	notifyExit() {
-		this._running = false;
 		this._loadedSave = null;
 		this.notifyStatus("stopped");
 
@@ -577,7 +576,6 @@ class Instance extends libLink.Link {
 		await this.sendSaveListUpdate();
 		await libPlugin.invokeHook(this.plugins, "onStart");
 
-		this._running = true;
 		this.notifyStatus("running");
 	}
 
@@ -602,7 +600,6 @@ class Instance extends libLink.Link {
 
 		await libPlugin.invokeHook(this.plugins, "onStart");
 
-		this._running = true;
 		this.notifyStatus("running");
 	}
 
@@ -683,7 +680,9 @@ class Instance extends libLink.Link {
 	 * Stop the instance
 	 */
 	async stop() {
-		this._running = false;
+		if (this._status === "stopped") {
+			return;
+		}
 		this.notifyStatus("stopping");
 
 		// XXX this needs more thought to it
@@ -703,7 +702,7 @@ class Instance extends libLink.Link {
 
 	async getMetricsRequestHandler() {
 		let results = [];
-		if (this._running) {
+		if (!["stopped", "stopping"].includes(this._status)) {
 			let pluginResults = await libPlugin.invokeHook(this.plugins, "onMetrics");
 			for (let metricIterator of pluginResults) {
 				for await (let metric of metricIterator) {
diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js
index fff1c043..812864eb 100644
--- a/test/integration/clusterio.js
+++ b/test/integration/clusterio.js
@@ -253,6 +253,17 @@ describe("Integration of Clusterio", function() {
 					await execCtl("instance config set 44 factorio.version latest");
 				}
 			});
+			it("should not leave the instance in the stopping state if it fails", async function() {
+				slowTest(this);
+				try {
+					await execCtl("instance config set 44 factorio.game_port 100000");
+					await assert.rejects(execCtl("instance start test"));
+					await checkInstanceStatus(44, "stopped");
+
+				} finally {
+					await execCtl("instance config set 44 factorio.game_port");
+				}
+			});
 			it("starts the given save", async function() {
 				slowTest(this);
 				await execCtl("instance start test --save world.zip");