Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

add with 1MB file triggers Uncaught Error: stream.push() after EOF #967

Closed
olizilla opened this issue Apr 11, 2019 · 11 comments · Fixed by hugomrdias/pull-to-stream#1 or #980
Closed

Comments

@olizilla
Copy link
Contributor

Uploading a file > 1MB, as a pull-stream fails, with the error:

Uncaught Error: stream.push() after EOF
    at new NodeError (errors-browser.js:30)
    at readableAddChunk (_stream_readable.js:269)
    at PullDuplexStream.push../node_modules/readable-stream/lib/_stream_readable.js.Readable.push (_stream_readable.js:240)
    at next (index.js:55)
    at FileReader.loaded (index.js:22)

The issue is occuring in master on webui.ipfs.io since upgrading v30.1.2 from v29.1.0

See: ipfs/ipfs-webui#1010

@olizilla
Copy link
Contributor Author

Also of note, in ipfs-webui we use https://github.com/tableflip/pull-file-reader

which has a default chunk size of 1MB https://github.com/tableflip/pull-file-reader/blob/master/index.js#L8 which suspiciously conincides with the threshold for triggering this issue.

@olizilla
Copy link
Contributor Author

@hugomrdias this is blocking a relase of webui and desktop. Are you digging in to it or should I jump on it?

@olizilla
Copy link
Contributor Author

investigating this some more...

Screenshot 2019-04-18 at 11 55 48

@hugomrdias
Copy link
Contributor

im on it, will report back asap

@olizilla
Copy link
Contributor Author

@hugomrdias I'm on it. The problem seems to be in pull-file-reader

@olizilla
Copy link
Contributor Author

There is nothing in pull-stream-reader to stop it from emitting the cb(true) for when the offset exeeds the filesize, while it is still waiting for a data callback from the FileReader...

Screenshot 2019-04-18 at 12 28 03

in the above, with a 2Mb file, we see it trigger the offset > filesize callback before the final data chunk.

@hugomrdias
Copy link
Contributor

But this worked before right? what changed in ipfs to trigger this problem?

@olizilla
Copy link
Contributor Author

It sure did! I think something else must have been dealing with it, when it should always have been an error. I just added a quick, "count the pending chunks" check to pull-stream-reader and it fixes it! Will PR a cleaner fix now.

@olizilla
Copy link
Contributor Author

Screenshot 2019-04-18 at 12 37 28

@olizilla
Copy link
Contributor Author

@hugomrdias oh no wait, you are right, this is super weird, the pull-stream sink should not be asking for more chunks before it gets a callback from the previous one... that's the weirdness that is causing this. I could guard against it in pull-file-reader, but it seems like we have a rogue pull-stream sink in ipfs now that is just pulling as fast as it can, rather than waiting for each chunk

@olizilla
Copy link
Contributor Author

Moved the guard to pull-to-stream, and we have a fix! hugomrdias/pull-to-stream#1

@ghost ghost removed the ready label Apr 24, 2019
@lidel lidel reopened this Apr 26, 2019
lidel added a commit to lidel/js-ipfs-http-client that referenced this issue Apr 26, 2019
This switches pull-to-stream to v0.1.1 which includes
a fix for ipfs-inactive#967

License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
@ghost ghost assigned lidel Apr 26, 2019
@ghost ghost added the in progress label Apr 26, 2019
alanshaw pushed a commit to lidel/js-ipfs-http-client that referenced this issue Apr 29, 2019
This switches pull-to-stream to v0.1.1 which includes
a fix for ipfs-inactive#967

License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
@ghost ghost removed the in progress label Apr 29, 2019
alanshaw pushed a commit that referenced this issue Apr 29, 2019
This switches pull-to-stream to v0.1.1 which includes
a fix for #967

License: MIT
Signed-off-by: Marcin Rataj <lidel@lidel.org>
olizilla added a commit to ipfs/ipfs-webui that referenced this issue May 8, 2019
Update ipfs-redux-bundle to ensure latest ipfs-http-client is used
with fix for ipfs-inactive/js-ipfs-http-client#967

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
olizilla added a commit to ipfs/ipfs-webui that referenced this issue May 9, 2019
- Update ipfs-redux-bundle to ensure latest ipfs-http-client is used
with fix for ipfs-inactive/js-ipfs-http-client#967

`let files = await filesOrPromise` was leaving the FileList empty
where it was a FileList rather than a promise of a FileList. No idea 
why yet, but this fixes it for now.


License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.