From 4df6f88da9f354b566da1ed6ac5a5a3fdac32305 Mon Sep 17 00:00:00 2001
From: Steven Hartland <stevenmhartland@gmail.com>
Date: Fri, 20 Oct 2023 23:00:36 +0100
Subject: [PATCH] fix: process handing

Fix the handling of the CodeNarc process when it's not accessible via
its web API.

We now maintain a handle to the process so if we can't get a response to
the initial ping requests we can still shut it down.

In addition to this correctly handle the timeout in the error case.
---
 .cspell.json           |  4 ++-
 lib/codenarc-caller.js | 56 +++++++++++++++++++++++++++++++++---------
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/.cspell.json b/.cspell.json
index 4adbda65..dd3460e9 100644
--- a/.cspell.json
+++ b/.cspell.json
@@ -281,6 +281,8 @@
     "yessss",
     "zalgo",
     "zpars",
-    "\u00ecnfo"
+    "\u00ecnfo",
+    "mkdocs",
+    "dockerfilelintrc"
   ]
 }
diff --git a/lib/codenarc-caller.js b/lib/codenarc-caller.js
index 533f957b..6aca813c 100644
--- a/lib/codenarc-caller.js
+++ b/lib/codenarc-caller.js
@@ -92,9 +92,10 @@ class CodeNarcCaller {
             response = await axios.request(axiosConfig);
             this.serverStatus = "running";
             const elapsed = parseInt(performance.now() - startCodeNarc, 10);
-            debug(`CodeNarcServer call result: (${response.status}) ${elapsed}ms ${JSON.stringify(response.data || {})}`);
+            debug(`CodeNarcServer call result: (${response.status}) ${elapsed}ms ${JSON.stringify(response.data || {}, null, 2)}`);
         } catch (e) {
             // If server not started , start it and try again
+            debug(`callCodeNarcServer code: ${e.code} error: ${e.message}`);
             if (
                 (startServerTried === false,
                 e.code && ["ECONNREFUSED", "ETIMEDOUT"].includes(e.code) && ["unknown", "running"].includes(this.serverStatus)) // running is here in case the Server auto-killed itself at its expiration time
@@ -272,12 +273,20 @@ class CodeNarcCaller {
             return false;
         }
 
+        // Store the process so we can stop it later.
+        this.codeNarcProcess = javaCallRes.childJavaProcess;
+
         // Poll it until it is ready
         const start = performance.now();
         let notified = false;
         let interval;
         await new Promise(resolve => {
             interval = setInterval(() => {
+                debug(
+                    `pinging CodeNarcServer at ${serverPingUri} notified: ${notified}, serverStatus: ${
+                        this.serverStatus
+                    }, since: ${performance.now() - start}, maxAttemptTimeMs: ${maxAttemptTimeMs}`
+                );
                 axios
                     .get(serverPingUri)
                     .then(response => {
@@ -291,18 +300,22 @@ class CodeNarcCaller {
                                 resolve();
                             }
                         } else if (notified === false && this.serverStatus === "unknown" && performance.now() - start > maxAttemptTimeMs) {
-                            // Timeout has been reached
-                            this.declareServerError(
-                                {
-                                    message: "Timeout after " + maxAttemptTimeMs + "\nResponse: " + JSON.stringify(response.toJSON())
-                                },
-                                interval
-                            );
+                            // Timeout has been reached.
+                            let since = performance.now() - start;
+                            debug(`Ping timeout after ${since}ms status: ${response.status}`);
+                            this.declareServerError({ message: `Timeout after ${since}ms} status: ${response.status}` }, interval);
                             resolve();
                         }
                     })
-                    .catch(() => {
-                        // Just do nothing
+                    .catch(e => {
+                        debug(`Ping code: ${e.code} message: ${e.message}`);
+                        let since = performance.now() - start;
+                        if (notified === false && this.serverStatus === "unknown" && since > maxAttemptTimeMs) {
+                            // Timeout has been reached
+                            debug(`Ping timeout after ${maxAttemptTimeMs}ms`);
+                            this.declareServerError({ message: `Timeout after ${since}ms error: ${e}` }, interval);
+                            resolve();
+                        }
                     });
             }, 400);
         });
@@ -315,8 +328,21 @@ class CodeNarcCaller {
         }
     }
 
+    // Kill CodeNarc process if running.
+    killCodeNarcProcess() {
+        if (this.codeNarcProcess) {
+            this.codeNarcProcess.kill("SIGKILL");
+            delete this.codeNarcProcess;
+            return "CodeNarcServer killed";
+        }
+        return "";
+    }
+
     // Stop polling and log error
     declareServerError(e, interval) {
+        // Kill off the process as it is not responding.
+        this.killCodeNarcProcess();
+
         this.serverStatus = "error";
         if (interval) {
             clearInterval(interval);
@@ -327,10 +353,16 @@ class CodeNarcCaller {
         console.error(c.grey(errMsg));
     }
 
-    // Kill CodeNarc server by telling it to do so
+    // Kill CodeNarc server.
     async killCodeNarcServer() {
+        // Try by process first as it's more reliable.
+        let outputString = this.killCodeNarcProcess();
+        if (outputString) {
+            return outputString;
+        }
+
+        // Process kill wasn't possible, so try sending a kill http request.
         const serverUri = this.getCodeNarcServerUri() + "/kill";
-        let outputString = "";
         try {
             const response = await axios.post(serverUri, { timeout: 5000 });
             if (response.data.status === "killed") {