Skip to content

fix(terminal): handle ImportError for termios on Windows in password reader - #207

Closed
Sertug17 wants to merge 1 commit into
NousResearch:mainfrom
Sertug17:main
Closed

fix(terminal): handle ImportError for termios on Windows in password reader#207
Sertug17 wants to merge 1 commit into
NousResearch:mainfrom
Sertug17:main

Conversation

@Sertug17

Copy link
Copy Markdown
Contributor

Problem

On Windows, import termios raises ModuleNotFoundError (a subclass of ImportError). The read_password_thread() function in terminal_tool.py only caught (EOFError, KeyboardInterrupt, OSError), so Windows users would get an unhandled exception crash when sudo password reading was triggered.

Same issue existed in the finally block where termios is imported again to restore terminal attributes.

Root Cause

termios is a Unix-only module. CONTRIBUTING.md explicitly calls out this pattern as a cross-platform requirement:

"termios and fcntl are Unix-only. Always catch both ImportError and NotImplementedError"

Fix

  • Main except block: added ImportError and NotImplementedError
  • Finally block: added ImportError and NotImplementedError

Testing

Verified the fix follows the exact pattern recommended in CONTRIBUTING.md.

@Sertug17 Sertug17 closed this Feb 28, 2026

@guog guog left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码审查:

🔴 Critical

  • tools/terminal_tool.py:221 / tools/terminal_tool.py:226 — 异常捕获语法不规范。在 Python 中,ImportErrorNotImplementedError 继承自 Exception。当你写成 except (Exception, ImportError, NotImplementedError) 时,其实相当于只写了 except Exception,这让特定的错误捕获失效。如果想遵循 CONTRIBUTING.md 中的规范,应将特定的错误和一般的 Exception 分开捕获:
except (ImportError, NotImplementedError):
    pass
except Exception:
    pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants