search

andreykaipov / goobs

Go client library for OBS Studio
Apache License 2.0
137 stars 22 forks source link

Update client.go #62

Closed ddaggHT closed 11 months ago

ddaggHT commented 11 months ago

whenever there is an error technically need to return, if we don't can get caught in a loop and have gorilla WebSockets library panic.

afriza commented 11 months ago

@ddaggHT Do you mind sharing the type and value (t and err) from t := err.(type) that cause the loop/panic?

andreykaipov commented 11 months ago

Hi @ddaggHT yeah I'm not sure I quite follow this. Could you provide a reproduction?

If it's an error that has to return early, we should put it in its own case clause like the error above.

I'd rather defer unknown errors because I think it's easier finding all the errors we have to return early from versus all the ones we're safe to continue from, e.g. if the message is invalid JSON for some reason.

andreykaipov commented 11 months ago

I added additional checks to return early here on websocket.ErrCloseSent:

https://github.com/andreykaipov/goobs/blob/bd628d3309ba36ab4527e9af7aa555312a6a3618/client.go#L274-L281

I ran into panic: repeated read on failed websocket connection for concurrent clients often after I upgraded gorilla/websocket to v1.5.1 in https://github.com/andreykaipov/goobs/pull/64. Oddly enough I wasn't able to reproduce it at all with gorilla/websocket v1.5.0, but I also don't see any relevant changes between the two versions, so I wonder if this is the same panic you ran into too.

If it isn't please open up a new issue with the details!

ddaggHT commented 11 months ago

Hey @andreykaipov yes that is the same issue, sorry I've been away. Looking at PR #80 that should do some good 🙂