Closed p-shahi closed 1 year ago
@marten-seemann yes this is particularly useful when debugging on multiple networks or performing upgrades where we upgrade the protocolID.
That doesn't answer my question. You can easily annotate the error yourself:
str, err := host.NewStream(ctx, peerID, proto)
if err != nil {
return fmt.Errorf("failed to open stream with proto %s: %w", proto, err)
}
Adding two cents here, @marten-seemann.
This is totally valid argument when you open or handle a new stream.
You know which protocol(s) you were trying to negotiate when you receive this error.
However, in the system, when a high-level operation consisting of multiple subprotocols fails, the error "protocol not supported" requires going over each protocol to understand where it's coming from. Of course, one could wrap the error in each protocol with its name, but it's still better to do it in one place. Also, add on the top @renaynay's point above multiple different networks, which are appended as suffixes to the protocol id. In the end, debugging something like this starts to be painful and simple mitigation is to add the protocol name.
That's a fair point. Want to submit a PR?
We should probably introduce an error type here, so you can get the list of protocols by using an error assertion. wdyt?
type ErrNotSupport struct {
Protos []string
}
@sukunrt are you interested in picking this up (other folks lmk if you've already started work on this)? This would be a good quality of life change!
Yes I'd like to pick this up.
Here we need to change ErrNotSupported
response to use a new type
type ErrNotSupport struct {
Protos []string
}
which will include all the protocols which were not supported.
This change will only impact client.go with methods like SelectOneOf
, right?
I also see one usage in doReadHandshake
, what about that one?
Here we need to change
ErrNotSupported
response to use a new typetype ErrNotSupport struct { Protos []string }
This would also need to be generic, right?
Yes. doReadHandshake will change.
Agreed, ErrNotSupport should be generic otherwise it'll be a lot of string(proto).
type ErrNotSupport struct {
Protos []string
}```
@marten-seemann This type can be named ```ErrNotSupported[T]```, right?
Migrating https://github.com/libp2p/go-libp2p/issues/1988