fix(adhoc-nats-src-object-store-rs): CU-86akbhg13 Object::read silently returns Ok(0) (EOF signal) when subscription.next() yields None even though has_pending_messages is still true - #104
Conversation
…(0) (EOF signal) when subscription.next() yields None even though has_pending_messages is still true
| } | ||
| } | ||
| return Ok(len); | ||
| } else { | ||
| return Err(io::Error::new( | ||
| ErrorKind::UnexpectedEof, | ||
| "subscription ended before all pending chunks were received", | ||
| )); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🦩 🟠 Object::read silently returns Ok(0) (EOF signal) when subscription.next() yields None even though has_pending_messages is still true
In impl io::Read for Object::read (nats/src/object_store.rs), changed the else branch that previously fell through to Ok(0) when self.subscription.next() returns None while has_pending_messages is still true. Now it returns Err(io::Error::new(ErrorKind::UnexpectedEof, ...)) instead of silently signaling clean EOF, so callers like io::copy/read_to_end will surface an error rather than silently truncating the object before digest verification. This is the smallest correct fix within this file; it does not attempt any retry/backoff logic for transient subscription gaps (which would require broader knowledge of PushSubscription's timeout semantics), so if next() returning None can legitimately happen transiently (not just at true stream end) a more complete fix would need to distinguish that case, possibly by retrying within a timeout before erroring.
(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)
🤖 Prompt for AI agents
In nats/src/object_store.rs around line 231, review and complete this code-review fix: Object::read silently returns Ok(0) (EOF signal) when subscription.next() yields None even though has_pending_messages is still true.
What the draft fix changed: In `impl io::Read for Object::read` (nats/src/object_store.rs), changed the `else` branch that previously fell through to `Ok(0)` when `self.subscription.next()` returns `None` while `has_pending_messages` is still `true`. Now it returns `Err(io::Error::new(ErrorKind::UnexpectedEof, ...))` instead of silently signaling clean EOF, so callers like `io::copy`/`read_to_end` will surface an error rather than silently truncating the object before digest verification. This is the smallest correct fix within this file; it does not attempt any retry/backoff logic for transient subscription gaps (which would require broader knowledge of `PushSubscription`'s timeout semantics), so if `next()` returning `None` can legitimately happen transiently (not just at true stream end) a more complete fix would need to distinguish that case, possibly by retrying within a timeout before erroring.
_(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)_
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 40 low — review closely — react 👍/👎 to teach the reviewer
Closes findings from rule adhoc-nats-src-object-store-rs — Object::read silently returns Ok(0) (EOF signal) when subscription.next() yields None even though has_pending_messages is still true.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
nats/src/object_store.rs:231What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
cfb21c27-56cf-4a3e-bbfe-02ce27b8b29bMerging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akbhg13 Code review fixes: NATS async-nats and CI review findings (3 PRs)