fix(windows): decode wmic output with explicit encoding to survive non-UTF-8 locales (#17049)#17435
Merged
Conversation
…n Windows non-UTF-8 locales (#17049) `hermes update` calls `_warn_stale_dashboard_processes()` to warn about dashboard processes still running the pre-update Python backend. On Windows, that scan shells out to `wmic process get ProcessId,CommandLine /FORMAT:LIST` with `text=True` and no explicit encoding. `wmic` emits text in the system code page (e.g. cp936 on zh-CN locales), not UTF-8. Without an explicit `encoding=`, Python's default UTF-8 decoder crashes the subprocess reader thread with `UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd0 ...`. In Python 3.11 that crash is silently absorbed: `subprocess.run()` returns a `CompletedProcess` with `result.stdout = None`, the next line calls `result.stdout.split("\n")`, and `hermes update` aborts with the exact `AttributeError: 'NoneType' object has no attribute 'split'` trace reported in #17049. Fix: pass `encoding="utf-8", errors="ignore"` so undecodable bytes cannot take down the reader thread (the parsing only matches the ASCII prefixes `CommandLine=` and `ProcessId=`, so dropping non-UTF-8 bytes is safe), and short-circuit when `result.stdout is None` as a defensive guard for environments where the reader thread still fails for other reasons. This is the same root cause as #17074 (which patches `hermes_cli/gateway._scan_gateway_pids` for the `hermes setup` path). That PR does not touch `_warn_stale_dashboard_processes`, so `hermes update` remains broken on the same locales until this lands. Regression test in `tests/hermes_cli/test_update_stale_dashboard.py`: - `test_wmic_invoked_with_utf8_ignore_errors` asserts the explicit encoding/errors kwargs reach `subprocess.run`. - `test_wmic_returns_none_stdout_does_not_crash` simulates the reader-thread-crashed `result.stdout=None` aftermath and asserts the function returns silently instead of raising AttributeError. Both new tests fail against clean origin/main (7d46484) reproducing the original AttributeError; both pass with this patch. The remaining 3 failures in `tests/hermes_cli/test_cmd_update.py` and `test_update_autostash.py` are pre-existing baselines on origin/main — they reproduce identically without this change and are unrelated to the wmic scan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ys.platform Two fix-ups for #17123: 1. Reword the inline comment in `_warn_stale_dashboard_processes` to accurately describe the failure mode (locale-dependent decoder, not a "default UTF-8 decoder") and identify `errors="ignore"` as the load-bearing protection. Per Copilot's review. 2. Switch `TestWindowsWmicEncoding` from `patch("hermes_cli.main.sys")` to `monkeypatch.setattr(sys, "platform", "win32")` — the codebase's canonical pattern (e.g. `tests/hermes_cli/test_auth_ssl_macos.py`). The MagicMock-replacement approach passed locally on Python 3.12 but the platform-equality check failed under CI's xdist+Python 3.11, leaving both new tests red despite the fix being present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pass encoding='utf-8', errors='ignore' and guard against result.stdout being None so _scan_gateway_pids() no longer crashes with UnicodeDecodeError + AttributeError on Windows systems whose default code page is not UTF-8 (e.g. cp936 on zh-CN). The parser only matches the ASCII prefixes CommandLine= and ProcessId=, so dropping undecodable bytes is safe. Closes #17049.
This was referenced Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
hermes setupandhermes updateboth scan the Windows process table viawmic process get ProcessId,CommandLine /FORMAT:LISTwithtext=Trueand no explicit encoding. On systems whose default code page is not UTF-8 (e.g. cp936 on zh-CN), the subprocess reader thread crashes withUnicodeDecodeError, leavesresult.stdout = None, and the subsequent.split("\\n")aborts both commands withAttributeError: 'NoneType' object has no attribute 'split'— exactly the trace in #17049.This PR closes the bug at both call sites by passing
encoding="utf-8", errors="ignore"and guarding againststdout is None. The parser only matches the ASCII prefixesCommandLine=andProcessId=, so dropping undecodable bytes is safe.Changes
hermes_cli/gateway.py—_scan_gateway_pids()(setup wizard / service detection path).hermes_cli/main.py—_warn_stale_dashboard_processes()(hermes updatepath).tests/hermes_cli/test_update_stale_dashboard.py— regression tests asserting the encoding kwargs reachsubprocess.runand that aNonestdout short-circuits instead of crashing.scripts/release.py—AUTHOR_MAPentry for @Kailigithub.Credit
Salvaged from two complementary contributor PRs, both preserved via rebase-merge:
_scan_gateway_pidsfix._warn_stale_dashboard_processesfix + regression tests.(#17271 and #17090 attempted the same fix but bundled unrelated changes; closing those in favor of this PR.)
Validation
tests/hermes_cli/test_update_stale_dashboard.py— 12/12 pass (10 existing + 2 new).tests/hermes_cli/test_gateway.py+test_gateway_service.py+test_update_gateway_restart.py— 169/169 pass.Closes #17049.