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

Big refactor of command handling, big payloads now correctly handled #6

Merged
merged 4 commits into from
Apr 8, 2019

Conversation

divmgl
Copy link
Member

@divmgl divmgl commented Apr 8, 2019

No description provided.

@divmgl divmgl self-assigned this Apr 8, 2019
@divmgl divmgl mentioned this pull request Apr 8, 2019
@divmgl divmgl merged commit 12b1f1b into master Apr 8, 2019
@divmgl divmgl deleted the big-payloads branch April 8, 2019 19:43
@divmgl
Copy link
Member Author

divmgl commented Apr 8, 2019

For anyone who is wondering about the technical side of this:

createCommandHandler was previously handling the chunks processing. This is incorrect because createCommandHandler is only invoked once all of the chunks are available. The code that involved a recursive call was removed because it would only ever be invoked once, and most of the buffering code was moved to the parent: the response event handler.

The next issue is that the response event handler was not buffering data correctly in multipart scenarios. Essentially, if an incoming data chunk did not have a \r\n, undefined would be sent to the command handler. This implementation continues to fill the buffer until it reaches a \r\n. If it reaches a \r\n and it's a multipart command, it will store the first result, clear the buffer, fill the buffer a second time until it reaches \r\n, then send both the previous pending command result and the payload over to the command handler.

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.

1 participant