ipfs.files.add is missing error handling on invalid response from go-ipfs

[lidel]

This bug was originally found in ipfs/ipfs-companion#480.
Since then I was able to confirm issue originates between js-ipfs-api and HTTP API exposed by go-ipfs and create demo app that reproduced issue on two separate machines.

The Bug

Sometimes (for specific multipart payloads in browser context) the ipfs.files.add backed by go-ipfs returns an error in the middle of valid payload:

{"Name":"T-RexFaceEmoji.jpg_original","Hash":"QmR1G8Jq4rNiYwCKyNWEZhNS7i8EWrCpz2xDjKsB9ofi82","Size":"23723"}
{"Name":"FlyingSaucerEmoji.jpg_original","Hash":"QmVWxHcLVHYpFJP6R1ZLBZcpAZsxyFMfU7BU84D2LAMSed","Size":"13071"}
{"Message":"http: invalid Read on closed Body","Code":0,"Type":"error"}

Then, upon receiving this payload, js-ipfs-api does not throw an error, but silently ignores the last line and returns a partial result list with only two first items. (screenshot)

A similar problem occurs for some single file uploads (with wrapWithDirectory: true), js-ipfs-api returns an empty list of responses [] (screenshot)

Note: I created upstream issue for go-ipfs ipfs/kubo#5168, this issue is about handling "partial responses with error" in js-ipfs-api in a way that does not fail silently.

How to Reproduce

Turns out the .zip with sample app triggers the issue 🙃 , so its easy to reproduce:

1. Expose API of go-ipfs 0.4.15 or 0.4.16-rc1 locally on port :5001
2. Download demo app: upload-multiple-files-via-browser-ipfs-api-bug-demo.zip
   (uses ipfs-api ^22.1.1 and uploads files via ipfs.files.add with wrapWithDirectory: true)
3. Build it and start via npm install && npm start, then open form at http://localhost:3000
4. Reproduce using samples known to trigger issue at go-ipfs
   • Sample 1: upload single file: upload-multiple-files-via-browser-ipfs-api-bug-demo.zip
     • This should produce this error
   • Sample 2: upload these three files at once
     • This should produce this error
     • Not sure how useful it is here, but I recorded HTTP Archive (HAR) of this interaction when it produced the error: Archive%2018-06-28%2015-34-47.har

[lidel]

Ok, so Client-side workaround for http: invalid Read on closed Body at HTTP API is to force HTTP client to sent upload request with Connection: close or Expect: 100-continue.

While it sounds like something js-ipfs-api could do, there are limitations defined in XHR Spec: it defines Connection and Expect as "a forbidden header names". This means these headers can't be modified from JS via setRequestHeader() method.

Are there any other options, or is this blocked until upstream fix lands?

[olizilla]

This is a blocker to releasing the share files feature in the new web ui, as it's purpose is to let the user upload multiple files for sharing with others.

ipfs-shipyard/ipfs-share-files#22

[alanshaw]

I think this might be where we might want to throw an error rather than ignore/smother: https://github.com/ipfs/js-ipfs-api/blob/63682dd7078197af5d820234e66aff29552288d4/src/utils/file-result-stream-converter.js#L32-L34

Looks like there's a PR open for go-ipfs now so hopefully this will go away soon, however if someone wants to do something in the mean time I'd start looking there ^^^