From 203f2d2a144f8b284dc66b737c093081ae2aadd4 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 21 May 2020 17:27:42 +0200
Subject: [PATCH] Fix Windows stop logic

Handle the Factorio server not exiting when encountering an error on
Windows and fix the hang detection for .stop().
---
 lib/factorio/server.js     | 74 ++++++++++++++++++++++++++++----------
 test/integration/server.js | 17 +++++++++
 test/lib/factorio/lines.js | 11 ++++++
 3 files changed, 83 insertions(+), 19 deletions(-)

diff --git a/lib/factorio/server.js b/lib/factorio/server.js
index bf31d557..cecbb9e2 100644
--- a/lib/factorio/server.js
+++ b/lib/factorio/server.js
@@ -381,6 +381,17 @@ const outputHeuristics = [
 		},
 	},
 
+	// Message indicating the server has started saving
+	{
+		filter: {
+			type: "log",
+			message: /^Saving game as /,
+		},
+		action: function(output) {
+			this.emit("_saving");
+		},
+	},
+
 	// Message indicating the server has finished saving
 	{
 		filter: {
@@ -405,6 +416,13 @@ const outputHeuristics = [
 		},
 		action: function(output) {
 			this.emit('_quitting');
+
+			// On windows the Factorio server will wait for input on the console input buffer
+			// when an error occurs.  Since we can't send input there we terminate the server
+			// process when this happens.
+			if (process.platform === "win32" && output.message === "Quitting: multiplayer error.") {
+				this._server.kill();
+			}
 		}
 	},
 
@@ -624,9 +642,13 @@ class FactorioServer extends events.EventEmitter {
 		};
 
 		this._rconClient = new rconClient.Rcon(config);
-		this._rconClient.on('error', (err) => this.emit('error', err));
+		this._rconClient.on("error", err => { /* Ignore */ });
 		this._rconClient.on('authenticated', () => { this._rconReady = true; this.emit('rcon-ready'); });
-		this._rconClient.on('end', () => {/* XXX TODO */});
+		this._rconClient.on("end", () => {
+			this._rconClient = null;
+			this._rconReady = false;
+			this._startRcon().catch(() => { /* Ignore */ });
+		});
 		await this._rconClient.connect();
 	}
 
@@ -843,7 +865,7 @@ class FactorioServer extends events.EventEmitter {
 	 * @returns {string} response from server.
 	 */
 	async sendRcon(message, expectEmpty) {
-		this._check(["running"])
+		this._check(["running", "stopping"])
 		if (!this._rconReady) {
 			await events.once(this, 'rcon-ready');
 		}
@@ -864,14 +886,38 @@ class FactorioServer extends events.EventEmitter {
 	async stop() {
 		this._check(["running"]);
 
-		// On Windows we need to save the map as there's no graceful shutdown.
-		if (this._rconClient && process.platform === "win32") {
-			let saved = events.once(this, '_saved');
-			await this.sendRcon("/server-save");
-			await saved;
-		}
+		let hanged = true;
+		function setAlive(output) { hanged = false; }
+
+		let timeoutId = setTimeout(() => {
+			if (hanged) {
+				console.error("Factorio appears to have hanged, sending SIGKILL");
+				if (this._rconClient) {
+					this._rconClient.end().catch(() => {});
+				}
+				this._server.kill("SIGKILL");
+			}
+		}, 5000);
 
 		this._state = "stopping";
+		// On Windows we need to save the map as there's no graceful shutdown.
+		if (this._rconClient && process.platform === "win32") {
+			// Not using events.once here as that will reject on error.
+			let saved = new Promise(resolve => this.once("_saved", resolve));
+			this.on("_saving", setAlive);
+
+			try {
+				await this.sendRcon("/server-save");
+				await saved;
+
+			} catch (err) {
+				// Ignore
+			}
+
+			clearTimeout(timeoutId);
+			this.off("_saving", setAlive);
+		}
+
 		if (this._rconClient) {
 
 			// If RCON is not yet fully connected that operation needs to
@@ -896,17 +942,7 @@ class FactorioServer extends events.EventEmitter {
 		// stop if it's stuck with an infinite lua code loop.  In either
 		// case there's no recovering from it.
 		if (process.platform !== "win32") {
-			let hanged = true;
-			function setAlive(output) { hanged = false; }
 			this.on('_quitting', setAlive);
-
-			let timeoutId = setTimeout(() => {
-				if (hanged) {
-					console.error("Factorio appears to have hanged, sending SIGKILL");
-					this._server.kill('SIGKILL');
-				}
-			}, 5000);
-
 			await events.once(this._server, 'exit');
 
 			clearTimeout(timeoutId);
diff --git a/test/integration/server.js b/test/integration/server.js
index 815fa852..dad51b53 100644
--- a/test/integration/server.js
+++ b/test/integration/server.js
@@ -167,6 +167,23 @@ describe("Integration of lib/factorio/server", function() {
 			});
 		});
 
+		describe(".start() error handling", function() {
+			it("should handle factorio erroring out", async function() {
+				slowTest(this);
+				log(".start() for error handling");
+
+				await server.start("test.zip");
+				if (!server._rconReady) {
+					await events.once(server, "rcon-ready");
+				}
+				server.sendRcon("/c script.on_nth_tick(1, function() o.o = 1 end)").catch(() => {});
+
+				function discard() { }
+				server.on("error", discard);
+				await new Promise(resolve => server.once("exit", resolve));
+				server.off("error", discard);
+			});
+		});
 		describe(".stop() hang detection", function() {
 			it("should detect factorio hanging on shutdown", async function() {
 				slowTest(this);
diff --git a/test/lib/factorio/lines.js b/test/lib/factorio/lines.js
index 2e316ac0..7d34fdd5 100644
--- a/test/lib/factorio/lines.js
+++ b/test/lib/factorio/lines.js
@@ -120,6 +120,17 @@ let testLines = new Map([
 			message: "Saving to _autosave4 (blocking).",
 		}
 	],
+	[
+		"  10.806 Info AppManager.cpp:394: Saving game as C:\\factorio\\saves\\world.zip",
+		{
+			format: "seconds",
+			time: "10.806",
+			type: "log",
+			level: "Info",
+			file: "AppManager.cpp:394",
+			message: "Saving game as C:\\factorio\\saves\\world.zip",
+		}
+	],
 	[
 		"4202.780 Info AppManagerStates.cpp:1802: Saving finished",
 		{