2lambda123 / ibkr-options-volatility-trading

MIT License
4 stars 1 forks source link

Got it running but no result. #7

Open stark4x opened 7 months ago

stark4x commented 7 months ago

Starting MarketWatcher... MarketWatcher started. Reading target stocks from file: src/market_watcher/research/target_stocks.yaml Instantiating email notifier... Instantiating slack notifier... Instantiating MarketWatcher and running the engine. WARNING:urllib3.connectionpool:Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLZeroReturnError(6, 'TLS/SSL connection has been closed (EOF) (_ssl.c:1135)'))': /

git-greetings[bot] commented 7 months ago

Thanks @stark4x for opening this issue!

For COLLABORATOR only :

git-greetings[bot] commented 7 months ago

First issue by @stark4x

Issues Details of @stark4x in ibkr-options-volatility-trading : OPEN CLOSED TOTAL
1 0 1
codeautopilot[bot] commented 7 months ago

Potential solution

The solution involves addressing multiple issues across different files to ensure robust error handling, proper instantiation of notifiers, and correct execution of the MarketWatcher engine. The plan includes fixing typos, enhancing error handling for SSL-related issues, ensuring that the YahooFinancials library is correctly handling SSL connections, and improving logging to provide better feedback during execution.

What is causing this bug?

The bug seems to be caused by an SSLError(SSLZeroReturnError) which indicates that the SSL/TLS connection has been closed unexpectedly. This error is likely occurring during an HTTP request made by the YahooFinancials library, which is used to fetch financial data. The lack of comprehensive error handling and logging in the market_watcher_cli.py and common.py files may be obscuring the underlying problem, making it difficult to diagnose and resolve the issue.

Code

To address the issues, the following code changes are recommended:

  1. In src/market_watcher/market_watcher_cli.py, fix the typo in the function name:

    # Before
    def get_terget_stocks():
    # After
    def get_target_stocks():
  2. Add error handling for SSLError in src/market_watcher/market_watcher_cli.py:

    try:
    # Existing code to start MarketWatcher engine
    except SSLError as ssl_error:
    # Log the SSL error for further investigation
    logger.error(f"SSL error occurred: {ssl_error}")
  3. Ensure that the YahooFinancials library is correctly handling SSL connections. If the library is not handling it, consider wrapping its calls in try-except blocks to handle the SSLError in src/market_watcher/common.py:

    
    from requests.exceptions import SSLError

try:

Call to YahooFinancials that may raise SSLError

except SSLError as ssl_error:

Handle the error, possibly with a retry mechanism or logging


4. Improve logging throughout the `start` command in `src/market_watcher/market_watcher_cli.py` to provide feedback on the progress and any potential issues encountered.

5. In `src/market_watcher/notifier.py`, replace assertions with proper error handling:
```python
response = requests.post(self.webhook_url, json=message_payload)
if response.status_code != 200 or response.text != "ok":
    # Log the error instead of asserting
    logger.error(f"Failed to send Slack message: {response.text}")

How to replicate the bug

To replicate the bug, one would need to run the MarketWatcher engine and trigger an SSL connection issue. This could be done by attempting to fetch financial data using the YahooFinancials library over an SSL connection that is prone to being closed unexpectedly. The exact replication steps would depend on the specific environment and external factors affecting SSL connections.

Task

To resolve the bug, the following tasks should be carried out:

  1. Correct all typos in function and variable names across the affected files.
  2. Implement comprehensive error handling for SSL-related issues and other potential exceptions that may occur during HTTP requests or notifier instantiation.
  3. Update the YahooFinancials library to the latest version or ensure it is correctly handling SSL connections. If the issue persists, consider using a different financial data API or library.
  4. Enhance logging throughout the MarketWatcher application to provide better feedback and facilitate debugging.
  5. Test the changes thoroughly to ensure that the bug is resolved and that the MarketWatcher engine can run without encountering the SSLZeroReturnError.

Click here to create a Pull Request with the proposed solution

Files used for this task:

Changes on src/market_watcher/market_watcher_cli.py Upon reviewing the `src/market_watcher/market_watcher_cli.py` file, the following observations and suggestions are made: 1. **Typo in Function Name**: The function `get_terget_stocks` is likely a typo and should be `get_target_stocks`. This typo could potentially cause an `AttributeError` if the correct function name is not being called elsewhere in the code. 2. **Error Handling**: The `start` command has a try-except block that catches `ValueError` exceptions. However, the user's issue mentions an `SSLError` which is not being caught here. It would be advisable to add error handling for `SSLError` or a more general exception handling to catch other types of exceptions that might occur during the execution of the `start` command. 3. **Notification Instantiation**: The email and slack notifiers are being instantiated based on the `context.state` values. It is important to ensure that the `context.state` is correctly set up to include the `email` and `slack` keys with boolean values indicating whether to use these notifiers. If these keys are not set or are set incorrectly, the notifiers will not be instantiated, and no notifications will be sent. 4. **Notification Sending**: There is no explicit error handling around the sending of notifications in the `test_slack` command. If the Slack API call fails, it will print the exception, but it might be beneficial to have more robust error handling and logging to understand why the notification failed. 5. **MarketWatcher Engine Execution**: The `MarketWatcherEngine` is instantiated with `target_stocks` and `notifiers`, and the method `search_for_intestment_opportunities` is called. It is important to ensure that this method is implemented correctly and that it is capable of sending notifications through the provided notifiers. If there is an issue within this method, it could result in the engine running without producing any results or notifications. 6. **Logging and Feedback**: The user's issue indicates that the program runs but produces no result. It would be beneficial to add more logging throughout the `start` command to provide feedback on the progress and any potential issues encountered during the execution. This could help in identifying at which point the program fails to produce the expected results. 7. **Command Descriptions**: There is a minor typo in the `config` command description ("Lists congiguration" should be "Lists configuration"). While this does not affect the functionality, it is good practice to maintain accurate documentation and help descriptions. In conclusion, while there are no direct indications of what might be causing the user's issue within this file, the lack of comprehensive error handling and logging could be obscuring the underlying problem. It is recommended to address the typo, improve error handling, ensure correct notifier instantiation, and enhance logging to facilitate better debugging and resolution of the issue.
Changes on src/market_watcher/common.py After reviewing the `src/market_watcher/common.py` file, the following observations and recommendations can be made regarding the SSLZeroReturnError and the HTTP request handling: 1. **SSLZeroReturnError**: This error indicates that the SSL/TLS connection has been closed unexpectedly. It is raised by the underlying SSL implementation (`_ssl.c`) and is propagated up through the `urllib3` library, which is used by `requests` or similar HTTP libraries. However, in the provided code, there is no direct use of `urllib3` or any HTTP requests being made. The error is likely coming from the `YahooFinancials` library, which is not shown in the provided code. To address this, we would need to inspect the `YahooFinancials` library or ensure that it is correctly handling SSL connections. 2. **HTTP Request Handling**: Since the code does not directly make HTTP requests, there is no HTTP request handling to update in this file. If the `YahooFinancials` library is responsible for making the HTTP requests, then the error handling and SSL configuration would need to be addressed within that library or by wrapping its calls in try-except blocks to handle the `SSLError`. 3. **Recommendations**: - If possible, update the `YahooFinancials` library to the latest version, as this might include fixes for SSL issues. - Implement error handling around the call to `yahoo_financials_target_stocks.get_current_percent_change()` to catch any `SSLError` and retry the request or log the error for further investigation. - If the `YahooFinancials` library allows for custom configuration of the HTTP session, ensure that it is configured to handle SSL properly, including the use of up-to-date cipher suites and protocols. - Consider using a different financial data API or library if the SSL issues with `YahooFinancials` cannot be resolved. 4. **Error Handling**: The code has a basic error handling mechanism in the `get_terget_stocks` function, which prints the exception. It would be beneficial to have more robust error handling throughout the `MarketWatcherEngine` class, including logging and potentially retry mechanisms for transient errors. 5. **Code Review**: There is a typo in the method name `get_terget_stocks` which should be `get_target_stocks`. Also, the variable names `pnl_threashold` and `threshlold` contain typos and should be `pnl_threshold` and `threshold`, respectively. In summary, the `common.py` file does not directly handle HTTP requests or SSL configurations. The SSLZeroReturnError is likely coming from an external library (`YahooFinancials`), and the solution would involve investigating and potentially updating the handling of HTTP requests within that library. Additionally, improving error handling and fixing typos in the code would be beneficial.
Changes on src/market_watcher/research/target_stocks.yaml The `target_stocks.yaml` file contains a list of stock symbols, each associated with a trading strategy (either "long straddle" or "short straddle"). The YAML format appears to be correct, with each stock symbol being a key followed by a colon, and the strategy being a nested key-value pair. Here are some points to consider regarding the `target_stocks.yaml` file: 1. **Format**: The YAML format is consistent and correct. Each entry has a stock symbol as the key and a dictionary containing the strategy as the value. 2. **Content**: All entries follow the same pattern, with "strategy" being the only key within each stock symbol's dictionary. There are no apparent typos or syntax errors in the strategies ("long straddle" and "short straddle"). 3. **Readability**: The file can be read by a YAML parser without issues, assuming the parser is correctly implemented in the MarketWatcher application. 4. **Processing**: Since the file format is correct, any issues with processing this file would likely be due to the code that reads and interprets the YAML data, not the file itself. 5. **Strategies**: The file only specifies the strategy for each stock. If the MarketWatcher application requires additional information to process each stock (such as thresholds, price targets, etc.), and that information is missing, it could lead to the application not producing any results. 6. **Duplication**: There are no duplicate entries for stock symbols, which is good as it prevents potential conflicts or overwriting of strategies. 7. **Completeness**: The file seems to be a complete list of target stocks with their respective strategies. There is no indication of missing data. 8. **External Dependencies**: The file itself does not have any external dependencies. However, the application might depend on external services to fetch stock data, and issues with those services could affect the application's performance. In conclusion, the `target_stocks.yaml` file appears to be correctly formatted and contains a consistent list of stock symbols with associated strategies. There is no indication within this file that would directly cause the bug described in the ticket. The issue might lie in the code that processes this file or in the external services that the MarketWatcher application relies on to fetch stock data.
Changes on src/market_watcher/notifier.py Upon reviewing the `notifier.py` file, the implementation of the notification system includes two classes: `EmailNotifier` and `SlackNotifier`, both of which inherit from a base `Notifier` class. The `EmailNotifier` class is responsible for sending emails, while the `SlackNotifier` class is responsible for sending messages to Slack channels using webhooks. Here are some observations and potential issues: 1. **EmailNotifier.send() Method:** - The `send` method of the `EmailNotifier` class uses `smtplib.SMTP_SSL` to send an email with SSL encryption. This method appears to be correctly implemented, including the use of `ssl.create_default_context()` for better security defaults. - The method prints a success message after sending the email, but there is no error handling in case the email fails to send. This could lead to silent failures where the system believes an email has been sent when it has not. It would be beneficial to add a try-except block to catch exceptions such as `smtplib.SMTPException` and log them. 2. **SlackNotifier.send_message() Method:** - The `send_message` method sends a message to a Slack channel using a webhook. It asserts that the response status code is 200 and the response body is "ok". - The use of assertions here is not ideal for production code. If the assertion fails, it will raise an `AssertionError`, which may not be caught if not properly handled. It would be better to replace the assertions with proper error handling and logging to ensure that any issues with sending Slack messages are reported. 3. **General Notification System:** - Both `EmailNotifier` and `SlackNotifier` classes have a `notify` method that formats and sends the messages. However, there is no indication of what happens if there is no data for either long or short straddle strategies. If `investment_data[STRATEGIES.LONG_STRADDLE.value]` or `investment_data[STRATEGIES.SHORT_STRADDLE.value]` is empty or missing, this could lead to errors or empty messages being sent. 4. **Logging and Error Reporting:** - The current implementation lacks comprehensive logging and error reporting. Adding logging statements and error handling would help identify issues when notifications fail to send. 5. **SSLZeroReturnError:** - The issue reported by the user includes a `WARNING:urllib3.connectionpool:Retrying` message with an `SSLError(SSLZeroReturnError(6, 'TLS/SSL connection has been closed (EOF) (_ssl.c:1135)'))`. This error is related to SSL connections but does not directly relate to the code in `notifier.py`. It is more likely related to the HTTP request handling in the `common.py` file, as per the plan. In conclusion, while the `notifier.py` file does not seem to be directly related to the `SSLError(SSLZeroReturnError)` mentioned in the user's ticket, there are improvements that can be made to the notification system to ensure better error handling and logging. Implementing these changes would help identify and resolve issues with sending notifications, but the SSL error will likely need to be addressed in the `common.py` file.
pairbot[bot] commented 7 months ago

Not Found

pairbot[bot] commented 7 months ago

Not Found

pairbot[bot] commented 7 months ago

Not Found