Skip to content

fix: terminate fallible iterators after stream read failures - #350

Merged
digizeph merged 3 commits into
mainfrom
fix/349-fallible-nontermination
Oct 7, 2026
Merged

digizeph merged 3 commits into
mainfrom
fix/349-fallible-nontermination

Conversation

@digizeph

@digizeph digizeph commented Oct 7, 2026

Copy link
Copy Markdown
Member

Fixes #349.

Fallible iterators (into_fallible_record_iter, into_fallible_elem_iter,
into_fallible_update_iter, into_fallible_route_iter) now latch a spent input stream: a
framing IoError/EofError whose kind is not Interrupted or WouldBlock is yielded once
and followed by permanent None, and normal EOF ends iteration permanently too. The failed
decoder is never polled again.

Isolated malformed records stay skippable: body parse errors (including the IoError they
carry) do not end iteration, so the documented skip-the-error pattern keeps working for
corrupted records inside a healthy stream.

Implementation notes:

  • Record framing and body parsing are separated on the fallible path
    (next_fallible_raw_record / next_fallible_record in src/parser/iters/mod.rs), so only
    framing read failures latch the stream.
  • parse_chunked_record_with_zebra_compat in src/parser/mrt/mrt_record.rs is shared with the
    existing parse_mrt_record_with_zebra_compat instead of duplicating its parse-from-clone
    logic.

Regression coverage (tests/fallible_compressed.rs, offline and deterministic): damaged
bzip2/gzip streams terminate for all four iterator kinds after the intact records parse, within
bounded pulls; truncated compressed streams report one error and end; clean streams reach EOF;
malformed records and headers remain skippable; retryable reader errors do not end iteration;
a failed reader is not polled again.

No public API, signature, error-variant or dependency changes. The original real-world RIB was
not re-run; the committed regression uses synthetic damaged streams.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 21:54

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.

🟡 Changes recommended

Partial reads followed by WouldBlock can leave retryable iteration permanently misaligned.

1 open finding
What changed in this PR

Fixes non-terminating fallible iterators after fatal stream failures.

Changes:

  • Separates MRT framing from body parsing.
  • Latches fatal framing failures and EOF.
  • Adds compressed-stream regression coverage.
File Description
CHANGELOG.md Documents corrected termination behavior.
src/​parser/​iters/​mod.rs Adds shared framing and termination logic.
src/​parser/​iters/​fallible.rs Applies termination state to record/element iteration.
src/​parser/​iters/​update.rs Applies termination state to updates.
src/​parser/​iters/​route.rs Applies termination state to routes.
src/​parser/​mrt/​mrt_record.rs Extracts parsing of chunked records.
tests/​fallible_compressed.rs Adds regression and error-handling tests.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/parser/iters/mod.rs Outdated
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.06%. Comparing base (ec90671) to head (c10bd5f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #350      +/-   ##
==========================================
+ Coverage   92.04%   92.06%   +0.02%     
==========================================
  Files          96       96              
  Lines       23396    23401       +5     
==========================================
+ Hits        21534    21545      +11     
+ Misses       1862     1856       -6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

A WouldBlock or Interrupted read that arrives after part of the header was consumed left the stream misaligned: the next pull framed a fresh header from a mid-record offset. Latch partial framing failures like other framing errors; a retryable error that consumed nothing stays retryable.
Copilot AI balanced review requested due to automatic review settings October 7, 2026 22:00

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.

🟢 Approval recommended

The implementation cleanly distinguishes fatal framing failures from skippable body errors and has comprehensive regression coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@digizeph
digizeph merged commit 0530ba2 into main Oct 7, 2026
10 checks passed
@digizeph
digizeph deleted the fix/349-fallible-nontermination branch October 7, 2026 22:18
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.

into_fallible_elem_iter can yield an unbounded error stream on corrupt (bit-flipped) MRT input

2 participants