Repository navigation
Report streaming protocol failures as WinRMClientException - #191
Conversation
Fixes #188. The streaming terminals (WqlRequest.stream(), CommandRequest.start(), RemoteFile.openStream()/openReader(), RemoteDirectoryListing.stream()) and the cursor close let RuntimeExceptions raised in the WSMan layer escape as-is, so an unexpected HTTP status, an exhausted authentication fallback or a malformed response surfaced as a raw IllegalStateException instead of the documented WinRMClientException. LightWinRMService.callStreaming() now wraps every non-typed failure in WinRMException, exactly like the blocking path's executeWithTimeout(), so the request classes translate it; the typed WinRMClientExceptions still pass through. The cursor's close() does the same. The stdin state checks ("input after the end mark") move out of the protocol step into RemoteCommand.checkStdinOpen(), called before it, so that caller bug stays an IllegalStateException. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16788af235
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A handle outliving its client (stream, close the client, advance past the buffered page) hit WsmanClient's closed guard inside the protocol step, which callStreaming now wrapped as a protocol failure. The executor's own checkNotClosed() runs before every streaming step, so the caller bug stays the documented IllegalStateException. Co-Authored-By: Claude Fable 5.1 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
winrm-java/src/main/java/org/metricshub/winrm/light/LightWinRMService.java
Lines 584 to 585 in 0ef6344
Preserve closed-state errors across the concurrent-close race
When a client shared between threads is closed after callStreaming() passes checkNotClosed() but before WsmanClient.request() checks its volatile closed flag, that request deliberately throws the documented closed-client IllegalStateException; this catch then wraps it in WinRMException, so the fluent API nondeterministically exposes WinRMClientException instead. Fresh evidence beyond the resolved sequential test is this time-of-check/time-of-use window: the client is documented as shareable across threads and WsmanClient explicitly supports close() racing a streaming request, so the closed-state exception needs to remain distinguishable inside this catch as well.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Fixes #188.
The streaming terminals (
WqlRequest.stream(),CommandRequest.start(),RemoteFile.openStream()/openReader(),RemoteDirectoryListing.stream()) andRemoteProcess.close()letRuntimeExceptions raised in the WSMan layer escape as-is: an unexpected HTTP status (e.g. a 503 from a proxy), an exhausted authentication fallback or a malformed response surfaced as a rawIllegalStateExceptioninstead of the documentedWinRMClientException.Changes
LightWinRMService.callStreaming()wraps every non-typed failure inWinRMException, exactly like the blocking path'sexecuteWithTimeout(), so the request classes translate it intoWinRMClientExceptionwith the raw failure in the cause chain. The typedWinRMClientExceptions (fault, authentication, timeout) still pass through unchanged. One shared helper, so all four terminals and every cursor step are covered.close()does the same for a failure answering the terminate Signal.RemoteCommand.checkStdinOpen(), called before it, so that caller bug stays anIllegalStateExceptionas documented and tested.FakeWsmanServertests: a 503 answering the Enumerate, the shell Create, and the terminate Signal of an early close.mvn clean verify sitepasses: 325 tests, 0 Checkstyle / PMD / SpotBugs findings.🤖 Generated with Claude Code