Skip to content

Don't return catch-all error messages for non-WSL specific error codes#40163

Merged
OneBlue merged 11 commits intofeature/wsl-for-appsfrom
user/oneblue/distro-error
Apr 14, 2026
Merged

Don't return catch-all error messages for non-WSL specific error codes#40163
OneBlue merged 11 commits intofeature/wsl-for-appsfrom
user/oneblue/distro-error

Conversation

@OneBlue
Copy link
Copy Markdown
Collaborator

@OneBlue OneBlue commented Apr 11, 2026

Summary of the Pull Request

This change updates LxssUserSession to explicitly return error messages when hitting user errors on distribution registration and removes the catch-all logic in wslutil.

This will prevent WSL-specific errors from being displayed in WSLC

PR Checklist

  • Closes: Link to issue #xxx
  • 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 11, 2026 00:47
return Localization::MessageNoDefaultDistro();

case HRESULT_FROM_WIN32(WSAECONNABORTED):
case HRESULT_FROM_WIN32(ERROR_SHUTDOWN_IN_PROGRESS):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Open to feedback on this: I chose to remove this because it can be misleading since a closed socket isn't necessarily a sign that the distro is terminating.

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

This PR refines how error messages are surfaced for distribution registration/import failures by moving WSL-specific user-facing messages to the service layer and removing generic Win32→WSL message translations from wslutil, so non-WSL callers (e.g., WSLC) don’t receive misleading WSL-specific text.

Changes:

  • Emit explicit, localized user errors from LxssUserSession for distro name/path conflicts during registration.
  • Remove several Win32 error-code special-cases from wslutil::GetErrorString.
  • Add a new localized “Failed to create disk …” message for VHD creation failures and update/import tests accordingly.

Reviewed changes

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

Show a summary per file
File Description
test/windows/UnitTests.cpp Updates import test expectations and adds a new path-conflict registration scenario
src/windows/service/exe/LxssUserSession.cpp Throws user-facing localized errors for distro name/path conflict cases
src/windows/common/wslutil.cpp Removes Win32→WSL-specific message mappings from GetErrorString
src/windows/common/WslCoreFilesystem.cpp Wraps CreateVirtualDisk failures with a new user-facing localized error
localization/strings/en-US/Resources.resw Adds the localized string used for disk creation failure messaging

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 11, 2026 00:57
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 5 out of 5 changed files in this pull request and generated 4 comments.

OneBlue and others added 5 commits April 13, 2026 11:37
- ImportDistro: Remove 'Failed to create disk' wrapper since the error
  occurs before CreateVhd, producing only the raw system error message
- ModernDistroInstall: Update to raw Win32 message for ERROR_ALREADY_EXISTS
  since the mapping was removed from GetErrorString and InstallDistro
  path doesn't use THROW_HR_WITH_USER_ERROR

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@OneBlue OneBlue marked this pull request as ready for review April 13, 2026 19:39
@OneBlue OneBlue requested a review from a team as a code owner April 13, 2026 19:39
Copilot AI review requested due to automatic review settings April 13, 2026 19:39
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 5 out of 5 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 13, 2026 20:08
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 5 out of 5 changed files in this pull request and generated 1 comment.

@guanwenbin123
Copy link
Copy Markdown

guanwenbin123 commented Apr 13, 2026 via email

Copilot AI review requested due to automatic review settings April 13, 2026 23:05
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 5 out of 5 changed files in this pull request and generated no new comments.

@OneBlue OneBlue merged commit a05c9f8 into feature/wsl-for-apps Apr 14, 2026
11 checks passed
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