timeout does not clean up processes started by 'command'

[thernstig]

Set a command like:

module.exports = {
  command: 'npm run start',
  launchTimeout: 1000,
}

If the timeout occurs, you get this message:

Server has taken more than 1000ms to start.

The problem here is that jest-process-manager then never sends the proper signals to the processes started by the command, meaning they get hanging. As a comparison, pressing ctrl+c after the server has started properly cleans up the resources.

To Reproduce

Add the above config and a very short launchTimeout and notice how the started command never stops (check e.g. via the ps command).

Expected behavior

jest-process-manager should send the proper signals to launched command, even after a launchTimeout occured.

[olee]

Same issue here - trying to start a webpack server but when the timeout is exceeded, jest terminates but webpack keeps on running forever

[mmarkelov]

I know that it was quite old issue. But can someone produce representation? Cause I can't figure out on the problem

[thernstig]

@mmarkelov I am unsure if I read your sentence correctly, but do you mean if we can explain it better?

Try creating a long-running process in command and a short timeout for launchTimeout e.g. 10 ms.
Then do a ps aux | grep command for the command you ran, and you will notice it got stuck hanging and never cleaned up properly.

When launchTimeout occurs, you need to send a SIGTERM followed by SIGKILL (after 10 sec which many tools do) to the PID started by command.

[mmarkelov]

@thernstig what is your OS? I can't reproduce it on my MAC machine

[thernstig]

@mmarkelov maybe this has been fixed since then? I can try again later possibly.

[mmarkelov]

@thernstig it will be nice. Also I added some cleanup functionality in catch block, so I hope it will help

[thernstig]

@mmarkelov I can still re-create this, so I took a look at the code. I believe I found multiple things that can cause errors, unless I am misreading the code.

1. This is what I believe should kill the processes if the waitOn() times out from a normal timeout:

jest-process-manager/src/index.ts

Lines 246 to 257 in 0fd77b1

	try {
	await waitOn(opts)
	} catch (err) {
	const [portProcess] = await findProcess('port', config.port)
	if (portProcess) {
	await killProc(portProcess)
	}
	throw new JestProcessManagerError(
	`Server has taken more than ${launchTimeout}ms to start.`,
	ERROR_TIMEOUT,
	)
	}

Note that this only sends a killProc() to a port. I am not sure why this assumption is taken in the code. Even though a program might wait for a port, the kill signal should be sent to the process that was started by command. E.g. in our case we use npm start with starts several subprocesses through a program called concurrently. One of them is a server we wait for. But the kill signal should be sent to "npm start" and the process it starts, so it in turn can send the signal to concurrently.

2. In the case no port is configured it looks like this:

jest-process-manager/src/index.ts

Lines 258 to 260 in 0fd77b1

	} else {
	runServer(config, index)
	}

and this piece of code seems to properly call treeKill() on the spawned process.

3. And if we come back to what was reported in this issue, it is that when I press ctrl+C (which sends a SIGINT), I cannot see that being handled anywhere in the code and sent to the appropriate process. I cannot see it in either case of config.port being present or not (the if-else branches).

Am I missing something in all of this?

[mmarkelov]

@thernstig So if I understood you correctly , I should check out the cases when port is not provided?

[thernstig]

Point 1 and 3 should be checked out. Point 1) is about a port being provided. Point 3) is about pressing ctrl+c (in both the port and no-port scenarios), then you need to send SIGINT to the processes started.