Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

Be the Switch 🌟 #242

Merged
merged 18 commits into from
Feb 7, 2018
Merged

Be the Switch 🌟 #242

merged 18 commits into from
Feb 7, 2018

Conversation

daviddias
Copy link
Member

No description provided.

@daviddias daviddias requested review from dryajov and pgte February 6, 2018 10:11
@ghost ghost assigned daviddias Feb 6, 2018
@ghost ghost added the in progress label Feb 6, 2018
@daviddias
Copy link
Member Author

This PR revealed some issues that happen just on Linux, investigating.

  1) Switch (everything all together) close a muxer emits event:

     Uncaught AssertionError: expected true to not exist

      at conn.getPeerInfo (test/swarm-muxing.node.js:187:28)

      at node_modules/async/internal/once.js:12:16

      at next (node_modules/async/waterfall.js:21:29)

      at node_modules/async/internal/onlyOnce.js:12:16

      at f (node_modules/once/once.js:25:25)

      at select (node_modules/multistream-select/src/dialer/index.js:38:16)

      at pullLP.decodeFromReader (node_modules/multistream-select/src/select.js:20:14)

      at Object.reader.read [as cb] (node_modules/pull-length-prefixed/src/decode.js:88:16)

      at drain (node_modules/pull-reader/index.js:42:23)

      at node_modules/pull-reader/index.js:59:18

      at node_modules/pull-reader/index.js:20:7

      at drain (node_modules/stream-to-pull-stream/index.js:141:18)

      at Stream.<anonymous> (node_modules/stream-to-pull-stream/index.js:162:5)

      at node_modules/spdy-transport/lib/spdy-transport/stream.js:654:10

      at _combinedTickCallback (internal/process/next_tick.js:131:7)

      at process._tickCallback (internal/process/next_tick.js:180:9)


  2) Switch (everything all together) "after all" hook:

     Uncaught TypeError: Cannot set property 'state' of undefined

      at node_modules/async/internal/parallel.js:39:9

      at node_modules/async/internal/once.js:12:16

      at iteratorCallback (node_modules/async/eachOf.js:60:13)

      at node_modules/async/internal/onlyOnce.js:12:16

      at node_modules/async/internal/parallel.js:36:13

      at node_modules/async/internal/parallel.js:39:9

      at node_modules/async/internal/once.js:12:16

      at replenish (node_modules/async/internal/eachOfLimit.js:59:25)

      at iterateeCallback (node_modules/async/internal/eachOfLimit.js:49:17)

      at node_modules/async/internal/onlyOnce.js:12:16

      at node_modules/async/internal/parallel.js:36:13

      at node_modules/async/internal/once.js:12:16

      at iterateeCallback (node_modules/async/internal/eachOfLimit.js:47:24)

      at node_modules/async/internal/onlyOnce.js:12:16

      at node_modules/async/internal/once.js:12:16

      at Server.iteratorCallback (node_modules/async/eachOf.js:60:13)

      at Server.<anonymous> (node_modules/async/internal/onlyOnce.js:12:16)

      at emitCloseNT (net.js:1655:8)

      at _combinedTickCallback (internal/process/next_tick.js:135:11)

      at process._tickCallback (internal/process/next_tick.js:180:9)


  3) circuit listed on the transports map:

     AssertionError: expected undefined to exist

      at Context.it (test/circuit-relay.node.js:66:45)

@daviddias daviddias force-pushed the feat/be-a-switch branch 2 times, most recently from d0d5493 to f6d5924 Compare February 6, 2018 11:23
@daviddias daviddias removed request for pgte and dryajov February 6, 2018 11:25
@daviddias
Copy link
Member Author

@dryajov any clue on the Circuit test fail?

@daviddias
Copy link
Member Author

Got it! 😃

image

@daviddias daviddias requested review from dryajov and pgte February 6, 2018 11:35
@daviddias
Copy link
Member Author

@pgte @dryajov mind reviewing?

switchA = new Switch(peerA, new PeerBook())

switchA.transport.add('ws', new WebSockets())
switchA.transport.listen('ws', {}, echo, cb)
Copy link
Member

Choose a reason for hiding this comment

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

this is probably correct, but just to bring up the question, why no stream muxer here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is for the transport only tests

@daviddias daviddias merged commit 5573289 into master Feb 7, 2018
@ghost ghost removed the in progress label Feb 7, 2018
@daviddias daviddias deleted the feat/be-a-switch branch February 7, 2018 05:33
@daviddias
Copy link
Member Author

@victorbjelkholm @dryajov related to ipfs/interop#2, see how this module was refactored and one of the steps were dedup of tests. My point here is that the door is open for improvement, it was never locked :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants