Skip to content

Fix surrogate values in hook tracing#702

Open
almondsun wants to merge 1 commit into
pytest-dev:mainfrom
almondsun:agent/fix-surrogate-tracing
Open

Fix surrogate values in hook tracing#702
almondsun wants to merge 1 commit into
pytest-dev:mainfrom
almondsun:agent/fix-surrogate-tracing

Conversation

@almondsun

Copy link
Copy Markdown

Summary

  • render traced hook argument values with repr while preserving structural labels
  • pre-format traced hook results with repr at the PluginManager boundary
  • cover surrogate-containing kwargs and first-result hook returns
  • add a bug-fix changelog fragment

Closes #681.

Root cause and impact

Tracing formatted arbitrary Python values with str, which could leave literal
surrogate characters in the resulting message. Writers using ordinary UTF-8
encoding then raised UnicodeEncodeError. Applying repr only at the two value
positions safely escapes surrogates without quoting trace labels or double-
representing unrelated structural content.

Validation

  • uv run pytest testing/test_tracer.py testing/test_pluginmanager.py::test_hook_tracing testing/test_pluginmanager.py::test_hook_tracing_safely_formats_surrogate_result — 8 passed
  • uv run pytest — 141 passed
  • uv run pre-commit run -a — all hooks passed, including Ruff, Flake8, MyPy, formatting, and ReST checks
  • git diff --check

I used Codex as an implementation assistant, reviewed and validated the complete
change, and take responsibility for the contribution and follow-up.

Co-authored-by: OpenAI Codex <codex@openai.com>
@almondsun
almondsun marked this pull request as ready for review July 21, 2026 18:50
Copilot AI review requested due to automatic review settings July 21, 2026 18:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@yangfan-yf-yf yangfan-yf-yf 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.

Reviewed b7179b9 independently.

The change keeps structural trace labels on str() while applying repr() only to arbitrary traced values, so surrogate characters are escaped without changing the label format. The two regression tests cover both kwargs and hook-result paths.

Validation on Windows with Python 3.12:

  • python -m pytest ? 141 passed
  • git diff --check ? clean

The GitHub test matrix and pre-commit checks are also passing.

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.

Tracing crashes with UnicodeEncodeError on surrogate escapes in hook arguments/results

3 participants