Skip to content

Conversation

@bryghtlabs-richard
Copy link
Contributor

@bryghtlabs-richard bryghtlabs-richard commented Jan 28, 2026

Description

If auto-reconnect-on-close is configured, close should not try to stop the client, but just close the current connection.

This is needed to support fault injection for reliability testing of protocols like SocketIO, who will perform a higher-level connection-state-recovery.

This is also needed to support load balanced services, where a SocketIO event might indicate the client should temporarily disconnect to reconnect to a healthier or longer-living backend server.

Related

I introduced this bug in 19891d8, though it only broke for anyone trying to cleanly close a connection without stopping the client with this particular configuration.

Testing

Tested by adding a button to my UI that calls esp_websocket_client_close_with_code(), and verifying I could manually close the websocket client and device automatically reconnects on demand.


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • [] Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

Note

Ensures esp_websocket_client_close_with_optional_body() does not stop the client when auto_reconnect and close_reconnect are enabled, allowing a clean close of the current connection and seamless auto-reconnect.

  • After sending CLOSE and setting CLOSE_FRAME_SENT_BIT, return early if config->auto_reconnect && config->close_reconnect, skipping STOP/WAIT logic
  • Minor: update SPDX copyright year to 2026

Written by Cursor Bugbot for commit 2083a0b. This will update automatically on new commits. Configure here.

If auto-reconnect-on-close is configured, just close the current
connection. Do not try to stop the client.
Copy link

@cursor cursor bot left a comment

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.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.


if (client->config->auto_reconnect && client->config->close_reconnect) {
// Client does not stop(STOPPED_BIT) with auto-reconnect-on-close
return ESP_OK;
Copy link

Choose a reason for hiding this comment

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

CLOSE_FRAME_SENT_BIT persists after failed send and reconnection

Medium Severity

When esp_websocket_client_send_close fails (e.g., transport error), esp_websocket_client_abort_connection is called which sets state to WEBSOCKET_STATE_WAIT_TIMEOUT for reconnection. However, CLOSE_FRAME_SENT_BIT is set unconditionally after send_close returns, regardless of success or failure. With the new early return, the client then reconnects with this bit still set, causing PINGs to be permanently suppressed on the new connection. Previously, a failed close would eventually trigger a force-stop, making this a non-issue. The bit should only be set if the close frame was actually sent successfully.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's intentional - CLOSE_FRAME_SENT_BIT is cleared by the client-thread when it processes the disconnect.

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.

1 participant