go-multistream icon indicating copy to clipboard operation
go-multistream copied to clipboard

[RFC] remove handler func support

Open Stebalien opened this issue 8 years ago • 4 comments

  1. It never actually worked (we always used the text match).
  2. It made it impossible to accurately list protocols (required by the protocol).

reverts c9587f16af873eced8fd29ea66c9c777d5c35d09

Alternatively (I'd actually prefer this) we should consider removing the "ls" command. That's not the job of this protocol (should use, e.g., an identify service).

Stebalien avatar Dec 29 '17 01:12 Stebalien

@diasdavid thoughts? (FYI, by "it never worked", I mean "it never worked in go"). It looks like js-libp2p does support this but I'd still like to go one way or another. Either support listing supported protocols or don't support listing supported protocols.

Stebalien avatar Dec 29 '17 01:12 Stebalien

Coverage Status

Coverage decreased (-1.2%) to 78.272% when pulling cd4d4051ca6d36ebcda7bb3a0c61c742511f8df4 on feat/remove-handler-func into b8f1996688ab586031517919b49b1967fca8d5d9 on master.

coveralls avatar Dec 29 '17 01:12 coveralls

To make this clear, we have two conflicting features:

  1. We have ls to list all supported protocols.
  2. We allow registering dynamic protocol handlers.
  • The first feature implies that we must know all supported protocols up-front.
  • The second feature prevents us from knowing all protocols up-front.

There are three ways of fixing this:

  1. Removing the dynamic protocol feature (this PR).
  2. Remove the ls protocol.
  3. Redefine the ls protocol to only list known protocols.

I filed this PR because the dynamic protocol feature is currently broken in go (and has never worked).

Stebalien avatar Jan 05 '18 21:01 Stebalien

So... It turns out that the ls protocol is also broken in go.

  1. NegotiateLazy doesn't implement it (it "lists" no protocols).
  2. Negotiate writes back the protocol list but doesn't acknowledge the ls protocol first (that is, it just sends back the list of supported protocols without sending back <len>ls\n first.

😭

Stebalien avatar Jan 09 '18 20:01 Stebalien