Skip to content

Fix port conflict error message and cleanup on failure#40102

Open
beena352 wants to merge 8 commits intomicrosoft:feature/wsl-for-appsfrom
beena352:user/beenachauhan/fix-port-conflict-error-message
Open

Fix port conflict error message and cleanup on failure#40102
beena352 wants to merge 8 commits intomicrosoft:feature/wsl-for-appsfrom
beena352:user/beenachauhan/fix-port-conflict-error-message

Conversation

@beena352
Copy link
Copy Markdown

@beena352 beena352 commented Apr 3, 2026

Summary of the Pull Request

Fix port conflict error message in wslc container run. When starting a container on a port that is already in use, users previously saw the misleading message “A distribution with the supplied name already exists.” With this fix, the correct error is shown (for example, “Port 8080 is already in use, cannot start container ”). Also clean up the container left behind when Start() fails.

PR Checklist

  • Closes: Link to issue #xxx
  • [ x] Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings April 3, 2026 23:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates WSLC container startup to surface a correct “port already in use” error to users (instead of a misleading “distribution already exists” message) and ensures containers are cleaned up when Start() fails during wslc container run.

Changes:

  • Switch port-conflict handling to THROW_HR_WITH_USER_ERROR(_IF) so a user-facing message can be surfaced.
  • Add error translation around MapPort() to try to convert port-conflict failures into a clearer user error.
  • Fix wslc container run failure cleanup by only disabling auto-delete after a successful Start().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
src/windows/wslcsession/WSLCContainer.cpp Improves port-conflict error surfacing during port allocation/mapping for container open/start.
src/windows/wslc/services/ContainerService.cpp Ensures container run doesn’t leak containers when Start() fails by keeping auto-delete enabled until after a successful start.

Copilot AI review requested due to automatic review settings April 7, 2026 00:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@benhillis
Copy link
Copy Markdown
Member

Hey @beena352 👋 — Following up on this draft PR. There are 2 unresolved review threads:

  • Hardcoded strings that need localization
  • Port reporting inconsistency in host networking mode

Is this change still being worked on? Addressing those review items would help move this forward.

@beena352 beena352 marked this pull request as ready for review April 7, 2026 15:59
@beena352 beena352 requested a review from a team as a code owner April 7, 2026 15:59
Copilot AI review requested due to automatic review settings April 7, 2026 15:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants