NewStream should block until connection is reversed instead of returning ErrTransientConn

[Jorropo]

Currently there is a weird logic race bug happening when trying to open a stream to a peer while hole punching is still in flight.
NewStream returns the more than unhelpfull ErrTransientConn, I think this is a bug because WTF am I supposed to do with that ? Let's assume I am writing a random protocol, I don't really care that the current connection is transient, what will I do with this information anyway ?

I think a saner option to wait until the direct connection is established.

The best solution afait is to add pooling around newstream:

var s network.Stream
for s == nil {
  var err error
  s, err = h.NewStream(<args>)
  switch err {
  case nil:
  case errors.ErrTransientConn:
    time.Sleep(1 * time.Second) // exponential backoff ?
  default:
    return err
  }
}

That code is bad for lots and lots of reasons: latency deficiencies because the pooling ins't continuous, wasting CPU cycles, knowing when to stop, really ugly code, ...

The behaviour I expect is that if .NewStream (assuming WithUseTransient is not used) would block until the hole punch either fails or succeeds, if it succeeds a stream on the new holepunched connection is returned (/ attempted, things might fail later when opening a new stream there), if holepunching fails an error is returned instead, maybe wrapping ErrTransientConn explains why the holepunch fails. (IMO ErrTransientConn wouldn't need to exists at this point and should be removed.)
If WithUseTransient is used, then it would try to open a stream on the relayed connection as current.

It's really easy to picture how this could be implemented with a channel select avoiding the inefficient polling (because if NewStream's context is canceled before the hole punch finishes I expect ctx.Err() to be returned).

[marten-seemann]

Duplicate of #1603?

[Jorropo]

@marten-seemann it looks like, remediation seems a bit different but effects are the same.