fix: Timed-out Unity process lookups on Windows now report a timeout instead of a bare exit status - #3029
Conversation
When PowerShell (Get-CimInstance) outlived the 10 s process-list limit, the kill left only "failed to retrieve Unity process list on Windows: exit status 1", so a PowerShell or WMI that was slow to start and a failing script looked identical (#2954). - The error now says the listing timed out after the limit and that PowerShell or WMI did not respond. It says so only while the caller's context is alive: a caller whose own earlier deadline or cancellation stopped the command reports that itself. - A script that fails on its own now carries PowerShell's stderr. - The 10 s limit stays: with the timeout named, a slow cold start costs one retry, while a longer limit would lengthen every genuine WMI stall. - The command run moves into runProcessListCommandWithin, which takes the timeout as a parameter like the focus helpers do, so tests reach the timeout without waiting. exitErrorStderr moves from the darwin-only focus file to the shared command_error.go so the Windows path can use it. The focus helpers already map their timeouts. - Shared release input stamps are refreshed for the cli/common change.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughWindows process listing now distinguishes its own timeout from caller cancellation. Command failures include captured stderr. Shared-input stamps were updated. ChangesWindows process-list error handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change appears mergeable after normal checks. Windows PowerShell execution still needs platform validation, but no concrete failure is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
exit status 1.User Impact
uloop launch) could fail with onlyfailed to retrieve Unity process list on Windows: exit status 1. A slow PowerShell or WMI start and a genuinely failing script looked identical, so there was no way to tell whether to wait, retry, or investigate.listing Unity processes on Windows timed out after 10s; PowerShell or WMI did not respond, retry the command. A lookup whose script fails on its own now includes PowerShell's error output.Changes
cli/common. release-please therefore attributes it to both the dispatcher and the project runner.Verification
cli/common/unityprocesscover the timeout message, a caller deadline that must not be reported as the list timeout, stderr on script failure, and successful output. They were written first and failed against the old behavior, except for the two guard cases. Dropping the caller-deadline guard makes its test fail.sleep/sh) through the same helper on macOS. The real PowerShell path on Windows was not run locally.gofmt,go vet(also withGOOS=windowsandGOOS=linux),golangci-lint,go testfor all four Go modules, cross-builds of the dispatcher and project runner for windows/linux/darwin,check-release-triggers, a stamp drift check, andscripts/check-file-length.sh.ipcendpoint(mkdir under/tmp) andprojectrunner(Unix socket bind). Both failed withoperation not permittedfrom the local sandbox; CI runs them unsandboxed.Closes #2954