Skip to content
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.

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

fix: fix silent failure to add data of more than ~16MB via add_bytes or add_bytes_named #36

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

rklaehn
Copy link
Collaborator

@rklaehn rklaehn commented Dec 10, 2024

Description

  • Make sure an error in send_all is not silently ignored but at least communicated to the remote side.
  • Split large Bytes in add_bytes into chunks so we don't exceed MAX_FRAME_SIZE of 1024 * 1024 * 16
  • Add some tests that use quinn

Breaking Changes

None

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

Some errors don't happen if you use the mem transport.
Also fix bug where add_bytes fails if the bytes are larger than the max
frame size.
Copy link

github-actions bot commented Dec 10, 2024

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-blobs/pr/36/docs/iroh_blobs/

Last updated: 2024-12-12T14:47:49Z

@@ -363,6 +372,15 @@ version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "597bb81c80a54b6a4381b23faba8d7774b144c94cbd1d6fe3f1329bd776554ab"

[[package]]
name = "bincode"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this making a comback?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what? How did this get here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, it was via quic-rpc, where I forgot to remove the feature from tokio-serde.

It is gone now:

❯ cargo tree --all-features -i bincode
error: package ID specification `bincode` did not match any packages

Copy link
Member

Choose a reason for hiding this comment

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

But the Cargo.lock file here still has bincode here, right?
Why does cargo tree tell you it's not in there, but the Cargo.lock file in this PR still adds it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rklaehn did you give up fixing this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will be in the next PR.

Copy link
Collaborator

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

the dependency changes need some review, otherwise LGTM

@rklaehn rklaehn merged commit dec9643 into main Dec 12, 2024
23 of 24 checks passed
@rklaehn rklaehn deleted the blobs-1g-test branch December 12, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants