Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

update package.json #198

Merged
merged 1 commit into from
May 8, 2016
Merged

update package.json #198

merged 1 commit into from
May 8, 2016

Conversation

daviddias
Copy link
Member

Created this PR to make travis run again on clean master to check the errors that @nginnever mentioned

@jbenet jbenet added the status/in-progress In progress label May 6, 2016
@daviddias
Copy link
Member Author

@daviddias daviddias added the kind/bug A bug in existing code (including security flaws) label May 6, 2016
@daviddias
Copy link
Member Author

@dignifiedquire you might want to take a look. This was introduced with the update on libp2p-swarm: libp2p/js-libp2p-switch@85a0647

Probably the problem was always there, but now it is really easy to see

@daviddias
Copy link
Member Author

18:57 daviddias: everything before this commit is passing for me
18:57 1251dea

@nginnever
Copy link
Member

nginnever commented May 6, 2016

18:57 daviddias: everything before this commit is passing for me
18:57 1251dea

I take that back, I started hard reseting commits again one at a time and the commit before this one threw the spdy error as well.

@daviddias
Copy link
Member Author

@nginnever wanna check with libp2p-swarm 0.12.5 ?

@nginnever
Copy link
Member

Downloaded the archived libp2p-swarm v0.12.5 and npm linked it to the js-ipfs repos, seems to fix the issue.

@nginnever
Copy link
Member

Changed libp2p-swarm to throw transport close errors

async.each(
  Object.keys(this.transports),
  (key, cb) => this.transports[key].close(cb),
  callback
)

and received

Error: there are no listeners

In js-ipfs libp2p core swarm.close

@daviddias daviddias force-pushed the smoke-test branch 2 times, most recently from 5a5b064 to 8b70d9d Compare May 7, 2016 00:08
@@ -27,7 +26,8 @@ function IPFS (repoInstance) {
throw new Error('Must be instantiated with new')
}

if (!(repoInstance instanceof IPFSRepo)) {
if (typeof repoInstance === 'string' ||
repoInstance === undefined) {
Copy link
Member Author

@daviddias daviddias May 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed that on Travis, the "isDaemonOn" always yielded true, making the 'cli offline' tests work online. Checking if this was the issue.

Good best practice, never do instanceof for 'types' outside the scope of the module, cause different versions will have different signatures and you never know which version will be thrown at the func.
@nginnever @dignifiedquire

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it fixed the 'weird fails' on the block tests : )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put these changes into the files PR.

@daviddias
Copy link
Member Author

Hopefully, this will give us a better answer spdy-http2/spdy-transport#26 (probably the right answer is the one that is most simply, we need to check for .on('error') in every muxer we create

@daviddias
Copy link
Member Author

GREEN :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants