websocket icon indicating copy to clipboard operation
websocket copied to clipboard

Use net.ErrClosed

Open emersion opened this issue 4 years ago • 13 comments

Go 1.16 has introduced net.ErrClosed, which should be returned/wrapped when an I/O call is performed on a network connection which has already been closed. This is useful to avoid cluttering logs with messages like "failed to close WebSocket: already wrote close".

Closes: https://github.com/nhooyr/websocket/issues/286

emersion avatar May 19 '21 21:05 emersion

Like!

Antonboom avatar May 20 '21 07:05 Antonboom

Hi @nhooyr, any chance to get this reviewed? Thanks!

emersion avatar Jun 01 '21 08:06 emersion

Ping @nhooyr, would you have the time to have a look?

emersion avatar Jun 11 '21 08:06 emersion

Sorry for the delay I'm on a rather long sabbatical right now. Will review when I'm back by the end of this month!

nhooyr avatar Jun 17 '21 04:06 nhooyr

Cool! No problem, take your time :)

emersion avatar Jun 17 '21 08:06 emersion

Any news?

Antonboom avatar Aug 31 '21 06:08 Antonboom

Hi again @nhooyr, any chance to get this reviewed?

emersion avatar Sep 13 '21 12:09 emersion

One is missing a !.

emersion avatar Sep 13 '21 15:09 emersion

Hi again @nhooyr, any chance to get this reviewed?

Sorry @emersion my sabbatical continues until at least mid October. Apologies for the delay and the broken promise above but I'm dealing with some personal stuff right now and was hoping to be back sooner.

I could merge this as is into master as I don't see anything wrong with it but I can't do any sort of release without dealing with #256 first which will take substantial effort. So I'd prefer to leave this PR open as is until I'm back.

nhooyr avatar Sep 19 '21 13:09 nhooyr

Apologies for the delay and the broken promise above but I'm dealing with some personal stuff right now and was hoping to be back sooner.

No problem, thanks for having a look.

I could merge this as is into master as I don't see anything wrong with it but I can't do any sort of release without dealing with #256 first which will take substantial effort. So I'd prefer to leave this PR open as is until I'm back.

I'd personally prefer for this to be merged so that I can upgrade to the latest commit even if there's no release yet. But up to you!

emersion avatar Sep 19 '21 13:09 emersion

Hi @nhooyr, is it possible to merge this?

emersion avatar Oct 29 '21 13:10 emersion

Gentle ping

emersion avatar Nov 17 '21 13:11 emersion

@nhooyr, any chance to get this merged?

emersion avatar Mar 21 '22 09:03 emersion

Hi, any news about this?

emersion avatar Feb 18 '23 17:02 emersion

Hm, why is that? net.Conn returns net.ErrClosed when attempting to write or close an already-closed connection.

emersion avatar Mar 07 '23 15:03 emersion

Hm, why is that? net.Conn returns net.ErrClosed when attempting to write or close an already-closed connection.

Neither of those are closed connections, they're readers and writers for a single frame. There's only the one spot in close_notjs.go where it's correctly used when the connection is already closed.

nhooyr avatar Mar 07 '23 15:03 nhooyr

Ah, I see, that makes sense! Fixed.

emersion avatar Mar 07 '23 15:03 emersion

+1 to this PR :)

rslobodian avatar Mar 22 '23 20:03 rslobodian

Can this thing go any further? Really needs it. At least the original error should be exported. 😥

AuroraTea avatar Aug 25 '23 13:08 AuroraTea

Sorry for the delays, thanks @emersion

nhooyr avatar Oct 13 '23 05:10 nhooyr