Skip to content

Commit

Permalink
fix: process handing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stevenh committed Oct 21, 2023
1 parent cd70bbd commit 4df6f88
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 13 deletions.
4 changes: 3 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@
"yessss",
"zalgo",
"zpars",
"\u00ecnfo"
"\u00ecnfo",
"mkdocs",
"dockerfilelintrc"
]
}
56 changes: 44 additions & 12 deletions lib/codenarc-caller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 => {
Expand All @@ -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);

Check warning on line 306 in lib/codenarc-caller.js

View check run for this annotation

Codecov / codecov/patch

lib/codenarc-caller.js#L304-L306

Added lines #L304 - L306 were not covered by tests
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) {

Check warning on line 313 in lib/codenarc-caller.js

View check run for this annotation

Codecov / codecov/patch

lib/codenarc-caller.js#L311-L313

Added lines #L311 - L313 were not covered by tests
// Timeout has been reached
debug(`Ping timeout after ${maxAttemptTimeMs}ms`);
this.declareServerError({ message: `Timeout after ${since}ms error: ${e}` }, interval);
resolve();

Check warning on line 317 in lib/codenarc-caller.js

View check run for this annotation

Codecov / codecov/patch

lib/codenarc-caller.js#L315-L317

Added lines #L315 - L317 were not covered by tests
}
});
}, 400);
});
Expand All @@ -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";

Check warning on line 336 in lib/codenarc-caller.js

View check run for this annotation

Codecov / codecov/patch

lib/codenarc-caller.js#L334-L336

Added lines #L334 - L336 were not covered by tests
}
return "";
}

// Stop polling and log error
declareServerError(e, interval) {
// Kill off the process as it is not responding.
this.killCodeNarcProcess();

Check warning on line 344 in lib/codenarc-caller.js

View check run for this annotation

Codecov / codecov/patch

lib/codenarc-caller.js#L344

Added line #L344 was not covered by tests

this.serverStatus = "error";
if (interval) {
clearInterval(interval);
Expand All @@ -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;

Check warning on line 361 in lib/codenarc-caller.js

View check run for this annotation

Codecov / codecov/patch

lib/codenarc-caller.js#L361

Added line #L361 was not covered by tests
}

// 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") {
Expand Down

0 comments on commit 4df6f88

Please sign in to comment.