fix(bottlecap): fix how Telemetry API stream is being read #259
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
read properly from the stream so we can read every byte, retry 3 times in case theres no data
also some integration tests, moving away from mocking `TcpStream`, since signature changed
astuyve
reviewed
May 31, 2024
bottlecap/src/telemetry/listener.rs
Outdated
| let content_length = parser | ||
| .headers | ||
| .get("content-length") | ||
| .expect("unfallible") |
Contributor
There was a problem hiding this comment.
do we want to expect here? can't we just drop the request if it's malformed or something broke in our assumption?
Contributor
Author
There was a problem hiding this comment.
The loop before that ensures that the key exist, which means that this is infallible, wdyt?
Contributor
There was a problem hiding this comment.
hmm, I think that's okay. It just makes me nervous
Contributor
Author
There was a problem hiding this comment.
I could definitely add another if let Some(...) block if that makes you feel more confident
astuyve
reviewed
May 31, 2024
| fn handle_stream( | ||
| stream: &mut impl Read, | ||
| mut buf: [u8; 262_144], | ||
| stream: &TcpStream, |
Contributor
There was a problem hiding this comment.
ah yeah I made this change in my branch too, a bit easier to handle than receiving a generic which implements Read.
astuyve
approved these changes
May 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
How?
Created a new
from_streammethod, which reads headers untilcontent-lengthis found, and proceeds to read the exact remaining body size.Motivation
First developed in #256, also SVLS-4932