[pubsub] Orbit electron concerns -- clashing ports and repos

jbenet commented 7 years ago

Status quo

current state of `orbit-electron@pubsub`, using `go-ipfs@feat/pubsub`

users may have installations of go-ipfs. at any of {@0.4.2 or lower, @0.4.3-rc#, @0.4.4-dev, @feat/pubsub}
users may have lingering $user/.ipfs repos at various versions. {1, 2, 3, 4}
orbit-electron requires using local repo ($user/.ipfs)
orbit-electron may use local ipfs or its own (i believe as of today it uses its own, go-ipfs@feat/pubsub}
if orbit-electron fails to use the repo (wrong version, locked, etc), the user gets no feedback, only a white screen on startup.
if orbit-electron fails to use the daemon, the user gets no feedback (? not sure about this)
if orbit-electron go-ipfs daemon fails to bind ports, it will fail, but orbit may still work and use the wrong daemon (no feedback)
if orbit-electron uses the daemon but it does not support pubsub, user gets no feedback, but things won't work well (? not sure about this)
Which add up to
failures on startup, which are not fixable by the user (without help from other people or github repositories. not good)
failures on startup that make it difficult for people to "pass an application bundle (.app) to each other, double click, and enjoy".
failures with terrible UX ("white screen", silent failures, etc)
Why all this is big problem
it is a goal for devcon that others are able to try our stuff on the spot
it is a goal for devcon that others are impressed by our tech and want to work with it / build on it
users may have installations of ipfs
users may want to try orbit
users will have broken UX and not know easily how to fix it
even if users manage to figure it out, they will react like "ughhh wtf it should be smarter than this"
this makes orbit almost entirely unusable by people who are not us.
I would not feel comfortable letting people try it
Suggested fixes

Fix1: No clashes (easiest)

Change Orbit to ensure it NEVER clashes with user or system ipfs processes, nor "other people's servers at ports :8080, :5001, :4001.

This means:

Orbit ALWAYS uses its own go-ipfs binary
Orbit chooses its own ports for ipfs daemon and sets them in the ipfs config
- either by choosing other ports (maybe :8180, :5101, :4101) -- this may still clash with other services, so orbit SHOULD STILL show an error if ipfs daemon fails to bind
- or by ensuring there are no port conflicts (use /tcp/0 in the ipfs config addresses, and read the bound ports from the daemon output)
Orbit ALWAYS uses its own local repo
Orbit chooses its own repo address.
- either within the app bundle (flaky and prone to failure if people give each other the .app)
- or at some well-known system location (eg $user/Application\ Support on OSX)

Pros:

no weird failures
easiest to do

Cons:

does not use people's local ipfs repos for content (note: not a problem if they user's ipfs daemon is running, because in that case Orbit's ipfs node will connect to user's ipfs node and get the content locally.
Fix2: User Friendly (harder)
Be explicit about what API and what repo are being used by orbit.
- Easiest way to do this is two fields to the Orbit splash screen.
- one for Repo populated with /Users/myuser/.ipfs and a help ( ? ) that gives users a bit of necessary context for changing it.
- one for IPFS API populated with /ip4/127.0.0.1/tcp/5001 and a help ( ? ) that gives users a bit of necessary context for changing it.
- maybe a third field ipfs bin, a path to the ipfs binary used by Orbit, populated with /path/to/Orbit.app/path/to/ipfs-floodsub
Have proper error handling (no "white screen failure"), in the orbit splash screen that correctly diagnoses problems and correctly gives user solutions. Problems to diagnose include (but not limited to):
- no daemon available, and failure in starting one
- some port in use, not ipfs daemon
- some port in use, wrong ipfs version (does not support pubsub)
- no repo in right location, but failed to init
- repo in right location, but wrong version
- IMPORTANT: DO NOT migrate a repo for the user without asking!
- repo in right location, right version, but locked?

Pros:

explicit in how to use it
can be recovered
no "non starter" failures

Cons:

more complex splash UX (may be ok for this build)
have to do very complex error handling + recovery

victorb commented 7 years ago

@haadcode I'm happy to help out with these if we can make a bullet list and I'll take some of the tasks, assuming this is something that needs fixing before devcon @jbenet

ghost commented 7 years ago

does not use people's local ipfs repos for content (note: not a problem if they user's ipfs daemon is running, because in that case Orbit's ipfs node will connect to user's ipfs node and get the content locally.

this can be achieved by unconditionally connecting to /ip4/127.0.0.1/tcp/4001 -- it'll always work, no matter the mDNS weirdnesses. (did we ever test mDNS on localhost?)

haadcode commented 7 years ago

Fix2: User Friendly (harder)

We definitely have a different bar what is user friendly ;)

Valid points re. the demo-esque nature of this build. Making sure it works is not much work, mainly handling errors properly in Orbit.

Easiest way to do this is two fields to the Orbit splash screen.

Even though it's a demo, these kind of configurations don't belong in the UI. I'll see what's the best way and make it a fluid experience so that there's no user configuration required. I'll let you know when these are fixed.

dignifiedquire commented 7 years ago

Re ports in use, you could also try different ports, if one fails to bind.

haadcode commented 7 years ago

I made a commit that (latest pubsub branch):

Uses application specific IPFS data directory (Library/Application Support/orbit-floodsub-ipfs-repo)
Make sure to either pickup a running floodsub-0 daemon
Or ask the user to shutdown running (incompatible) daemons
Starts a floodsub-0 daemon if none running and binds to .../tcp/0

These steps make it behave nicely and while the user experience is not fantastic, it prevents errors, makes sure the user gets feedback and guides the user to remedy the situation.

For the future, to make this (controlling the daemon and error handling) nice, we'll have to fix things in ipfsd-ctl, there be dragons.

haadcode commented 7 years ago

Improved the path and error handling further with this commit: https://github.com/haadcode/orbit/commit/a2256d5dec2e8a2363e3d1b8d54e43b15b2e3803

victorb commented 7 years ago

@haadcode not sure if it's accidental or not, but a2256d5dec2e8a2363e3d1b8d54e43b15b2e3803 contains code outputted from build in client/dist/

haadcode commented 7 years ago

not sure if it's accidental or not, but a2256d5 contains code outputted from build in client/dist/

Not accidental. Putting the UI build in so that it's quicker to get started (instead of doing 2x npm install & npm run build).

jbenet commented 7 years ago

thanks for improving this 👍 ❤️

haadcode commented 7 years ago

Improved this further as per our discussion: Orbit now always spawns its own daemon.

jbenet commented 7 years ago

Thanks ❤️❤️❤️ On Fri, Sep 16, 2016 at 5:47 AM Haad notifications@github.com wrote:

Improved this further as per our discussion: Orbit now always spawns its own daemon.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/haadcode/orbit/issues/104#issuecomment-247560967, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIcoQLaO1Hbq15-87NF3M6Eppapc1Q9ks5qqmWpgaJpZM4J8C-k .

haadcode commented 7 years ago

I believe these issues were resolved during the time leading up to Devcon2. The fixes are in devcon2 branch and will be merged to master along with all the other improvements and new features. If you experience more "smoothness" issues, please open a new issue. Thank you all for the testing and work you did for this! <3

orbitdb-archive / orbit