Wire errors messages in WSLAContainer#14199
Wire errors messages in WSLAContainer#14199OneBlue wants to merge 5 commits intofeature/wsl-for-appsfrom
Conversation
| THROW_HR_MSG(E_FAIL, _Msg ## ". Error: %hs", __VA_ARGS__, (_Ex).what()); \ | ||
| } | ||
|
|
||
| #define CATCH_THROW_DOCKER_USER_ERROR(_Msg, ...) \ |
There was a problem hiding this comment.
I'm not 100% happy with this macro's name. Open to better ideas for it
There was a problem hiding this comment.
Pull request overview
This PR aims to propagate (“wire”) Docker/runtime error messages back to COM clients for WSLAContainer, so tests and callers can surface user-facing error details instead of generic failures.
Changes:
- Adds Docker exception-to-user-error translation helpers/macros and applies them to several
WSLAContainerImplDocker operations. - Updates
WSLAContainerto support COM error info and introducesCOMServiceExecutionContextusage across COM entrypoints. - Updates process/container launcher APIs and adjusts Windows tests to validate the new error-message behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLATests.cpp | Updates structured bindings and enables/updates assertions for COM error messages. |
| src/windows/wslasession/WSLAContainer.h | Updates Exec signature and adds ISupportErrorInfo / InterfaceSupportsErrorInfo. |
| src/windows/wslasession/WSLAContainer.cpp | Wraps Docker calls to surface user error messages; adds COMServiceExecutionContext; implements InterfaceSupportsErrorInfo. |
| src/windows/wslasession/DockerHTTPClient.h | Introduces macros to map DockerHTTPException into user-facing errors. |
| src/windows/wslaservice/inc/wslaservice.idl | Changes COM interface method signature for IWSLAContainer::Exec. |
| src/windows/common/WSLAProcessLauncher.h | Adjusts LaunchNoThrow return ordering; simplifies Launch() error handling. |
| src/windows/common/WSLAProcessLauncher.cpp | Implements updated LaunchNoThrow tuple shapes and Exec call signature changes. |
| src/windows/common/ExecutionContext.h | Adds a new macro intended to set user error info while throwing. |
| *Errno = -1; | ||
| return CallImpl(&WSLAContainerImpl::Exec, Options, Process, Errno); | ||
| COMServiceExecutionContext context; | ||
|
|
There was a problem hiding this comment.
WSLAContainer::Exec dereferences Process unconditionally (*Process = nullptr) without validating it (and does not validate Options). For COM methods, this should return E_POINTER when callers pass null out/in pointers instead of AV’ing.
| if (Process == nullptr || Options == nullptr) | |
| { | |
| return E_POINTER; | |
| } |
| } \ | ||
| else \ | ||
| { \ | ||
| THROW_HR_MSG(E_FAIL, _Msg##". Error: %hs", __VA_ARGS__, (_Ex).what()); \ |
There was a problem hiding this comment.
The format string construction _Msg##". Error: %hs" uses token-pasting with string literals, which does not produce a valid string literal token and is likely to fail compilation. Prefer normal string-literal concatenation (e.g. _Msg ". Error: %hs") or build the final format string without ##.
| THROW_HR_MSG(E_FAIL, _Msg##". Error: %hs", __VA_ARGS__, (_Ex).what()); \ | |
| THROW_HR_MSG(E_FAIL, _Msg ". Error: %hs", __VA_ARGS__, (_Ex).what()); \ |
| { \ | ||
| ::wsl::windows::common::SetErrorMessage(std::wstring(_messageWide)); \ | ||
| } \ | ||
| THROW_HR_MSG(Result, "%ls"##Format, _messageWide.c_str(), __VA_ARGS__); \ |
There was a problem hiding this comment.
THROW_HR_MSG(Result, "%ls"##Format, ...) uses token pasting between a string literal and Format. Token-pasting does not create a valid string literal here and is likely to fail compilation; use standard adjacent string literal concatenation instead (e.g. "%ls" Format) so the preprocessor produces a single format string.
| THROW_HR_MSG(Result, "%ls"##Format, _messageWide.c_str(), __VA_ARGS__); \ | |
| THROW_HR_MSG(Result, "%ls" Format, _messageWide.c_str(), __VA_ARGS__); \ |
|
|
||
| HRESULT WSLAContainer::InterfaceSupportsErrorInfo(REFIID riid) | ||
| { | ||
| return riid == __uuidof(WSLAContainer) ? S_OK : S_FALSE; |
There was a problem hiding this comment.
InterfaceSupportsErrorInfo should return S_OK for __uuidof(IWSLAContainer) (as WSLASession does for IWSLASession), not for __uuidof(WSLAContainer). As written, COM clients querying error info for the IWSLAContainer interface may not see the expected error details.
| return riid == __uuidof(WSLAContainer) ? S_OK : S_FALSE; | |
| return riid == __uuidof(IWSLAContainer) ? S_OK : S_FALSE; |
1 participant
Summary of the Pull Request
This change wires user error message in WSLAContainer.cpp.
The error management logic is factored in macros so that line information is preserved and we can easily have a single logline with all the debugging information in one place
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed