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

Better Chunk loading handling to avoid HTTP 416 (Range Not Satisfiable) Error #832

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

t83714
Copy link
Contributor

@t83714 t83714 commented Sep 18, 2020

Fixed: #831

When request content in chunks, PapaParse may trigger HTTP 416 (Range Not Satisfiable) Error under the following two situation:

  • we always calculate end as: var end = this._start + this._config.chunkSize - 1; see here. This likely always make the last chunk request to request a range that is beyond the actual file size (unless the actual last chunk size is just equal to the this._config.chunkSize.) Many servers will response HTTP 416 error for this.
    • Solution: use Content-Range header to retrieve the actual file size and use actual content size - 1 as the end for the last chunk
  • As we often don't know the actual size of the target file before we request it, it's possible the this._config.chunkSize is configured to a size that is larger than the file size. e.g. the file is only 100 bytes and this._config.chunkSize is configured to 64KB. When this happens, PapaParse will always fail on the first chunk request due to HTTP 416 (Range Not Satisfiable) Error
    • Solution: we should retry (only once) on the first chunk request without Range header on 416 error

@RoelRoel
Copy link

I really need this. Why is it not merged?

@pokoli
Copy link
Collaborator

pokoli commented Apr 19, 2023

@RoelRoel Nobody reviewed the code, will you like to do so?
Also How can I ensure that it works? Do we have some way to test it?

@t83714
Copy link
Contributor Author

t83714 commented Apr 20, 2023

@pokoli Will nock based test cases be sufficient to prove its correctness?
If they are sufficient and @RoelRoel or anyone else can help with the review, I am happy to add test cases to the PR.

@sztupy
Copy link

sztupy commented Apr 25, 2023

This bug effectively makes PapaParse unusuable as a stream CSV loader on some webservers, so it would be good to have this merged with a good regression test to avoid this breaking again in the future. I am happy to lend a hand if needed

@jsmolina
Copy link

jsmolina commented Jul 6, 2023

this issue should be merged asap

@RoelRoel
Copy link

RoelRoel commented Jul 6, 2023

In our app which runs locally in a browser we now download the file first so the browser has it in its cache so you won't get the 416 error. But it would be nice to be able to remove this workaround. At the moment I unfortunately cannot find time to add tests to this library, maybe later when I have less work to do.

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.

Better Chunk loading handling to avoid HTTP 416 (Range Not Satisfiable) Error
5 participants