fix: guidance for port-conflict errors on 443 / service ports#364
Draft
gtsiolis wants to merge 2 commits into
Draft
fix: guidance for port-conflict errors on 443 / service ports#364gtsiolis wants to merge 2 commits into
gtsiolis wants to merge 2 commits into
Conversation
The extra-port pre-flight check (443 and the 4510-4559 service range) surfaced a bare "LocalStack requires this port" error with no next steps, unlike the primary 4566 check which pointed at the config file. Route both branches through emitPortInUseError so they render the same guidance, and add an OS-aware "Identify the process using it" action (lsof on macOS/Linux, netstat on Windows) so users can find what is holding the port instead of guessing. PRO-363
The "Identify the process using it" hint suggested a plain "lsof -i tcp:<port>", but lsof only lists other users' sockets under sudo — so on a privileged port such as 443 (commonly held by a root-owned process like sshd) the suggested command returns nothing, the exact case that prompted this. Suggest "sudo lsof" for ports below 1024 and keep plain lsof for the 4510-4559 service range. Also correct the extra-ports comment: a running LocalStack is already ruled out by FindRunningByImage upstream, so a conflict here means an unrelated process holds the port, not a second LocalStack instance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
lstk startruns a pre-flight port check before starting the emulator. The primary edge port (4566) conflict path gave the user a next step (reconfigure the port); the extra-ports path (443 and the 4510–4559 service range) emitted a bare title + summary with no actions at all. Bart hit this on 443 (held by an unrelatedsshd) and had no way to tell what was holding the port.This routes both branches through the same
emitPortInUseErrorhelper so they render consistent guidance, and adds an OS-aware "identify the process" hint.Before (extra-port conflict)
After (both 4566 and 443 / service ports)
Details
internal/container/start.go— extra-ports branch now callsemitPortInUseErrorinstead of emitting a bareErrorEvent; the helper gained an "Identify the process using it" action.internal/ports/ports.go— newInspectCommand(port):lsof -i tcp:<port>on macOS/Linux,netstat -ano | findstr :<port>on Windows. Privileged ports (<1024, e.g. 443) suggestsudo lsof, becauselsofonly lists other users' sockets under sudo — without it the hint returns nothing for a root-owned holder likesshdon 443, i.e. the reported case.FindRunningByImage, and Bart's wassshd).inspectCommandacross OS/privileged-port branches; integration testTestStartCommandFailsWhenExtraPortInUse(binds 4510, since 443 is privileged and unbindable in a test).Design choice, consistent with peers: this suggests a command rather than naming the process inline. Docker keeps its hot-path error generic and points to
netstatin docs; ddev put process-naming in a separate opt-inport-diagnosecommand. Inline process-naming was intentionally kept out.Scope / follow-ups
This closes the concrete defect (a guidance-less extra-ports error). It does not yet cover the case where the pre-flight
localhostdial passes but Docker fails to bind at container start — that rawBind for …: port is already allocatedstill surfaces with no guidance and is mis-bucketed asstart_failed. That path (which a privileged-port / non-loopback-bindHost / TOCTOU conflict routes into) is the harder half and is tracked as a follow-up; this PR's green check does not mean that half is done.Refs PRO-363.