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

CurlWebSocketResponseBody: handle meta.bytesleft #1

Open
wants to merge 7 commits into
base: curl/ws
Choose a base branch
from

Conversation

git-bruh
Copy link

Subsystem
Client/Server, related modules

Motivation
Describe what problem this PR solves and why it is important. Refer to a bug/ticket #.

Solution
Describe your solution.

// the CURLWS_CONT bit set, so we buffer internally
if (meta.bytesleft == 0L) {
onFrame(partialFragBuffer, meta.flags)
partialFragBuffer = byteArrayOf()

Choose a reason for hiding this comment

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

is there any failure scenarios where we also need to do this?

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by failure scenarios here? The callback would only be called when we receive data so there's no failure case as such, are you referring to needing this pattern in other places?

Choose a reason for hiding this comment

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

if the connection drops while receiving data for example, I'd assume we would need to start partialFragBuffer from scratch.

Copy link
Author

Choose a reason for hiding this comment

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

Ah that makes sense, I'll review the flow in this library and get back to you

@dtretyakov dtretyakov force-pushed the curl/ws branch 2 times, most recently from 0a9f16f to f996e9d Compare December 19, 2024 06:08
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.

3 participants