Skip to content

fix: hide BrokenResourceError warnings that polute logs#265

Closed
mangelajo wants to merge 1 commit intomainfrom
remove-brokenres-warnings
Closed

fix: hide BrokenResourceError warnings that polute logs#265
mangelajo wants to merge 1 commit intomainfrom
remove-brokenres-warnings

Conversation

@mangelajo
Copy link
Copy Markdown
Member

@mangelajo mangelajo commented Feb 25, 2026

when the remote end closes the call (the call has ended) we get a BrokenResourceError in the stream. We need unecessary warnings, we add masking that was removed in e756ba9.

Fixes this:

j power on
~ ⚡qti-snapdragon-ride4-sa8775p-03 ➤ 
[02/24/26 13:29:42] WARNING  WARNING:jumpstarter.streams.common:stream copy interrupted (BrokenResourceError):                   common.py:30

Summary by CodeRabbit

  • Bug Fixes
    • Refined error handling during streaming operations to improve reliability and provide enhanced diagnostic logging for better troubleshooting.

when the remote end closes the call (the call has ended) we get a
BrokenResourceError in the stream. We need unecessary warnings,
we add masking that was removed in e756ba9.
@bennyz
Copy link
Copy Markdown
Member

bennyz commented Feb 25, 2026

similar to #257

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

Error handling in the copy_stream function was restructured to separate exception handling logic. BrokenResourceError is now caught and suppressed independently during teardown, while ClosedResourceError and asyncio.InvalidStateError are handled separately with warning logs and debug tracing.

Changes

Cohort / File(s) Summary
Error Handling Refactoring
python/packages/jumpstarter/jumpstarter/streams/common.py
Split combined exception clause in copy_stream into two branches: one suppressing BrokenResourceError during teardown, and another logging warnings for ClosedResourceError and asyncio.InvalidStateError with optional debug root cause tracing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

Streams now flow with clarity bright,
Error paths split left and right,
Broken resources fade from sight,
While warnings guard the flow at night,
A rabbit hops with pure delight! 🐰

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: hiding BrokenResourceError warnings that clutter logs, which directly matches the PR objective of masking unnecessary warnings in stream teardown.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-brokenres-warnings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mangelajo
Copy link
Copy Markdown
Member Author

similar to #257

oh, in fact I like his PR better, I was thinking that it was better to move it under debug.

Thanks for the pointer.

@mangelajo mangelajo closed this Feb 25, 2026
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