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

[Probably a bug] Streaming #83

Closed
piliugin-anton opened this issue Jul 5, 2022 · 6 comments
Closed

[Probably a bug] Streaming #83

piliugin-anton opened this issue Jul 5, 2022 · 6 comments

Comments

@piliugin-anton
Copy link
Contributor

I think there is a possible bug here

according to example from here: https://github.com/uNetworking/uWebSockets.js/blob/d52c79d5166b7579c56f419bfaf59bbd64fe1588/examples/VideoStreamer.js#L67 we should send a chunk - offset, I fixed it in my package (still not sure if it's correct): piliugin-anton/uQuik@b9f95cf
The problem appears when you have a situation when chunk is not been sent

@piliugin-anton
Copy link
Contributor Author

I found out that offset in drain and lastOffset is the same value

@kartikk221
Copy link
Owner

kartikk221 commented Jul 5, 2022

Yeah, I used to actually have that sliced chunk being served for backpressuring but it's a very weird implementation and it turned out that chunk slicing was not needed because the write() method buffers the chunk under the hood anyways. So essentially, you should serve the next chunk after backpressure relieves rather than serve the same chunk but sliced. If you find this behavior to be incorrect, feel free to link the test code to recreate the bug in hyper-express and I'd be glad to push a fix.

@piliugin-anton
Copy link
Contributor Author

The issue is stream hangs on slow internet connection when streaming a few megabytes in few files. I'm using this method stream to stream static files, the issue appears when I'm testing it on a production server over internet. So you may try to reproduce it by serving multiple large images over network (you may try to throttle the bandwidth in browser devtools). It may appear in future when you will be developing a middleware for static files serving

@kartikk221
Copy link
Owner

I see, I'll try to recreate this myself. If you end up writing a snippet to simulate this bug with high probability feel free to share.

@piliugin-anton
Copy link
Contributor Author

I think i finally fixed this method issues for both: streams with size and without size

@kartikk221
Copy link
Owner

I think i finally fixed this method issues for both: streams with size and without size

Great! Feel free to make a PR and I'd be glad to have it implemented.

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

No branches or pull requests

2 participants