Check err for nil before taking error

[ineiti]

When checking err.Error() for a value, the code panics if err is nil.

Closes #2865

[sukunrt]

Interesting. Can you point me to some code where you're using this?

The problem here is closing l.nl after closing l.Server. If you called WebsocketTransport.Listen it'd have eventually called the serve method. After calling serve, the second close l.nl.Close() should always return an error.

Anyway,
The correct thing to do here is to first do l.nl.Close() then do l.Server.Close and to not return transport.ErrListenerClosed when calling Close. It's meant for the Accept method.

I'm happy to take this change up if you don't have the time to make the changes to your PR.

[ineiti]

Thanks for answering. The problem was that I couldn't find a specific Test where it happened. It was only happening when doing tests in parallel with other, non-libp2p network tests, in (some of) the following tests:

https://github.com/c4dt/dela/tree/minows/mino/minows

So, something along the lines of:

git clone https://github.com/c4dt/dela -b minows
cd dela/mino
go test ./... -count=1

On my Mac M2, it never failed. On the github runners it failed very often. It was very Heisenbug-like: the more tests in mino/minows/*test.go I t.Skip()ed, the rarer the error got. Sometimes it all went well, sometimes it panicked.

I also tried to follow your instructions to invert the two Close. But then I saw that the l.server.Close() also returns an error which is not taken into account, and I have no idea how to handle that. So if you can take care of that, I would be very happy :)

[sukunrt]

But then I saw that the l.server.Close() also returns an error which is not taken into account, and I have no idea how to handle that. So if you can take care of that, I would be very happy :)

We can take the error from both the close calls.

Closing this in favor of #2867