Skip to content

Fix: IPC AsyncMessageReader does not handle partial reads #5758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
connortsui20 merged 4 commits into from
Jan 5, 2026
Merged

Fix: IPC AsyncMessageReader does not handle partial reads #5758

connortsui20 merged 4 commits into from
Jan 5, 2026

Conversation

@connortsui20
Copy link
Contributor

@connortsui20 connortsui20 commented Dec 16, 2025 •
edited
Loading

Fixes: #5633

I doubt that the extra enums I added for readability will hurt performance in any meaningful way.

@connortsui20 connortsui20 added the changelog/fix A bug fix label Dec 16, 2025
@connortsui20
Copy link
Contributor Author

connortsui20 commented Dec 16, 2025

actually I just realized this does not handle EOF in the middle of a partial read as an error

@connortsui20 connortsui20 marked this pull request as draft December 16, 2025 18:30
@codecov
Copy link

codecov bot commented Dec 16, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 94.68085% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.95%. Comparing base (da5d463) to head (dc65ef8).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-ipc/src/messages/reader_async.rs 88.09% 5 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 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.

gatesn
gatesn previously approved these changes Dec 16, 2025
View reviewed changes
@connortsui20
Copy link
Contributor Author

connortsui20 commented Dec 16, 2025

@gatesn can you take a look again I made some changes

@connortsui20 connortsui20 marked this pull request as ready for review December 16, 2025 18:46
blaginin and others added 2 commits December 16, 2025 13:55
@blaginin @connortsui20
Add a failing test_async_stream_chunked
a7d5768 
Signed-off-by: blaginin <dima@spiraldb.com>
@connortsui20
fix AsyncMessageReader to handle partial reads
d534e33 
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 requested a review from gatesn December 16, 2025 18:55
@codspeed-hq

This comment was marked as off-topic.

Sign in to view
@connortsui20 connortsui20 enabled auto-merge (squash) December 17, 2025 18:08
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 3, 2026 •
edited
Loading

Deploying vortex-bench with  Cloudflare Pages  Cloudflare Pages

Latest commit: dc65ef8
Status: ✅  Deploy successful!
Preview URL: https://7259c965.vortex-93b.pages.dev
Branch Preview URL: https://db-failing-stream.vortex-93b.pages.dev

View logs

@connortsui20 connortsui20 merged commit b3231e4 into develop Jan 5, 2026
49 checks passed
@connortsui20 connortsui20 deleted the db/failing-stream branch January 5, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert3005 robert3005 robert3005 approved these changes

@AdamGS AdamGS Awaiting requested review from AdamGS

@blaginin blaginin Awaiting requested review from blaginin

@gatesn gatesn Awaiting requested review from gatesn

Assignees

No one assigned

Labels

changelog/fix A bug fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@connortsui20 @robert3005 @gatesn @blaginin