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 streaming response callbacks #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GAMMACASE
Copy link

This changes callback handling when headers and data are received. Before this pr if you register data or headers callbacks, they wont work as expected, and would fire when RequestCompleted callback is triggered, and in most cases headers and data callbacks got incorrect parameters (not sure if it ever worked, for me it was always incorrect) as it was propagated by HTTPRequestCompleted_t struct and not the others.

This pr changes the callbacks to be global ones, which do work in this instance, but they require manually filtering which request it's called for. Thus there were multiple options for me to take, one is store all the ongoing requests in a list and have one global callback instance to filter there all the requests and fire appropriate forwards. Second as how I did it here, I'm instantiating a new callbacks each time you create a new request, but I do filter them by checking the request handle as it's conveniently provided in the resulting structures. This tho does comes with its downside as it's basically would mean that for each new request other callbacks would be called the same amount of times as there are requests active (yet they would do nothing), not sure if it's anything crucial performance wise.

I also would like to hear your opinion on that matter as well.

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