ipfs-shipyard / ipfs-share-files

Share files via IPFS
https://share.ipfs.io
MIT License
149 stars 31 forks source link

Strange behaviour without IPFS Companion #22

Closed fsdiogo closed 3 years ago

fsdiogo commented 6 years ago

When using the app with IPFS Companion everything works great 🌟

But a simple app like this should require the minimum amount of setup, that being said it should work out-of-the-box with just an IPFS daemon running.

Something strange is happening. Steps to reproduce:

  1. Disable the IPFS Companion if it's active
  2. Fire up an IPFS daemon in the terminal
  3. Start the app and upload a single file -- it should upload successfully and generate a share link
  4. Try to upload another file -- it uploads but it doesn't generate another share link as it should (and does with Companion) because the request hangs forever:
    http://127.0.0.1:5001/api/v0/object/patch/add-link?arg=QmPvs3XyQqFY8sprbS6A18EqCS1b6A2FwKL8tx4DEjVRUD&arg=package.json&arg=QmdXb9kCfLv4bGHRiiFStC66JSH36Gr2y7j7hW9cnrZqUm&stream-channels=true

The problem should come from this func: https://github.com/ipfs-shipyard/ipfs-share-files/blob/3d8c417ffc0c2ababd771e1277afbecd851e1a55/src/lib/files.js#L47-L56

I can't understand why it works with Companion & it works the first time the func is called, it generates the share link. But when calling again it hangs. It also happens if you try to upload multiple files the first time, I think there is something wrong when creating the new unixfs-dir and/or adding links to it.

Ideas? @ipfs-shipyard/gui

lidel commented 6 years ago

@fsdiogo I am not sure if I reproduced the same error :upside_down_face: , but adding multiple files worked for me until I hit this known problem with go-ipfs+js-ipfs-api.

UI froze at this state (I can't remove greyed one):

screenshot_29

You can try reproducing it by adding multiple files and then upload-multiple-files-via-browser-ipfs-api-bug-demo.zip (this .zip triggered the go-ipfs big for me, but due to the nature of the bug it may not work the same on every machine).

As you can read in https://github.com/ipfs/js-ipfs-api/issues/797#issuecomment-401469158 there is no known workaround for HTML context. The only thing we can do is to count input arguments and detect if files.add result list is too short and display error to the user. I described all the steps in https://github.com/ipfs-shipyard/ipfs-webui/pull/769#issuecomment-416900828.

You can add error handling in the app, but in the long run we probably should fix this upstream in js-ipfs-api ((https://github.com/ipfs/js-ipfs-api/issues/797)

PS. It probably worked with Companion because WebExtension has more powerful APIs than HTML JS and applies a fix for /api/v0/add.

fsdiogo commented 6 years ago

@lidel adding multiple files works, the problem is that the share link doesn't get generated!

You are right, I was focusing on the share link because it was the last step that seemed to be broken, but it doesn't generate a new one because of the API problem you mentioned! Well, that sucks and takes too much fun of this app, so I would say to use Companion until the bug is solved. What do you think?

PS: The UI isn't frozen in that state, that 'x' is not clickable, although in v1.1 I'll add the ability to remove those files.

lidel commented 6 years ago

AFAIK we can't fix it from the app itself: website's JS can't set Connection nor Expect headers :( However, I've added a new workaround for go-ipfs issue (https://github.com/ipfs-shipyard/ipfs-companion/pull/562) to IPFS Companion. When merged, it will finally fix issue under Chrome if user has our extension enabled.

See the PR for more details.

jessicaschilling commented 3 years ago

@lidel can we close this?

lidel commented 3 years ago

Yup, this is no longer relevant as we deprecated window.ipfs and will use js-ipfs running on the page itself.