You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Discuss alternatives for getRelayUptimeSeconds error handling #590
In #585 , getRelayUptimeSeconds was patched to wrap runCommandWithOutput in a try/catch to handle the case where
Docker is not installed. When Docker is missing, spawn emits
an ENOENT error instead of returning a non-zero exit code, which
was causing info --json to crash and break the pre-commit hook.
Problem
The current fix works but is a short-term solution. We should
discuss better long-term alternatives.
Potential Alternatives
Pre-check if Docker is installed before invoking the command
TBD
Goal
Agree on a consistent approach so similar issues don't arise in
other command wrappers.
HI, @cameri@CKodidela, my recommendation is to solve this in the shared command runner (the layer behind runCommandWithOutput) instead of per call site try/catch.
spawn errors like ENOENT (docker not installed) are a different failure path from non-zero exit codes, and today callers need to know that detail. That’s why info --json crashed.
I’d suggest we standardize runCommandWithOutput to return a normalized result/error model for all cases:
Agreed. Normalizing all failure cases in runCommandWithOutput keeps callers clean and prevents the same bug from recurring elsewhere. A discriminated union on the return type would make the contract explicit.
Activity
CKodidela commentedon Apr 28, 2026
@cameri
CKodidela commentedon Apr 29, 2026
@cameri we can discuss on this issue when you are available
cameri commentedon Apr 29, 2026
@Mahmoud-s-Khedr since you worked on the CLI migration, do you mind providing your recommendation here?
Mahmoud-s-Khedr commentedon Apr 29, 2026
HI, @cameri @CKodidela, my recommendation is to solve this in the shared command runner (the layer behind
runCommandWithOutput) instead of per call sitetry/catch.spawnerrors likeENOENT(docker not installed) are a different failure path from non-zero exit codes, and today callers need to know that detail. That’s whyinfo --jsoncrashed.I’d suggest we standardize
runCommandWithOutputto return a normalized result/error model for all cases:ENOENT)EACCES)Then callers only apply feature policy:
null/unknown)This gives us one consistent contract across the CLI and prevents repeating the same class of bug in other wrappers.
What do you think do you this approach?
CKodidela commentedon Apr 30, 2026
Agreed. Normalizing all failure cases in runCommandWithOutput keeps callers clean and prevents the same bug from recurring elsewhere. A discriminated union on the return type would make the contract explicit.
CKodidela commentedon Apr 30, 2026
@cameri i can make a PR if this suggestion is agreed upon
cameri commentedon Apr 30, 2026
Yes, let's move forward with @Mahmoud-s-Khedr recommendation
CKodidela commentedon Apr 30, 2026
Should I work on this?
If yes can you assign it.
cameri commentedon Apr 30, 2026
Assigned to you
Mahmoud-s-Khedr commentedon Apr 30, 2026
@CKodidela feel free to tag me if you need any help with this.
CKodidela commentedon Apr 30, 2026
Sure