Repository navigation
fix(services): fall back to ss and netstat when lsof is absent - #10459
diegosouzapw merged 1 commit into
Conversation
resolvePortPid shelled out to lsof alone. On a host without it, spawn raises ENOENT, the error handler turned that into null, and the caller could not tell 'nothing holds this port' from 'I have no way to look' - so a service adopted on a supervisor restart kept pid: null forever, silently, which is the regression the adopt-branch test guards against. Probes lsof, then ss, then netstat, sharing one deadline so the whole lookup still costs at most PID_RESOLVE_TIMEOUT_MS. Output parsing for each is a pure exported function so the formats are unit-testable without the binary being installed. netstat cannot filter by port, so its parser matches the local-address column rather than scanning the line, keeping a foreign address that ends in the same number from being read as a listener.
|
Thanks @aron-intframe — the lsof→ss→netstat probe chain fixes the silent |
972c459
into
cheaper-inference:release/v3.8.50
|
Thanks for the read, and sorry for the slow reply. One thing worth leaving here for whoever touches Your confirming the red CI as inherited was the useful half of the review for us, by the way. We had it in the PR body as a claim about someone else's tree; you turned it into a fact. |
…er-inference#10459) resolvePortPid shelled out to lsof alone. On a host without it, spawn raises ENOENT, the error handler turned that into null, and the caller could not tell 'nothing holds this port' from 'I have no way to look' - so a service adopted on a supervisor restart kept pid: null forever, silently, which is the regression the adopt-branch test guards against. Probes lsof, then ss, then netstat, sharing one deadline so the whole lookup still costs at most PID_RESOLVE_TIMEOUT_MS. Output parsing for each is a pure exported function so the formats are unit-testable without the binary being installed. netstat cannot filter by port, so its parser matches the local-address column rather than scanning the line, keeping a foreign address that ends in the same number from being read as a listener.
…er-inference#10459) resolvePortPid shelled out to lsof alone. On a host without it, spawn raises ENOENT, the error handler turned that into null, and the caller could not tell 'nothing holds this port' from 'I have no way to look' - so a service adopted on a supervisor restart kept pid: null forever, silently, which is the regression the adopt-branch test guards against. Probes lsof, then ss, then netstat, sharing one deadline so the whole lookup still costs at most PID_RESOLVE_TIMEOUT_MS. Output parsing for each is a pure exported function so the formats are unit-testable without the binary being installed. netstat cannot filter by port, so its parser matches the local-address column rather than scanning the line, keeping a foreign address that ends in the same number from being read as a listener.
Closes #10431.
resolvePortPid()only shells out tolsof. No lsof meansspawnraises ENOENT,on("error")turns it intonull, and now the caller can't tell "nothing holds this port" from "I had no way to look" — so a service adopted after a supervisor restart keepspid: nullforever. That's the state the adopt-branch test atServiceSupervisor.test.ts:246exists to prevent.Now it tries
lsof, thenss, thennetstat, sharing one deadline so the whole lookup still costs at mostPID_RESOLVE_TIMEOUT_MSinstead of three times it.The output parsing for each is a pure exported function (
parseLsofPid,parseSsPid,parseNetstatPid) so the formats are testable on a box that doesn't have the binary.netstathas no port filter, so its parser matches the local-address column rather than scanning the line — otherwise a foreign address ending in the same number reads as a listener. There's a test for that row.Validation (Hard Rule #18, TDD)
This box has lsof, so I emulated the reported environment by pointing
PATHat a dir holding onlyssandnetstat. Same script, same host, onlyportProbe.tsswapped:That's now a permanent test (
resolvePortPid still resolves a pid on a host without lsof). It skips rather than passing vacuously if neither fallback is installed, and it asserts lsof is genuinely unreachable on the shimmed PATH first so it can't pass for the wrong reason.Pre-commit ran clean as well (prettier, eslint with suppressions,
check-docs-sync,check:any-budget:t11,check:tracked-artifacts).One pre-existing failure worth disclosing
tests/unit/services/ServiceSupervisor.test.tsis 6 passed / 1 failed here, onadopted service resolves and records the real pid of the process holding the port('error' !== 'running'). It fails the same way on a pristinerelease/v3.8.50tree with my files removed, so it isn't mine — but it's the neighbouring test, so I'd rather say it than have it look like I didn't run it.Environment: Node 22.14.0 (engines want >=22.19.0;
better-sqlite3warns EBADENGINE on it). lsof, ss and netstat are all present here, which is why the fallback had to be exercised through a shimmed PATH rather than by uninstalling something.