js-libp2p stream

stream.js

const ROLE = {
  INITIATOR: 0,
  RESPONDER: 1
}

class Stream {
  constructor(iterableDuplex, connection, isInitiator = true) {
    /**
     * Stream identifier
     */
    this.id = (~~(Math.random() * 1e9)).toString(36) + Date.now()

    /** 
     * Streaming iterable duplex object
     */
    this._iterableDuplex = iterableDuplex

    /**
     * Stream parent connection
     */
    this.connection = connection

    /**
     * Role in the stream, initiator or responder
     */
    this.role = isInitiator ? ROLE.INITIATOR : ROLE.RESPONDER

    /** 
     * Stream timeline
     */
    this.timeline = {
      openTs: Date.now(),
      closeTs: undefined
    }

    /**
     * User provided tags
     */
    this.tags = []

    /** 
     * Stream protocol
     */
    this._protocol = undefined
  }

  sink () {

  }

  source () {

  }

  close () {

  }
}

We should only call the Stream "stream", otherwise we risk the same "Connection" naming nightmare, I'd rather see something like this._iterableDuplex = iterableDuplex

Great, I will change that!

What's your thought behind protocol being in the constructor? When we create a stream we likely won't know the protocol. We won't know it until multistream-select has run on that stream and negotiate a protocol. We could set it then.

I am not familiar with the multistream-select, but when we open a stream or register a stream handler (in the libp2p-daemon-client for instance) we define the protocol we want to use. Is there something I am not following?

What are the tags for? Tagging connections is meaningful as we can say this is a Bootstrap, DHT, etc connection. Do we get value from tagging each stream?

During the discussions for the introspection/visualization tool, we discussed that it would be beneficial for users of libp2p, such as ipfs, to open streams and add tags to them, which will allow filtering by those tags to decrease the debugging time.

Should we add a reference to the streams parent connection? It could make connection and stream cleanup a bit easier, rather than having to go back down the libp2p stack to get the connection.

👍

Do you have any suggestion for computing a connectionId and streamId?

I am not familiar with the multistream-select, but when we open a stream or register a stream handler (in the libp2p-daemon-client for instance) we define the protocol we want to use. Is there something I am not following?

Correct, but this is at a higher level. The flow of this might look like:

const stream = connection.newStream()
// `Connection.newStream`
newStream (protocol) {
  // Create the new stream from the multiplexer, it shouldn't care about protocols
  const stream = this.multiplexer.newStream()  // stream.protocol == null
  this.streams.push(stream)
  if (!protocol) {
    // If no protocol was provided, we could just return the stream for the user to manage
    return stream
  }
  // Create a new instance of multistream to handle protocol selection
  const multistream = new Multistream.Dialer()
  // Negotiate the multistream protocol
  await multistream.handle(stream)
  // Handshake on the desired `protocol`, this should set the protocol on the stream
  await multistream.select(protocol) // stream.protocol == protocol
  // Return the stream
  return stream
}

So while the user doesn't care, if we are returning a Stream from the multiplexer, it won't know about the protocol yet. I haven't thought through the multistream api too much yet, so that could potentially be improved as well.

Do you have any suggestion for computing a connectionId and streamId?

We could just use the same logic webrtc-star uses for intent ids. The timestamp, plus the random string should have a pretty reliable non collision rate.

const id = (~~(Math.random() * 1e9)).toString(36) + Date.now()

Thanks for the feedback @jacobheun , I now agree with you regarding the protocol!

Updated the gist code according to the suggestions

this.connectionId = connectionId

Any reason to not just make this the actual connection reference? Just thinking about usability here. If I have a stream and want to get the connection, I'll have to take the connectionId and then look up the connection via the Switch. I don't think it's critical, just a potential usability improvement.

No reason, I will change to the connection reference!

@alanshaw do you envision sink and source as you have in js-libp2p-websockets?

https://github.com/libp2p/js-libp2p-websockets/pull/82/files#diff-1fdf421c05c1140f6d71444ea2b27638R47

In this case, we need to pass the options to this stream class for the abortable, and in newStream for the connection.