[BUG] websocket on compress close duplicate

[t0jc]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

in this line

websocket/compression.go

Line 138 in ac0789b

if err := r.Close(); err != nil {

and this line

websocket/conn.go

Line 1024 in ac0789b

if err := c.reader.Close(); err != nil {

Expected Behavior

if close duplicate will report this log

2023/10/30 21:56:02 websocket: discarding reader close error: io: read/write on closed pipe

Steps To Reproduce

1. create websocket link with "EnableCompression"
2. use this websocket write someting
3. in server side got log "websocket: discarding reader close error: io: read/write on closed pipe"

Anything else?

2023/10/30 21:56:02 websocket: discarding reader close error: io: read/write on closed pipe

[DVKunion]

the same +1, any solutions or temporary repair suggestions? :)

[davidnewhall]

Disabling compression does not get rid of the error. The error seems triggered by keep alives because it happens without any other requests. I sure wish I could pass in a custom logger for this so I can tune the error and figure out which thread it comes from.

[jaitaiwan]

What would really help with moving forward with this bug is having reproduction steps. Then we can just run a go debugger and poke at the stacks.

[davidnewhall]

Sounds a bit beyond my skill, but I'd be happy to work with someone to figure this out.

I have an app that reproduces this error consistently. I have thousands of users, and I'd like to get this figured out before I release an app update with v1.5.1 of this library (which adds the new log message). If you download the app, you'll also need a (free) account at https://notifiarr.com. There's no (email or anything) verification so you can remain 100% anonymous there.

I'll try to get a reproducible setup using just mulery. Notifiarr users this code for the web socket, and it's much leaner. I'm 100% expecting the "bug" to be in my code, but I really don't know how to find it.

Looks like this error print has been removed. That's interesting and makes me think we're overlooking a bug somewhere.

[ghost]

This issue (along with others) was introduced in commit 666c197. The commit message gives no reason for the change. Fix by reverting the changes around the lines of code linked in the issue.

[jaitaiwan]

Thanks for the feedback folks. We recognise that we made a mistake in merging some of these changes and we're working on getting new PRs written and reviewed to fix this going forward.

[DVKunion]

Disabling compression does not get rid of the error. The error seems triggered by keep alives because it happens without any other requests. I sure wish I could pass in a custom logger for this so I can tune the error and figure out which thread it comes from.

Yes, this is just a temporary solution. I have abandoned comrpress to ensure that my logs have no erroneous outputs.If there is a better fix method, please remind me :)

[elepedus]

Did anyone manage to resolve this issue? We're using the library to proxy websocket connections, and would love a fix 🙏

[jaitaiwan]

We’ve done a revert and plan to do a temporary release to cover things I believe .

[elepedus]

Oh awesome! When do you expect to release the fixed version?

[jaitaiwan]

We’re talking about internally how to version it, once the decision is made we’ll do it straight away. In the meantime master represents that future version.

[elepedus]

Thank you! 🙏

[conbanwa]

same issue on @1.5.1, and is was just ok on @1.5.0 version

[hulkingshtick]

Was this fixed? There are no calls to the log package on the main branch.

[jaitaiwan]

I believe so - v1.5.3 should be safe now.