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

Websocket event "connection_ack" with "payload" as null result into closed connection #450

Closed
guilhermeavanci opened this issue Feb 24, 2023 · 3 comments · Fixed by #456
Closed
Labels
enhancement New feature or request released Has been released and published

Comments

@guilhermeavanci
Copy link

Screenshot
image

Expected Behaviour
I expected it to treat null as a valid value to be received through the payload property, or at least not throw an error in this case.

Actual Behaviour
Right now if the server for some reason returns the payload as null the following error is thrown:
"connection_ack" message expects the 'payload' property to be an object or missing, but got "null"

This error stops the connection and prevents the websocket from being used.

Debug Information
This is the commit where the error throw was added.

Before that, apparently, this situation wasn't an issue simply because it doesn't stop the websocket connection, right!? (Not sure about that)

I tried to use both jsonMessageReviver and jsonMessageReplacer to intercept the payload and turn it into undefined to prevent the error throw, but I had no success. Is there a way to do that?

Further Information
Sadly I can't change the backend right now for other reasons.

Thank you very much in advance for the help!

@enisdenjo
Copy link
Owner

The idea was to completely omit the field if it does not exist, decreasing the message size. But, I don't see why we couldn't make it nullable.

I will take care of this, thanks for writing!

@enisdenjo enisdenjo added the enhancement New feature or request label Feb 26, 2023
@guilhermeavanci
Copy link
Author

I agree! Decreasing message size is definitely the better approach. Sadly we can't change this on the backend right now, we're using another third-party library there, which also has been informed about the "null issue". It's a corner case between both libraries.

Thank you very much for the quick response! We're going to wait for the fix!

@enisdenjo
Copy link
Owner

Closed and released with #456.

@enisdenjo enisdenjo added the released Has been released and published label Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released Has been released and published
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants