Merge pull request #251 from Hornwitser/server-robustness

Improve handling of FactorioServer.stop()

lib/factorio/server.js

@@ -281,6 +281,17 @@ const outputHeuristics = [
 		}
 	},
 
+	// Message indicating the server is shutting down
+	{
+		filter: {
+			type: 'generic',
+			message: /^Quitting: /
+		},
+		action: function(output) {
+			this.emit('_quitting');
+		}
+	},
+
 	// Messages that might be tha cause of an unexpected shutdown.
 	{
 		filter: {
@@ -620,16 +631,14 @@ class FactorioServer extends events.EventEmitter {
 
 		this._state = "stopping";
 		if (this._rconClient) {
-			await this._rconClient.end();
 
-			// Unfortunately version 4.1.0 of rcon-client doesn't send an
-			// end event on .end() and causes an error if the close operation
-			// fails.  Wait a bit here to ensure it's closed before killing
-			// Factorio.  This is only an issue on Windows where Factorio is
-			// terminated without cleanup.
-			await new Promise((resolve, reject) => {
-				setTimeout(resolve, 100);
-			});
+			// If RCON is not yet fully connected that operation needs to
+			// complete before the RCON connection can be closed cleanly.
+			if (!this._rconReady) {
+				await events.once(this, 'rcon-ready');
+			}
+
+			await this._rconClient.end();
 		}
 
 		// On linux this sends SIGTERM to the process.  On windows this
@@ -638,7 +647,34 @@ class FactorioServer extends events.EventEmitter {
 		// developers.  Rcon does not recognize /quit, stdin is not
 		// recognized, and there's no "send CTRL+C to process" on Windows.
 		this._server.kill();
-		await events.once(this._server, 'exit');
+
+		// There appears to be an race condition where sending SIGTERM
+		// immediatly before the RCON interface comes online causes the
+		// Factorio server to hang.  It's also possible the server doesn't
+		// 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);
+			this.off('_quitting', setAlive);
+
+		// On windows the process is terminated immediately, but to keep
+		// ordering wait until after the exit event here.
+		} else {
+			await events.once(this._server, 'exit');
+		}
 	}
 
 	/**

package.json

@@ -58,7 +58,7 @@
     "node-factorio-api": "^0.3.8",
     "node-forge": "^0.7.1",
     "prom-client": "^10.2.3",
-    "rcon-client": "^4.0.4",
+    "rcon-client": "^4.2.0",
     "request": "^2.83.0",
     "sanitizer": "^0.1.3",
     "simple-git": "^1.96.0",

test/integration/server.js

@@ -1,4 +1,5 @@
 const assert = require("assert").strict;
+const events = require("events");
 const fs = require("fs-extra");
 const path = require("path");
 
@@ -165,5 +166,21 @@ describe("Integration of lib/factorio/server", function() {
 				assert(pass, "server did not output line from test scenario");
 			});
 		});
+
+		describe(".stop() hang detection", function() {
+			it("should detect factorio hanging on shutdown", async function() {
+				slowTest(this);
+				log(".start() for hang detection");
+
+				await server.start("test.zip");
+				if (!server._rconReady) {
+					await events.once(server, 'rcon-ready');
+				}
+				server.sendRcon("/c while true do end").catch(() => {});
+				await new Promise((resolve) => setTimeout(resolve, 300));
+				log(".stop() for hang detection");
+				await server.stop();
+			});
+		});
 	});
 });