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

Sending incorrect json to wsjson.Read closes connection #399

Closed
Anupam-Ashish-Minz opened this issue Jul 7, 2023 · 7 comments
Closed

Sending incorrect json to wsjson.Read closes connection #399

Anupam-Ashish-Minz opened this issue Jul 7, 2023 · 7 comments

Comments

@Anupam-Ashish-Minz
Copy link

I don't know where this is intended or not but it seem that sending incorrect json data to wsjson.Read closes the websocket connection even though an error object is returned.

Incorrect json data like

{
  "message": "hello world"
} something

Here a part of my code

var v interface{}
for {
    err = wsjson.Read(ctx, ws, &v)
    if err != nil {
        log.Println(err)
        continue
    }
    log.Println(v)
}

I was trying to continue using the connect in spite of error however this causes the connection to close and an infinite loop to run continuously printing the error.

@nhooyr
Copy link
Contributor

nhooyr commented Sep 28, 2023

Why do you want to try and reuse the connection if you read corrupted JSON? There's no way for you to do anything useful with the connection if the data you're reading is corrupt.

@nhooyr
Copy link
Contributor

nhooyr commented Oct 13, 2023

Feel free to comment when you can and I'll reopen.

@nhooyr nhooyr closed this as completed Oct 13, 2023
@aadithyen
Copy link

Well I agree that most of the time misformatted JSON won't come through. Might be better to close the connection and retry. But rather than wson.Read closing it, I think it would be better if it just returns an error and leaves it to the caller to handle it however they want, such as if they want to return an error, or log the scenario before closing.

@nhooyr
Copy link
Contributor

nhooyr commented May 10, 2024

I don't understand the purpose of that. What does giving the caller that flexibility do? They'll just have to close it themselves in the same way we do here. Just more code for them.

@aadithyen
Copy link

Maybe returning a bad request response before or instead of closing the connection.

@nhooyr
Copy link
Contributor

nhooyr commented May 10, 2024

The code closes with the proper close status and error message so that shouldn't be necessary in most cases.

See https://github.com/nhooyr/websocket/blob/master/wsjson/wsjson.go#L39

In the cases where it is, where you do want to send your own protocol level error message first, then I think it's ok to write your own helpers instead of using wsjson as wsjson is for the common case. See also #418

I've opened #450 to document this at least.

@achsvg
Copy link

achsvg commented Jun 27, 2024

Why do you want to try and reuse the connection if you read corrupted JSON? There's no way for you to do anything useful with the connection if the data you're reading is corrupt.

In our case we'd like to send an error message back to the user, prompting him to retry with a well formatted json without having to re-open a websocket connection.

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

No branches or pull requests

4 participants