Skip to content

fix: add break label in ws loop to properly escape outer loop #347

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

almk-dev
Copy link
Member

@almk-dev almk-dev commented Jul 23, 2025

Description

Addresses: EVM-168

The WebSocket logic conditionally uses return, continue, and break as part of its control flow. However, the break statements inside the switch block is a no-op as it just exits the switch (which will happen anyways as it's the last statement in the case).

If the original intent is to exit the read loop and stop the socket flow entirely, we need a label for the outer loop and break that instead.


Author Checklist

I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the main branch

Reviewers Checklist

I have...

  • added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • confirmed all author checklist items have been addressed
  • confirmed that this PR does not change production code
  • reviewed content
  • tested instructions (if applicable)
  • confirmed all CI checks have passed

@almk-dev almk-dev changed the title add break label fix: add break label in ws loop to properly escape outer loop Jul 23, 2025
@almk-dev almk-dev marked this pull request as ready for review July 23, 2025 15:51
@almk-dev almk-dev requested review from a team as code owners July 23, 2025 15:51
@aljo242
Copy link
Contributor

aljo242 commented Jul 24, 2025

Is there a way to test this functionality?

@almk-dev
Copy link
Member Author

@aljo242 We'd have to devise a test suite for the private methods. It might be more trouble than it's worth as this is just a fix meant to reflex the intent of the original logic and not a behavioral change.

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