Feature/logging error handling 优化异常处理/日志打印逻辑#9
Conversation
- 增加exception_context自定义装饰器，支持同步/异步函数 - 将各个步骤函数均增加该装饰器确保异常发生时可以快速定位 - 去除了可读性差的多处try except代码块
- 新增logger模块，集成RichHandler实现美化终端输出 - 所有模块(auth/browser/video/main)统一使用logger记录日志 - 保留用户交互场景的print语句(登录选择、Cookie输入等) - 日志同时输出到debug.log文件和终端，便于调试和追踪 - 优化异常处理，针对TargetClosedError提供友好提示
- 使用 RotatingFileHandler 实现日志文件自动轮转（默认5MB，保留3个备份） - 添加第三方库（playwright、httpx等）日志级别过滤，减少噪音 - 增加全局初始化标志防止重复配置 - 为 setup_logging 添加可配置参数（日志文件路径、大小、备份数） - 启用 RichHandler 的 rich_tracebacks 功能 - 优化类型注解，使用现代 Python 语法（str | None） - get_logger 自动调用 setup_logging 确保日志系统已初始化
- 导入exception_context模块 - 使用@exception_context装饰器包装cookie_fix函数，提供"Cookie转换"异常上下文 - 增强错误追踪和调试能力
- 从 playwright.async_api 导入 PlaywrightTimeoutError - 将 TimeoutError 替换为 PlaywrightTimeoutError 以提高异常捕获的准确性 - 移除日志消息中的 Rich 颜色标记（[yellow]、[green]、[cyan]、[red]等），使日志输出更简洁统一
- 新增 BrowserClosedError 自定义异常类用于标识用户主动关闭浏览器 - 实现浏览器关闭错误检测函数，支持异常链和错误消息匹配 - 在异常装饰器中统一捕获并转换浏览器关闭相关异常 - 添加自定义 unraisablehook 抑制 asyncio 清理时的资源警告 - 优化主程序异常处理，区分用户主动关闭和程序错误 - 改善用户体验，浏览器关闭时友好提示而非报错
在 main.py 中添加 KeyboardInterrupt 异常捕获，优雅处理用户中断操作；debug.log 记录了新的程序启动和 Cookie 登录流程日志
新增 _custom_excepthook 函数并设置为 sys.excepthook，用于捕获和记录所有未处理的异常到日志文件。KeyboardInterrupt 异常被排除在外以保持友好的用户体验。此功能增强了应用程序的可调试性和错误追踪能力。
- 移除异常检测中的硬编码中文错误消息 - 在 video.py 中使用 BrowserClosedError 替代通用 Exception - 简化 main.py 中的异常处理逻辑，移除重复的浏览器关闭检测 - 通过专用异常类型提高代码可维护性和类型安全性
- 删除根目录的 debug.log 文件，避免敏感信息泄露 - 更新 .gitignore 忽略 log/ 目录 - 重构 logger.py：自动创建 log/ 目录并将日志文件统一存放其中 - 改进日志文件路径处理，使用 Path 对象提高可维护性
📝 WalkthroughWalkthroughIntroduces centralized logging and an exception_context decorator; replaces print statements with logger calls across modules, wraps public automation methods with exception_context, adds BrowserClosedError handling, and updates .gitignore to ignore the log directory. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Main
participant AuthManager
participant Browser as PlaywrightPage
participant Logger
participant ExceptionCtx as exception_context
User->>Main: start program
Main->>Logger: setup_logging()
Main->>AuthManager: interactive_login_and_save_cookies(...)
AuthManager->>ExceptionCtx: decorated call enters context
ExceptionCtx->>Browser: open login page / interact
Browser->>AuthManager: page content / cookies
AuthManager->>Logger: log steps (info/warn)
ExceptionCtx-->>AuthManager: on exception -> wrap/raise BrowserClosedError or RuntimeError
AuthManager->>Main: return login result / saved cookies
Main->>Logger: final status logs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @YewFence, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在通过引入一个集中式的日志系统和一套健壮的异常处理机制，显著提升项目的可维护性和稳定性。通过将分散的 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 围绕异常处理和日志打印进行了出色的重构。通过引入统一的 logger 模块和 exception_context 装饰器，代码的健壮性和可维护性得到了显著提升。主要改进包括：
- 使用
logging模块替代了print，实现了日志的结构化输出（终端+文件）。 - 利用装饰器统一了异常捕获逻辑，简化了业务代码。
- 增强了对用户关闭浏览器等场景的处理，并优雅地处理了相关的
asyncio清理问题。 - 增加了全局异常钩子，确保未捕获的异常也能被记录。
整体来看，这些改动非常专业，使项目向生产级应用迈出了一大步。我只提出了一些微小的建议，以进一步提升代码的一致性。
![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: 045c3c9d2e
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
automation/video.py (1)
124-149: CatchPlaywrightTimeoutErrorinstead of built‑inTimeoutError.The file already imports Playwright's timeout exception as
PlaywrightTimeoutError(line 7). Whenvideo.wait_for()times out, it raises this Playwright-specific exception, not the built-inTimeoutError. The current handler won't catch it, causing an unhandled exception instead of gracefully handling the "not a video page" case.🐛 Proposed fix
- except TimeoutError: + except PlaywrightTimeoutError:
🤖 Fix all issues with AI agents
In `@automation/exception_context.py`:
- Around line 5-16: Remove the internal Playwright import and the isinstance
check against TargetClosedError in _is_browser_closed_error; instead rely on
BrowserClosedError and message-based detection. Concretely, delete the line
importing TargetClosedError and change _is_browser_closed_error(e: Exception) to
return True if isinstance(e, BrowserClosedError) or if the exception message
(str(e) or ''.join(map(str, e.args))) contains the phrase "target page, context
or browser has been closed"; otherwise return False. Ensure the function always
returns a boolean.
In `@cookie_fix.py`:
- Around line 16-20: The function currently calls json.loads(content) which can
raise a JSONDecodeError (and with `@exception_context` that exception escapes
instead of returning False); wrap the json.loads call in a try/except that
catches json.JSONDecodeError (or ValueError for older py versions), log a clear
error via logger.error including the invalid content or a short snippet and the
decode error message, and return False instead of letting the exception
propagate; update the code paths referencing browser_cookies to only proceed
when the load succeeded.
🧹 Nitpick comments (2)
main.py (1)
179-181: Preferlogger.exceptionoverlogger.error+traceback.print_exc().
logger.exception()automatically includes the exception traceback and keeps it within your logging pipeline with consistent formatting and handlers, whiletraceback.print_exc()bypasses your logging configuration and writes directly to stderr.♻️ Proposed refactor
-import traceback @@ - except Exception as e: - logger.error(f"\n❌ 发生错误: {e}") - traceback.print_exc() - suggestions() + except Exception: + logger.exception("\n❌ 发生错误") + suggestions()automation/video.py (1)
46-74: Align return type with possibleNone.
ensure_video_playingreturnsNonewhen no element is found, so the annotation should be optional to match runtime behavior.♻️ Proposed fix
- async def ensure_video_playing(self, video_selector: str = "video") -> dict: + async def ensure_video_playing(self, video_selector: str = "video") -> Optional[dict]:
移除 Playwright 内部异常依赖，统一用日志堆栈输出，并修复超时/无效Cookie JSON 的回退逻辑。
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.py (1)
74-189: Add finally block to ensure browser cleanup on all exit paths.
browser_manageris initialized but has no guaranteed shutdown. The cleanup methodclose()exists in BrowserManager but is never called. Multiple early returns (login failure, user interrupt, etc.) skip exception handlers entirely, causing resource leaks.Add a
finallyblock after the exception handlers:Fix pattern
try: # existing code except BrowserClosedError: logger.info("\n👋 检测到浏览器已关闭，程序正常退出") except Exception: logger.error("\n❌ 发生错误", exc_info=True) suggestions() + finally: + if browser_manager: + await browser_manager.close()
🤖 Fix all issues with AI agents
In `@automation/exception_context.py`:
- Around line 11-33: The helper _is_browser_closed_error currently walks the
exception chain via __cause__ but misses exceptions linked through __context__;
update _is_browser_closed_error to also inspect e.__context__ (in addition to
e.__cause__) by recursively calling _is_browser_closed_error on e.__context__
when present (and keep the existing __cause__ check), so browser-closed errors
propagated via implicit context are detected; ensure you reuse the same function
name _is_browser_closed_error for the recursion to avoid duplicating logic.
| def _is_browser_closed_error(e: BaseException) -> bool: | ||
| """检查是否为浏览器关闭相关的错误""" | ||
| if isinstance(e, BrowserClosedError): | ||
| return True | ||
| # 检查异常链中是否包含浏览器关闭错误 | ||
| if e.__cause__ and _is_browser_closed_error(e.__cause__): | ||
| return True | ||
| # 检查错误消息 | ||
| # 不依赖 Playwright 的内部异常类型；用消息匹配即可覆盖“目标已关闭”等场景。 | ||
| error_msg = ( | ||
| (str(e) or "").lower() | ||
| + " " | ||
| + ("".join(map(str, getattr(e, "args", ()))) or "").lower() | ||
| ) | ||
| if any( | ||
| keyword in error_msg | ||
| for keyword in [ | ||
| "target page, context or browser has been closed", | ||
| "browser has been closed", | ||
| ] | ||
| ): | ||
| return True | ||
| return False |
There was a problem hiding this comment.
Include __context__ when walking the exception chain.
Exceptions raised without from store the original in __context__, so browser-closed errors can be missed. Consider checking it alongside __cause__.
🔧 Suggested update
# 检查异常链中是否包含浏览器关闭错误
if e.__cause__ and _is_browser_closed_error(e.__cause__):
return True
+ if e.__context__ and _is_browser_closed_error(e.__context__):
+ return True🧰 Tools
🪛 Ruff (0.14.14)
19-19: Comment contains ambiguous ； (FULLWIDTH SEMICOLON). Did you mean ; (SEMICOLON)?
(RUF003)
🤖 Prompt for AI Agents
In `@automation/exception_context.py` around lines 11 - 33, The helper
_is_browser_closed_error currently walks the exception chain via __cause__ but
misses exceptions linked through __context__; update _is_browser_closed_error to
also inspect e.__context__ (in addition to e.__cause__) by recursively calling
_is_browser_closed_error on e.__context__ when present (and keep the existing
__cause__ check), so browser-closed errors propagated via implicit context are
detected; ensure you reuse the same function name _is_browser_closed_error for
the recursion to avoid duplicating logic.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.