fix: use UTF-8 with error-replace for all subprocess.run calls (Windows robustness) - #43494
fix: use UTF-8 with error-replace for all subprocess.run calls (Windows robustness)#43494Colinchiu007 wants to merge 1 commit into
Conversation
austinpickett
left a comment
There was a problem hiding this comment.
Approving — the core objective (no more bare text=True, explicit UTF-8 on all subprocess calls) is achieved and the Windows robustness goal is met.
Coverage in gateway.py: 13 calls converted from text=True → encoding='utf-8', errors='replace' ✓
Coverage in gateway_windows.py: 1 call converted ✓
Two calls not normalised (non-blocking, flagging for awareness):
Lines ~387 and ~411 in gateway.py (WMIC and PowerShell process-list calls) already had encoding="utf-8", errors="ignore" before this PR. The diff correctly removes the now-redundant text=True from both, but leaves errors="ignore" rather than normalising to errors="replace". These calls still cannot raise UnicodeDecodeError, so the primary bug is fixed either way. However the PR title claims "ALL subprocess.run calls" use error-replace; these two are a minor discrepancy worth a follow-up if uniform behaviour is desired.
gateway_windows.py comment: The existing comment notes schtasks output uses the console code page, not UTF-8; hard-coding encoding='utf-8' means non-ASCII characters on non-English Windows will get replacement chars (\ufffd). This is acceptable for crash-safety but worth a TODO for a future locale-aware decode.
|
Thanks for identifying the Windows decoding failure mode. Automated hermes-sweeper review found that current
The submitted hard-coded UTF-8 change for |
Background
On Chinese-Windows the gateway crashes repeatedly with:
The traceback points to
subprocess.py line 1599in_readerthread. The root cause: manysubprocess.run(..., text=True, ...)calls use the system default encoding (GBK on Chinese Windows). When a subprocess writes any byte sequence that is invalid GBK (UTF-8 JSON, model responses, URLs, etc.), the reader thread raisesUnicodeDecodeError, the gateway process exits, and the Desktop UI shows offline.Setting
PYTHONUTF8=1only changes the Python interpreter's default encoding — it does not affectsubprocess.run(text=True), which still falls back to the system code page. AfterPYTHONUTF8=1was set, the error appeared as a UTF-8 decode error instead of GBK.Fix
Replaced every
text=Trueinhermes_cli/gateway.pyandhermes_cli/gateway_windows.pywith an explicit UTF-8 decode that never raises:This forces UTF-8 decoding and replaces invalid byte sequences with the Unicode replacement character, preventing the reader thread from crashing.
Impact
No runtime behaviour changes other than robust subprocess output handling. Works on every platform; completely eliminates the GBK-related crash on Windows.
Testing
hermes gateway runon a Chinese-Windows machine.gateway-crash.logno longer containsUnicodeDecodeError.