Nicer return for quarto call engine julia status when no server is up (instead of error)

[cderv]

This is what I get when no server is running

❯ quarto call engine julia status
ERROR: ConnectionRefused: Aucune connexion n’a pu être établie car l’ordinateur cible l’a expressément refusée. (os error 10061)

Stack trace:
    at async Object.connect (ext:deno_net/01_net.js:640:11)
    at async getReadyServerConnection (file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:78051:16)
    at async _Command.logStatus [as actionHandler] (file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:78366:16)
    at async _Command.execute (file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:9833:7)
    at async _Command.parseCommand (file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:9710:14)
    at async quarto (file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:170932:5)
    at async file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:170960:5
    at async file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:170815:14
    at async mainRunner (file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:170817:5)
    at async file:///C:/Users/chris/scoop/apps/quarto-prerelease/current/bin/quarto.js:170953:3

I am thinking that status should return some nicer outputs instead of an error. I used this command to check that the server was indeed closed.

cc @jkrumbiegel what do you think ?

[jkrumbiegel]

Hm I haven't seen that before, I always get

$ quarto call engine julia status
Julia control server is not running.

[cderv]

Oh interesting! Then I'll spend time debugging to understand. Maybe this is windows problem.

[cderv]

Ok in clean environment I have it

❯ quarto call engine julia status
Julia control server is not running.

It happens on a project while I was testing for #10569 (reply in thread)

With the server hanging...

[cderv]

It happens after I call

quarto call engine julia kill

because

quarto call engine julia stop

was not working.

So I guess calling kill did not cleanup the transportfile

quarto-cli/src/execute/julia.ts

Lines 886 to 890 in 75d1c60

	const transportFile = juliaTransportFile();
	if (!existsSync(transportFile)) {
	info("Julia control server is not running.");
	return;
	}

Should this leftover transportfile case be handled ?

Should it be cleaned there maybe ?

quarto-cli/src/execute/julia.ts

Lines 915 to 924 in 75d1c60

	function killJuliaServer() {
	const transportFile = juliaTransportFile();
	if (!existsSync(transportFile)) {
	info("Julia control server is not running.");
	return;
	}
	const transportOptions = readTransportFile(transportFile);
	Deno.kill(transportOptions.pid, "SIGTERM");
	info("Sent SIGTERM to server process");
	}

[jkrumbiegel]

it should in theory be deleted on the julia side before the server process exits, but that can fail if it doesn't handle the kill signal gracefully. So yeah maybe it should be deleted there, I'm not sure. But for running notebooks there is already code to handle the case where the server from the file can't be connected to, in which case it will delete the transport file and retry, not sure right now why status doesn't take that code path

[cderv]

One think that maybe be related or not but from my Windows user experience; Usually SIGTERM is not supported well, or handled the same as on Unix.

I believe SIGINT is used most often for Windows (or SIGKILL maybe). We do for example listen on SIGINT only in Quarto

quarto-cli/src/core/main.ts

Lines 34 to 37 in 75d1c60

	Deno.addSignalListener("SIGINT", abend);
	if (!isWindows) {
	Deno.addSignalListener("SIGTERM", abend);
	}

Though I believe SIGTERM is still working, but on windows it may be the same as SIGINT.

Anyhow, it seems it works as the process is killed, but could it have impact on what is really happening ?

But for running notebooks there is already code to handle the case where the server from the file can't be connected to, in which case it will delete the transport file and retry, not sure right now why status doesn't take that code path

I believe for running notebook there is a try catch handling that handles it

quarto-cli/src/execute/julia.ts

Lines 526 to 548 in 75d1c60

	const conn = await getReadyServerConnection(transportOptions, options);
	if (typeof conn === "string") {
	// timed out or otherwise not ready
	throw new Error(conn);
	} else {
	return conn;
	}
	} catch (e) {
	if (reused) {
	trace(
	options,
	"Connecting to server failed, a transport file was reused so it might be stale. Delete transport file and retry.",
	);
	safeRemoveSync(juliaTransportFile());
	return await getJuliaServerConnection(options);
	} else {
	error(
	"Connecting to server failed. A transport file was successfully created by the server process, so something in the server process might be broken.",
	);
	throw e;
	}
	}
	}

but not for the status

quarto-cli/src/execute/julia.ts

Lines 897 to 916 in 75d1c60

	const successfullyConnected = typeof conn !== "string";
	if (successfullyConnected) {
	const status: string = await writeJuliaCommand(
	conn,
	{ type: "status", content: {} },
	transportOptions.key,
	{} as JuliaExecuteOptions,
	);
	Deno.stdout.writeSync((new TextEncoder()).encode(status));
	conn.close();
	} else {
	info(`Found transport file but can't connect to control server.`);
	}
	}
	function killJuliaServer() {
	const transportFile = juliaTransportFile();

The logic is different as there is expectation for existing transport file, with possibly no connection

quarto-cli/src/execute/julia.ts

Line 915 in 75d1c60

function killJuliaServer() {

but it assumes that getReadyServerConnection returns a non string value and does not error

quarto-cli/src/execute/julia.ts

Lines 897 to 901 in 75d1c60

	const successfullyConnected = typeof conn !== "string";
	if (successfullyConnected) {
	const status: string = await writeJuliaCommand(
	conn,

getJuliaServerConnection() has a reused logic and will indeed remove and retrying

quarto-cli/src/execute/julia.ts

Lines 534 to 541 in 75d1c60

	if (reused) {
	trace(
	options,
	"Connecting to server failed, a transport file was reused so it might be stale. Delete transport file and retry.",
	);
	safeRemoveSync(juliaTransportFile());
	return await getJuliaServerConnection(options);
	} else {

Should the logics be aligned ?