Allow custom js-ipfs #5

icidasset · 2020-02-15T01:35:08Z

Adds the option:

jsIpfs: "https://unpkg.com/ipfs@latest/dist/index.min.js" // Default
jsIpfs: async () => await import("ipfs")
jsIpfs: () => Promise.resolve(Ipfs)

dholms · 2020-02-19T20:48:07Z

src/get-ipfs.ts

+
+  if (!ipfsPkg?.create) return null
+  const peers = (config.peers || []).concat(config.browserPeers || [])
+  const ipfs = await ipfsPkg.create({ config: { Addresses: { Swarm: peers }}})


Does this work? I tried locally and got Error: no valid addresses were provided for transports [WebSockets,WebRTCStar,Circuit]

Yeah, I'm getting this error as well. That's what I'm trying to figure out with the new js-ipfs. This used to work, see our Fission Drive code: https://github.com/fission-suite/drive/blob/22468f5908b7715bf516c87debf05ae36c974588/src/Javascript/ipfs.js#L32-L34

Could it be something regarding "delegated routing" which they excluded now? https://blog.ipfs.io/2020-02-13-js-ipfs-0-41/ (search for "Delegated peer and content routing")

Are we doing delegated routing with us passing the peer id in the multiaddr? (eg. /dns4/node.fission.systems/tcp/4003/wss/ipfs/QmVLEz2SxoNiFnuyLpbXsH6SvjPTrHNMU88vCQZyhgBzgw)

I... don't think so? Not very familiar with that. But delegated routing seems to be "okay I have this peer id, but not the full multiaddr, can you connect me?"

It's actually surprising to me that that worked in the Fission Drive code. I thought I remembered trying this back when originally making this lib and it not working. I either had to use ipfs.swarm.connect() or put the addresses in config.Bootstrap

Huh weird. Ok, I'll do some more digging because this is all very confusing 😅

Awesome thanks 👍 I did notice you added a tries: number param, but never used it. Do you want me to implement this? Also, no timeout/delay between retries?

Quick code edits squeezed in before team meetings come back to bite me 😛

Yes I'll fix that haha

And the timeout doesn't seem to be necessary. It either requires multiple connect attempts or a timeout of about 200ms 🤷‍♂️

I'll add it as an additional param though in case we want to use it

😬 😄 Ok cool. So weird these connection issues.

One last thing, I'm getting an error on safari when using the latest js-ipfs. Could you check if you get that too with ipfs-user-settings? 🙏 I'm getting a timeoutController.clear is not a function error. Made an issue for that on the libp2p repo, but trying to get more info.

okay pushed the new code 👍

I'm on linux so unfortunately don't have Safari 😬 but I don't get any such error in FF/Chrome

Ah right. Carry on! 😬 😅

src/get-ipfs.ts

icidasset · 2020-02-20T21:15:54Z

@dholms How do I use this semantic-release thing you've setup? Are we generating a changelog based on the git commits using this?

dholms · 2020-02-21T04:46:52Z

ah semantic-release is actually left over from the boilerplate I used to set this library up. It included some tooling that we haven't been using. I did get rid of it with our typescript-boilerplate library

So yeah just ignore it for now. There's a little more cleanup I need to do on the tooling as well. I'll make an issue for it

icidasset · 2020-02-21T15:28:48Z

Cool, sounds good 👍 I've bumped the version and added a changelog.

dholms

Alright I'm gonna give this an approval because it seems like this is working for both of us with the retry logic.

If you could take a quick look at the code I added before merging, that'd be great 👍

I'm also going to create a separate issue about the AggregateError for further review, but I don't think that needs to block us from merging this

wip

8c1d2e2

icidasset added the 💗 enhancement New feature or request label Feb 15, 2020

icidasset added 2 commits February 18, 2020 22:58

multiaddr

0fb4794

Remove multiaddr

019ae82

icidasset requested a review from dholms February 19, 2020 18:00

icidasset added 2 commits February 19, 2020 19:05

Remove bootstrap config

96fca84

Remove multiaddr types dependency

54fc373

dholms reviewed Feb 19, 2020

View reviewed changes

src/get-ipfs.ts Outdated Show resolved Hide resolved

dholms reviewed Feb 19, 2020

View reviewed changes

src/get-ipfs.ts Show resolved Hide resolved

dholms reviewed Feb 19, 2020

View reviewed changes

src/get-ipfs.ts Outdated Show resolved Hide resolved

icidasset added 4 commits February 19, 2020 22:49

Revert connect setup

b768429

Improve "connect to" message

62070a6

Improve connect peers list

de844bc

Fix linting errors

b97181b

Bump version and add CHANGELOG

39522c4

dholms added 2 commits February 21, 2020 11:56

Retry failed peer connections

61bf090

add params for number of peer connect retries + timeout

d3b9e60

dholms approved these changes Feb 21, 2020

View reviewed changes

dholms mentioned this pull request Feb 21, 2020

AggregateError on first connect #7

Open

icidasset added 2 commits February 24, 2020 21:53

Refactor and expose connectPeers

a37a007

It should compile though

d1cac78

icidasset merged commit 9ac5dbd into master Feb 24, 2020

icidasset deleted the custom-url branch February 24, 2020 21:05

Allow custom js-ipfs #5

Allow custom js-ipfs #5

Uh oh!

Conversation

icidasset commented Feb 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

icidasset commented Feb 20, 2020

Uh oh!

dholms commented Feb 21, 2020

Uh oh!

icidasset commented Feb 21, 2020

Uh oh!

dholms left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

icidasset commented Feb 15, 2020 •

edited

Loading