Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

[WIP] Move to pull-streams #18

Merged
merged 3 commits into from
Sep 6, 2016
Merged

[WIP] Move to pull-streams #18

merged 3 commits into from
Sep 6, 2016

Conversation

dignifiedquire
Copy link
Member

@dignifiedquire dignifiedquire commented Aug 11, 2016

Investigate delay. Ref pull-stream/pull-ws#13

if (typeof options === 'function') {
callback = options
cb = options
Copy link
Member

Choose a reason for hiding this comment

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

Let's continue to use callback for top level callback and cb for internals (like run-series cb)

Copy link
Member Author

@dignifiedquire dignifiedquire Aug 18, 2016

Choose a reason for hiding this comment

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

No description provided.

@daviddias
Copy link
Member

@whyrusleeping @jbenet Would like your thoughts on a extension to WebSockets to enable halfClosed pipes - pull-stream/pull-ws#13 (comment), it might just be a pattern we need to apply to other transports anyway.

return cb(null, [ma])
}

const conn = new Connection(socket)
Copy link
Member

Choose a reason for hiding this comment

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

@dignifiedquire
Copy link
Member Author

@diasdavid as long as we are using something on top, like a stream muxer that has an end handshake we should be fine. At least as far as I understand

@daviddias
Copy link
Member

daviddias commented Aug 18, 2016

@dignifiedquire not quite, because spdy also has a closing ceremony (it believes it is on top of TCP), so it will call .end/.close, and not wait to receive the signal to close graciously, causing the same problem.

@dignifiedquire
Copy link
Member Author

😢

cb(null, [listeningMultiaddr])
}

return listener
Copy link
Member

@daviddias daviddias Sep 1, 2016

Choose a reason for hiding this comment

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

It is missing to bubble up the events from the listener:

  • event: 'listening'
  • event: 'close'
  • event: 'connection'
  • event: 'error'

See interface-transport.

  • Add tests for the above

@daviddias
Copy link
Member

daviddias commented Sep 1, 2016

Last things to do:

@dignifiedquire
Copy link
Member Author

Can't websockets dial timeout? If yes, we need to cb(err)

no timeout by default as far as I know on dial

@daviddias
Copy link
Member

interface-transport tests also fail here :( @dignifiedquire

@dignifiedquire
Copy link
Member Author

interface-transport tests also fail here

CI looks pretty green to me after I gave circle a non cache kick

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