Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Disconnected event gets incorrect values #103

Open
DmitriySalnikov opened this issue Sep 7, 2022 · 6 comments
Open

[BUG] Disconnected event gets incorrect values #103

DmitriySalnikov opened this issue Sep 7, 2022 · 6 comments

Comments

@DmitriySalnikov
Copy link

Issue Type

  • Bug

Describe the bug
The Disconnected event returns only values from WebSocketCloseStatus (1000-1011) and marks them as ObsCloseCodes. Whereas the OBS protocol returns the values 0, 4000, 4002, 4003-4012.

To Reproduce
Steps to reproduce the behavior:

  1. Connect to OBS
  2. Place the breakpoint on OBSWebsocket.OnWebsocketDisconnect()
  3. Close OBS
  4. Check the variable d.CloseStatus

Expected behavior
Probably I would like to get either a combined enum from the OBS protocol and the WebSocket enum, or get some kind of remap of WebSocket values to the values from the OBS protocol enum.

Screenshots
image

image

Versions
OBS Version: 28.0.1
OBS WebSocket Version:
OBS WebSocket Dotnet (this library) Version: 257e7b4
OS: Windows 11

@BarRaider
Copy link
Owner

I'm not sure what exactly is the problem you're mentioning. It definitely does return codes 4000+ as we use it to determine Auth failure

Can you make a small sample code replicating the issue?

@DmitriySalnikov
Copy link
Author

DmitriySalnikov commented Sep 7, 2022

I got error 1001, as in the screenshot, using the original TestClient project. If the error was in the range of 4000-4012, then it would be displayed in text, not numbers..

image
image

Steps to reproduce the behavior:

  • Connect to OBS using TestClient
  • (optional) Place the breakpoint on OBSWebsocket.OnWebsocketDisconnect() to check the original value of WebSocketCloseStatus
  • Close OBS
  • A MessageBox will appear with CloseCode: 1001 or check the variable closeCode in debugger

image

@BarRaider
Copy link
Owner

1000 is not a OBS Close Code so it won't return an enum value.
You can see the valid codes here:
https://github.com/obsproject/obs-websocket/blob/master/docs/generated/protocol.md#websocketopcode

The other codes are internal Websocket codes

@DmitriySalnikov
Copy link
Author

DmitriySalnikov commented Sep 7, 2022

The other codes are internal Websocket codes

Yes, I understood that. It's just not documented.
It seemed to me that this could be done more explicitly.

At the moment I have to do something like this?

if ((int)e.ObsCloseCode < 4000)
{
    var ee = (System.Net.WebSockets.WebSocketCloseStatus)e.ObsCloseCode;
    switch (ee)
    {
        case System.Net.WebSockets.WebSocketCloseStatus.NormalClosure:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.EndpointUnavailable:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.ProtocolError:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.InvalidMessageType:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.Empty:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.InvalidPayloadData:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.PolicyViolation:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.MessageTooBig:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.MandatoryExtension:
            break;
        case System.Net.WebSockets.WebSocketCloseStatus.InternalServerError:
            break;
    }
}
else
{
    switch (e.ObsCloseCode)
    {
        case ObsCloseCodes.DontClose:
            break;
        case ObsCloseCodes.UnknownReason:
            break;
        case ObsCloseCodes.MessageDecodeError:
            break;
        case ObsCloseCodes.MissingDataField:
            break;
        case ObsCloseCodes.InvalidDataFieldType:
            break;
        case ObsCloseCodes.InvalidDataFieldValue:
            break;
        case ObsCloseCodes.UnknownOpCode:
            break;
        case ObsCloseCodes.NotIdentified:
            break;
        case ObsCloseCodes.AlreadyIdentified:
            break;
        case ObsCloseCodes.AuthenticationFailed:
            break;
        case ObsCloseCodes.UnsupportedRpcVersion:
            break;
        case ObsCloseCodes.SessionInvalidated:
            break;
        case ObsCloseCodes.UnsupportedFeature:
            break;
    }
}

If the answer is yes, then I think you can close this question.

@ProbablePrime
Copy link

I too am seeing 1000, 1001 etc coming in this handler from e.ObsCloseCode

In my case, connect to OBS, then force close OBS -> 1001.

You asked for a reproduction:

using OBSWebsocketDotNet;
using OBSWebsocketDotNet.Communication;

namespace OBSWebsocketTest
{
    public class Program
    {
        public static Task Main(string[] args) => new Program().MainAsync();
        public async Task MainAsync()
        {
            OBSWebsocket obs = new OBSWebsocket();

            obs.Connected += Obs_Connected;
            obs.Disconnected += Obs_Disconnected;
            obs.ConnectAsync("ws://127.0.0.1:4455", "");

            // Block this task until the program is closed.
            await Task.Delay(-1);
        }
        void Obs_Connected(object? sender, EventArgs e)
        {
            Console.WriteLine("Connected");
        }

        void Obs_Disconnected(object? sender, ObsDisconnectionInfo e)
        {
            Console.WriteLine(e.ObsCloseCode);
        }
  

Running this, with OBS open, waiting till you see connected and then closing OBS will result in a 1001 close code.

I think the original poster is saying that the additional codes(<4000) are not documented in the library and that the type(ObsCloseCodes) does not handle them. This leads to a confusing experience when consuming this client.

I guess that when I consume a client application, I expect an Enum value to not contain values that are not defined in the Enum.

If I were writing this library I would probably either:

  1. Extends ObsCloseCodes from another Enum type that does contain codes <4000
  2. Change the handling/invocation of the disconnected handler to not send <4000 codes via the ObsCloseCodes type.

In my case, I can handle this with some extra code similar to the original poster, but I do agree with them it is confusing.

@BarRaider
Copy link
Owner

I will address this moving forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants