Skip to content

Conversation

@sixtysixx
Copy link
Contributor

@sixtysixx sixtysixx commented Jun 24, 2025

Summary by CodeRabbit

  • New Features

    • Enhanced the asynchronous keep-alive connection manager with robust reconnection, event handling, and detailed connection statistics for improved reliability.
    • Added a demo function to illustrate persistent connection usage.
    • Updated requirements with new and upgraded packages for better compatibility and features.
  • Bug Fixes

    • Updated WebSocket origin header to ensure compatibility with the latest PocketOption endpoint.
  • Documentation

    • Refined README instructions and removed emojis for clearer, more professional output.
    • Updated to-do lists for improved project planning.
  • Style

    • Removed emojis from various test output messages for consistency.
  • Chores

    • Removed deprecated funding and legacy to-do files.
    • Improved logging messages for clarity.

@coderabbitai
Copy link

coderabbitai bot commented Jun 24, 2025

Walkthrough

This update introduces a comprehensive refactor of the asynchronous keep-alive connection manager for the PocketOption API, replacing the previous implementation with a modern, task-based async architecture. It also updates documentation, requirements, and test output formatting, removes deprecated configuration and to-do files, and makes minor improvements to logging and headers in test utilities.

Changes

File(s) Change Summary
pocketoptionapi_async/connection_keep_alive.py Major async refactor: new ConnectionKeepAlive class, async event/task management, improved reconnection, message handling, and stats.
connection_keep_alive.py Deleted: old keep-alive connection manager and demo coroutine removed.
.github/FUNDING.yml, TODO/todo.md Deleted: removed funding configuration and legacy to-do list.
pocketoptionapi_async/init.py Removed a comment from the __all__ list; no functional changes.
pocketoptionapi_async/connection_monitor.py Changed import from absolute to relative for AsyncPocketOptionClient.
requirements.txt Upgraded pandas, loguru; added selenium, webdriver-manager, psutil; reordered packages.
README.md Updated Quick Start heading (removed emoji); updated SSID scraper folder reference.
todo.md Added new to-do list with specific tasks and deadlines.
tools/client_test.py Changed websocket origin header; improved log messages for clarity and consistency.
tests/test_complete_ssid.py, tests/test_demo_live_connection.py, tests/test_fixed_connection.py, tests/test_new_api.py, tests/test_persistent_connection.py Removed emojis from print statements for cleaner test output.

Sequence Diagram(s)

sequenceDiagram
    participant UserScript
    participant ConnectionKeepAlive
    participant WebSocketServer
    UserScript->>ConnectionKeepAlive: start_persistent_connection()
    ConnectionKeepAlive->>WebSocketServer: Establish websocket connection
    WebSocketServer-->>ConnectionKeepAlive: Initial handshake messages
    ConnectionKeepAlive->>WebSocketServer: Send authentication (SSID)
    WebSocketServer-->>ConnectionKeepAlive: Auth success / messages
    loop Background Tasks
        ConnectionKeepAlive->>WebSocketServer: Send ping (every 20s)
        WebSocketServer-->>ConnectionKeepAlive: Pong
        WebSocketServer-->>ConnectionKeepAlive: Incoming messages
        ConnectionKeepAlive->>UserScript: Emit events (message, connect, etc.)
        ConnectionKeepAlive->>WebSocketServer: Send messages (on demand)
    end
    alt Disconnection/No pong
        ConnectionKeepAlive->>WebSocketServer: Attempt reconnect (with delay)
    end
    UserScript->>ConnectionKeepAlive: stop_persistent_connection()
    ConnectionKeepAlive-->>WebSocketServer: Close connection
Loading

Possibly related PRs

Suggested reviewers

  • theshadow76

Poem

In the warren of code, where async flows run,
A rabbit refactored—now the keep-alive’s fun!
Emojis have hopped off the test output’s trail,
While new tasks and handlers ensure we prevail.
With headers improved and requirements anew,
This patch is a leap—hippity-hop, through and through! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🔭 Outside diff range comments (1)
tests/test_fixed_connection.py (1)

18-25: Same integration-test concern as other test modules

test_connection_fix() opens a live websocket and waits on network I/O. Mark it @pytest.mark.integration or relocate to an examples/ folder so routine CI runs stay hermetic.

🧹 Nitpick comments (15)
pocketoptionapi_async/connection_monitor.py (1)

192-199: psutil.cpu_percent() returns 0 without an interval

Calling psutil.cpu_percent() (default interval=None) right after process creation almost always yields 0.0, making the metric meaningless. Either:

  1. Call it twice with a small interval:
process.cpu_percent(interval=None)  # first call to initialise
cpu_percent = process.cpu_percent(interval=0.1)
  1. Cache the value from a previous run and compute a delta.

Minor, but improves snapshot accuracy.

todo.md (1)

1-7: Resolve markdown-lint & grammar warnings

-# TO-DO List
-
-### Add Template for a basic PO bot
- - Soon- Preferably before July 4th.
-
-### Integrate more fallbacks if there is any errors
- - No ETA
+# TODO List
+
+## Add template for a basic PO bot
+- Soon – preferably before **July 4th**
+
+## Integrate more fallbacks if there are any errors
+- No ETA

Fixes:
• heading-increment (MD001)
• list indentation (MD007)
• grammar (“there are any errors”)

README.md (1)

74-74: Make the “tools” reference clickable for quick navigation

-If you are unable to find it, try running the automatic SSID scraper under the `tools` folder.
+If you are unable to find it, run the automatic SSID scraper located in the [`tools`](tools/) folder.
tests/test_complete_ssid.py (1)

20-38: Emoji remnants contradict the “purge emojis” PR objective

Several print statements still contain emojis (📝, 📤, 🔍, 👤, 🏷️, ⚡).
Suppress them to keep test output clean and consistent.

Example change:

-    print("📝 Testing with complete SSID format:")
+    print("Testing with complete SSID format:")

Apply similarly to other lines in this block.

tests/test_new_api.py (1)

181-236: Remove leftover emojis in test logs

The test suite still outputs emojis (📡, 📊, ⚡, etc.). This undermines the recent “purge emojis” cleanup and clutters CI logs.

Consider stripping all emojis from print statements in this file for consistency.

tests/test_persistent_connection.py (1)

170-180: Inconsistent emoji purge

Lines 55–143 still emit emoji glyphs (📊, 🎉, 📤 …) whereas the commit message says “purge emojis”.
Either keep them everywhere (for fancy console output) or remove them consistently to prevent mixed styles and broken glyphs on minimal terminals.

tests/test_demo_live_connection.py (1)

15-16: Emoji removal not complete

You removed 🌐 but kept ⏰ later. Unify formatting across the file for log consistency.

requirements.txt (1)

3-13: Remove std-lib packages and pin breaking upgrades

  1. asyncio ships with CPython; listing it in requirements.txt is unnecessary and confuses dependency resolvers.

  2. Jumping from pandas 1.5.x2.x and pydantic 2.x may introduce breaking API changes. Ensure:

    • All dataframe code (esp. .append, .ix, etc.) is updated.
    pydantic v2 migration steps (BaseModel config, validators) are covered with tests.

  3. Prefer upper bounds (<3) or explicit minor pins for libraries that have a history of breaking changes (pydantic, pandas, selenium).

Diff suggestion:

-asyncio
-selenium>=4.0.0
+selenium>=4.0.0,<5
 webdriver-manager>=4.0.0
 psutil>=5.9.0
 loguru>=0.7.2
 pydantic>=2.0.0,<3
 pandas>=2.0.0,<3
tests/test_fixed_connection.py (2)

119-124: Cosmetic: stray emojis remain after emoji-purge

Lines 27, 41–45, 60–66 still contain 📝 🔍 👤 🏷️ 📊 📋 etc. Decide on one style for readability.


146-151: Heading prints should be outside conditional

If success is False the “Key improvements” banner is skipped entirely; readers may miss the summary. Consider moving the block outside the if or adding an else: with a brief explanation.

tools/client_test.py (1)

14-18: Fix E261 spacing & extract magic constant

Flake8 flags missing spaces before the inline comment. Also, the origin URL is a logical constant—extracting it avoids repetition.

-                i,
-                extra_headers={
-                    "Origin": "https://pocketoption.com/" # main URL
-                },
+                i,
+                extra_headers={
+                    # main site URL required by server
+                    "Origin": "https://pocketoption.com/",
+                },

Optionally:

ORIGIN_HDR = {"Origin": "https://pocketoption.com/"}
...
extra_headers=ORIGIN_HDR,

origin update itself is correct

pocketoptionapi_async/connection_keep_alive.py (4)

188-191: Use contextlib.suppress for cleaner exception handling.

Replace empty try-except blocks with contextlib.suppress for better readability.

Add import at the top:

from contextlib import suppress

Then replace the try-except blocks:

-                    try:
-                        await self.websocket.close()
-                    except Exception:
-                        pass
+                    with suppress(Exception):
+                        await self.websocket.close()

Also applies to: 362-365


431-438: Remove unnecessary else after return.

The else block is not needed after a return statement.

         try:
             if self.is_connected and self.websocket:
                 await self.websocket.send(message)
                 self.connection_stats["total_messages_sent"] += 1
                 logger.debug(f"Message: Sent: {message[:50]}...")
                 return True
-            else:
-                logger.warning("Caution: Cannot send message: not connected")
-                return False
+            logger.warning("Caution: Cannot send message: not connected")
+            return False
         except Exception as e:

466-471: Simplify conditional returns in region extraction.

Remove unnecessary elif after return for cleaner code flow.

             if "api-" in parts:
                 return parts.replace("api-", "").upper()
-            elif "demo" in parts:
+            if "demo" in parts:
                 return "DEMO"
-            else:
-                return "UNKNOWN"
+            return "UNKNOWN"

39-40: Consider implementing exponential backoff for reconnection attempts.

The fixed reconnect_delay of 5 seconds might overwhelm the server during extended outages. Consider implementing exponential backoff to gradually increase the delay between reconnection attempts.

# In __init__:
self.reconnect_delay_base = 5  # Base delay in seconds
self.reconnect_delay_max = 300  # Max delay (5 minutes)
self.current_reconnect_delay = self.reconnect_delay_base

# In _handle_reconnection (after failed attempt):
self.current_reconnect_delay = min(
    self.current_reconnect_delay * 2,  # Exponential backoff
    self.reconnect_delay_max
)
await asyncio.sleep(self.current_reconnect_delay)

# Reset on successful connection:
self.current_reconnect_delay = self.reconnect_delay_base

Also applies to: 386-386

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f5e7d8 and 25f2de2.

📒 Files selected for processing (15)
  • .github/FUNDING.yml (0 hunks)
  • README.md (2 hunks)
  • TODO/todo.md (0 hunks)
  • connection_keep_alive.py (0 hunks)
  • pocketoptionapi_async/__init__.py (0 hunks)
  • pocketoptionapi_async/connection_keep_alive.py (1 hunks)
  • pocketoptionapi_async/connection_monitor.py (1 hunks)
  • requirements.txt (1 hunks)
  • tests/test_complete_ssid.py (1 hunks)
  • tests/test_demo_live_connection.py (3 hunks)
  • tests/test_fixed_connection.py (4 hunks)
  • tests/test_new_api.py (1 hunks)
  • tests/test_persistent_connection.py (2 hunks)
  • todo.md (1 hunks)
  • tools/client_test.py (2 hunks)
💤 Files with no reviewable changes (4)
  • pocketoptionapi_async/init.py
  • .github/FUNDING.yml
  • TODO/todo.md
  • connection_keep_alive.py
🧰 Additional context used
🪛 LanguageTool
todo.md

[grammar] ~6-~6: Did you mean “there are any errors”?
Context: ...y 4th. ### Integrate more fallbacks if there is any errors - No ETA

(THERE_S_MANY)

🪛 markdownlint-cli2 (0.17.2)
todo.md

3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


4-4: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)


7-7: Unordered list indentation
Expected: 0; Actual: 1

(MD007, ul-indent)

🪛 Flake8 (7.2.0)
tools/client_test.py

[error] 16-16: at least two spaces before inline comment

(E261)

🪛 Pylint (3.3.7)
pocketoptionapi_async/connection_keep_alive.py

[refactor] 16-16: Too many instance attributes (18/7)

(R0902)


[refactor] 76-85: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)


[refactor] 343-398: Too many nested blocks (6/5)

(R1702)


[refactor] 431-438: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)


[refactor] 466-471: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

🪛 Ruff (0.11.9)
pocketoptionapi_async/connection_keep_alive.py

107-110: Use contextlib.suppress(asyncio.CancelledError) instead of try-except-pass

Replace with contextlib.suppress(asyncio.CancelledError)

(SIM105)


188-191: Use contextlib.suppress(Exception) instead of try-except-pass

Replace with contextlib.suppress(Exception)

(SIM105)


362-365: Use contextlib.suppress(Exception) instead of try-except-pass

Replace with contextlib.suppress(Exception)

(SIM105)

🔇 Additional comments (3)
tests/test_persistent_connection.py (1)

40-60: ```shell
#!/bin/bash

Display the beginning of the test file to inspect the test implementation

if [ -f "tests/test_persistent_connection.py" ]; then
sed -n '1,200p' tests/test_persistent_connection.py
else
echo "tests/test_persistent_connection.py not found"
fi


</details>
<details>
<summary>tests/test_demo_live_connection.py (1)</summary>

`28-33`: **Possible `None` dereference**

If `client_demo.connection_info` is `None`, the `if` guard prevents the print *body*, but inside the f-string you still index `.region`, which would raise.  
Refactor to:

```diff
-if hasattr(client_demo, "connection_info") and client_demo.connection_info:
-    print(f"   Connected to: {client_demo.connection_info.region}")
+ci = getattr(client_demo, "connection_info", None)
+if ci:
+    print(f"   Connected to: {ci.region}")

Likely an incorrect or invalid review comment.

tools/client_test.py (1)

44-55: Tighten log strings & remove trailing spaces

Minor wording tweaks already look good; consider using logger instead of print for structured output so you can change verbosity with one switch.

from loguru import logger

from pocketoptionapi_async.client import AsyncPocketOptionClient
from client import AsyncPocketOptionClient
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use an explicit relative import to avoid package-resolution issues

from client import AsyncPocketOptionClient searches sys.meta_path for a top-level module called client.
When the file is executed inside the pocketoptionapi_async package (e.g. python -m pocketoptionapi_async.connection_monitor or from tests), that module is not on sys.path, so the import will fail.

Switch to an explicit relative import:

-from client import AsyncPocketOptionClient
+from .client import AsyncPocketOptionClient
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from client import AsyncPocketOptionClient
from .client import AsyncPocketOptionClient
🤖 Prompt for AI Agents
In pocketoptionapi_async/connection_monitor.py at line 16, the import statement
uses an absolute import for AsyncPocketOptionClient which can cause
package-resolution issues when running inside the pocketoptionapi_async package.
Change the import to an explicit relative import by prefixing it with a dot, so
it correctly resolves the module within the package context.

Comment on lines 12 to 18
# Test SSID with demo=1 hardcoded (should be overridden by is_demo parameter)
demo_ssid = r'42["auth",{"session":"n1p5ah5u8t9438rbunpgrq0hlq","isDemo":1,"uid":72645361,"platform":1,"isFastHistory":true}]'

print("🌐 Testing Demo/Live Connection Fix")
print("Testing Demo/Live Connection Fix")
print("=" * 50)

# Test 1: Demo mode connection (should connect to demo regions)
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mark demo/live connection test as integration or skip by default

Like the persistent-connection test, this function opens real sockets and waits up to 30 s. Consider:

-async def test_demo_live_connection():
+@pytest.mark.integration
+async def test_demo_live_connection():

or wrap the body with

if not os.getenv("RUN_INTEGRATION"):
    pytest.skip("slow network test")

to avoid slowing / breaking the default test pipeline.

🤖 Prompt for AI Agents
In tests/test_demo_live_connection.py around lines 12 to 18, the demo/live
connection test opens real sockets and can take up to 30 seconds, which may slow
down or break the default test pipeline. To fix this, mark the test as an
integration test or add a conditional skip by checking if an environment
variable like RUN_INTEGRATION is set; if not, call pytest.skip to skip the test.
This prevents the test from running by default and only runs it when explicitly
enabled.

Comment on lines +56 to 58
f"Connected to: {client.connection_info.region if client.connection_info else 'Unknown'}"
)

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard against None before accessing .region

client.connection_info can be None when auth fails (expected with fake SSID). Dereference defensively:

-if client.connection_info:
-    print(f"Connected to: {client.connection_info.region}")
+ci = client.connection_info
+if ci:
+    print(f"Connected to: {ci.region}")

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In tests/test_fixed_connection.py around lines 56 to 58, the code accesses
client.connection_info.region without checking if client.connection_info is
None, which can cause an error if authentication fails. Update the code to first
check if client.connection_info is not None before accessing the .region
attribute, using a conditional expression or similar defensive coding to avoid
dereferencing None.

Comment on lines +133 to +134
import ssl

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Move SSL import to module level.

Import statements should be at the top of the file, not inside methods.

Move this import to the top of the file with other imports:

import ssl
🤖 Prompt for AI Agents
In pocketoptionapi_async/connection_keep_alive.py around lines 133 to 134, the
import statement for the ssl module is inside a method, which is not
recommended. Move the line "import ssl" from inside the method to the top of the
file with the other import statements to follow best practices and improve code
clarity.

Comment on lines +135 to +137
ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
ssl_context.check_hostname = False
ssl_context.verify_mode = ssl.CERT_NONE
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security concern: SSL verification is disabled.

Disabling SSL hostname checking and certificate verification exposes the connection to man-in-the-middle attacks. Consider:

  1. Using proper SSL certificates in production
  2. Adding a warning comment if this is intentional for development
  3. Making SSL verification configurable based on environment

Would you like me to implement a configurable SSL context that enables verification in production while allowing it to be disabled for development/testing?

🤖 Prompt for AI Agents
In pocketoptionapi_async/connection_keep_alive.py around lines 135 to 137, SSL
hostname checking and certificate verification are disabled, which is a security
risk. Modify the code to make SSL verification configurable based on the
environment by adding a parameter or config flag that enables full verification
in production and allows disabling it only in development or testing. Also, add
a clear warning comment explaining the security implications if verification is
disabled intentionally.

self.is_demo = is_demo

# Connection state
self.websocket: Optional[websockets.WebSocketServerProtocol] = None
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect type hint for websocket client.

The type hint should be WebSocketClientProtocol instead of WebSocketServerProtocol since this is a client connecting to a server.

-        self.websocket: Optional[websockets.WebSocketServerProtocol] = None
+        self.websocket: Optional[websockets.WebSocketClientProtocol] = None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self.websocket: Optional[websockets.WebSocketServerProtocol] = None
self.websocket: Optional[websockets.WebSocketClientProtocol] = None
🤖 Prompt for AI Agents
In pocketoptionapi_async/connection_keep_alive.py at line 26, the type hint for
the websocket attribute is incorrectly set to WebSocketServerProtocol. Change
this type hint to WebSocketClientProtocol to correctly represent that this is a
client connecting to a server.

Comment on lines +343 to 396
while self.should_reconnect:
try:
await asyncio.sleep(5) # Check every 5 seconds

if success:
self._connection_stats["total_reconnections"] += 1
logger.info("Reconnection successful")
await self._trigger_event_async("reconnected")
else:
logger.error("Reconnection failed")
await asyncio.sleep(10) # Wait before next attempt
if not self.is_connected and self.should_reconnect:
logger.warning(
"Persistent: Detected disconnection, attempting reconnect..."
)

async def disconnect(self):
"""Disconnect and clean up resources"""
logger.info("Disconnecting...")
self.current_reconnect_attempts += 1
self.connection_stats["total_reconnects"] += 1

if self.current_reconnect_attempts <= self.max_reconnect_attempts:
logger.info(
f"Persistent: Reconnection attempt {self.current_reconnect_attempts}/{self.max_reconnect_attempts}"
)

# Clean up current connection
if self.websocket:
try:
await self.websocket.close()
except Exception:
pass
self.websocket = None

# Try to reconnect
success = await self._establish_connection()

if success:
logger.success("Success: Reconnection successful!")
await self._emit_event(
"reconnected",
{
"attempt": self.current_reconnect_attempts,
"url": self.connection_info.url
if self.connection_info
else None,
},
)
else:
logger.error(
f"Error: Reconnection attempt {self.current_reconnect_attempts} failed"
)
await asyncio.sleep(self.reconnect_delay)
else:
logger.error(
f"Error: Max reconnection attempts ({self.max_reconnect_attempts}) reached"
)
await self._emit_event(
"max_reconnects_reached",
{"attempts": self.current_reconnect_attempts},
)
break

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reduce nesting complexity in reconnection monitor.

The method has 6 levels of nesting which makes it hard to follow. Consider extracting the reconnection logic into a separate method.

     async def _reconnection_monitor(self):
         """Monitor for disconnections and automatically reconnect (like old API)"""
         logger.info("Persistent: Starting reconnection monitor...")
 
         while self.should_reconnect:
             try:
                 await asyncio.sleep(5)  # Check every 5 seconds
 
                 if not self.is_connected and self.should_reconnect:
-                    logger.warning(
-                        "Persistent: Detected disconnection, attempting reconnect..."
-                    )
-
-                    self.current_reconnect_attempts += 1
-                    self.connection_stats["total_reconnects"] += 1
-
-                    if self.current_reconnect_attempts <= self.max_reconnect_attempts:
-                        logger.info(
-                            f"Persistent: Reconnection attempt {self.current_reconnect_attempts}/{self.max_reconnect_attempts}"
-                        )
-
-                        # Clean up current connection
-                        if self.websocket:
-                            try:
-                                await self.websocket.close()
-                            except Exception:
-                                pass
-                            self.websocket = None
-
-                        # Try to reconnect
-                        success = await self._establish_connection()
-
-                        if success:
-                            logger.success("Success: Reconnection successful!")
-                            await self._emit_event(
-                                "reconnected",
-                                {
-                                    "attempt": self.current_reconnect_attempts,
-                                    "url": self.connection_info.url
-                                    if self.connection_info
-                                    else None,
-                                },
-                            )
-                        else:
-                            logger.error(
-                                f"Error: Reconnection attempt {self.current_reconnect_attempts} failed"
-                            )
-                            await asyncio.sleep(self.reconnect_delay)
-                    else:
-                        logger.error(
-                            f"Error: Max reconnection attempts ({self.max_reconnect_attempts}) reached"
-                        )
-                        await self._emit_event(
-                            "max_reconnects_reached",
-                            {"attempts": self.current_reconnect_attempts},
-                        )
-                        break
+                    await self._handle_reconnection()
 
             except Exception as e:
                 logger.error(f"Error: Reconnection monitor error: {e}")
+
+    async def _handle_reconnection(self):
+        """Handle reconnection attempt"""
+        logger.warning("Persistent: Detected disconnection, attempting reconnect...")
+        
+        self.current_reconnect_attempts += 1
+        self.connection_stats["total_reconnects"] += 1
+        
+        if self.current_reconnect_attempts > self.max_reconnect_attempts:
+            logger.error(
+                f"Error: Max reconnection attempts ({self.max_reconnect_attempts}) reached"
+            )
+            await self._emit_event(
+                "max_reconnects_reached",
+                {"attempts": self.current_reconnect_attempts},
+            )
+            self.should_reconnect = False
+            return
+        
+        logger.info(
+            f"Persistent: Reconnection attempt {self.current_reconnect_attempts}/{self.max_reconnect_attempts}"
+        )
+        
+        # Clean up current connection
+        await self._cleanup_connection()
+        
+        # Try to reconnect
+        success = await self._establish_connection()
+        
+        if success:
+            logger.success("Success: Reconnection successful!")
+            await self._emit_event(
+                "reconnected",
+                {
+                    "attempt": self.current_reconnect_attempts,
+                    "url": self.connection_info.url if self.connection_info else None,
+                },
+            )
+        else:
+            logger.error(
+                f"Error: Reconnection attempt {self.current_reconnect_attempts} failed"
+            )
+            await asyncio.sleep(self.reconnect_delay)
+            
+    async def _cleanup_connection(self):
+        """Clean up current websocket connection"""
+        if self.websocket:
+            with suppress(Exception):
+                await self.websocket.close()
+            self.websocket = None

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Pylint (3.3.7)

[refactor] 343-398: Too many nested blocks (6/5)

(R1702)

🪛 Ruff (0.11.9)

362-365: Use contextlib.suppress(Exception) instead of try-except-pass

Replace with contextlib.suppress(Exception)

(SIM105)

🤖 Prompt for AI Agents
In pocketoptionapi_async/connection_keep_alive.py around lines 343 to 396, the
reconnection monitor method has excessive nesting making it hard to read.
Refactor by extracting the entire reconnection logic inside the while loop into
a separate async method. This new method should handle checking connection
status, incrementing attempts, closing the websocket, attempting reconnection,
logging results, and emitting events. Then call this new method from the loop to
reduce nesting and improve clarity.

@theshadow76 theshadow76 merged commit b9241ea into ChipaDevTeam:main Jun 24, 2025
1 check passed
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