Skip to content

Add IsNotWsError method to WSConn#496

Open
wubie052 wants to merge 1 commit into
topfreegames:mainfrom
wubie052:patch-5
Open

Add IsNotWsError method to WSConn#496
wubie052 wants to merge 1 commit into
topfreegames:mainfrom
wubie052:patch-5

Conversation

@wubie052

@wubie052 wubie052 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Ignore normal closure errors


Note

Low Risk
Low risk: only changes error handling in WSConn.GetNextMessage to suppress expected close codes; main risk is masking unexpected shutdowns if callers relied on those errors.

Overview
WebSocket reads now ignore normal close events. A new WSConn.IsNotWsError helper treats close codes 1000 (normal closure) and 1001 (going away) as non-errors.

WSConn.GetNextMessage now returns an error from ReadMessage only when it is not one of those normal closure conditions, reducing noisy errors on routine disconnects.

Reviewed by Cursor Bugbot for commit ae8e7aa. Bugbot is set up for automated code reviews on this repo. Configure here.

Ignore normal closure errors
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 25046437266

Coverage decreased (-0.06%) to 58.688%

Details

  • Coverage decreased (-0.06%) from the base build.
  • Patch coverage: 6 uncovered changes across 1 file (1 of 7 lines covered, 14.29%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
pkg/acceptor/ws_acceptor.go 7 1 14.29%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
pkg/modules/binary.go 2 78.26%

Coverage Stats

Coverage Status
Relevant Lines: 8661
Covered Lines: 5083
Line Coverage: 58.69%
Coverage Strength: 0.65 hits per line

💛 - Coveralls

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ae8e7aa. Configure here.

func (c *WSConn) GetNextMessage() (b []byte, err error) {
_, msgBytes, err := c.conn.ReadMessage()
if err != nil {
if err != nil && c.IsNotWsError(err) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normal closure becomes misleading invalid header error

High Severity

The IsNotWsError check prevents GetNextMessage from returning normal websocket close errors (1000/1001). Instead, it proceeds with an empty message, returning packet.ErrInvalidPomeloHeader. This causes the session handler to log an Errorf for normal client disconnects, which is contrary to the PR's goal of silent handling and diverges from the TCP acceptor's clean close behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ae8e7aa. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the AI didn't see the internal implementation of the IsNotWsError method, which led to the opposite conclusion.

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

Successfully merging this pull request may close these issues.

2 participants