fix: Improve WebSocket Handling and Error Logging #3140
Conversation
|
@Alphena-EK Did #3134 not fix it? (working on latest version?) |
|
PR #3134 fixed WebSocket errors by correcting handler functions and upgrading the websockets library to version 15.0. However, our PR #3140 goes beyond just fixing these errors—it improves error logging, enhances WebSocket connection stability, and makes debugging easier. Here’s what we changed and why it matters: What’s New in PR #3140?
WebSocket errors are now logged with more details. Instead of just showing a generic error, logs now include the error type, connection status, and a full stack trace. Why? Clearer logs make it easier for developers to quickly find the root cause of WebSocket failures.
Previously, some WebSocket errors failed silently in the background, leading to unnoticed issues. Now, these critical errors are properly caught and logged, improving system reliability. Why? WebSocket errors often happen due to connection drops, network issues, or client disconnections. If they are not handled correctly, the system may run into unexpected failure states.
If a WebSocket connection unexpectedly closes, the system now logs the event properly and prevents unnecessary resource usage. Why? WebSocket closures can happen due to browser behavior, server timeouts, or network disruptions. The system should be able to handle these situations smoothly.
We added a debug mode that provides real-time WebSocket status updates. Developers can enable this mode to get more detailed logs and track connection events step by step. Why? Debugging WebSocket issues can be difficult. This feature gives developers more visibility into connection events, making troubleshooting easier. PR #3134 fixed the core WebSocket issues, but our changes add extra value by improving error logging and connection management. If the Caldera team finds these enhancements useful, we can update PR #3140 to focus on them. If the existing fixes are considered sufficient, we can close the PR. 🫡 |
|
@uruwhy Have time to take a look at this since you just worked on the code? |
|
@Alphena-EK Good write up, give us a day to look at more. |
I updated the code to convert the port string to an integer during validation, allowing us to catch errors early. I also added a check to ensure the port is within the valid range (1-65535), preventing unexpected failures when calling websockets.serve.
|
We'll also need to add a check here: https://github.com/mitre/caldera/blob/master/app/service/event_svc.py#L16 |
1. Added WebSocket URI validation.
If self.get_config('app.contact.websocket') returns None, it now defaults to ws://localhost:8888 instead of causing an error.
2. Modernized asynchronous task handling.
Replaced asyncio.get_event_loop().create_task(...) with asyncio.create_task(...) to use a more up-to-date and cleaner approach.
3. Improved global event listener handling.
Now, the system checks if the callback function is asynchronous (async def). If so, it awaits the function; otherwise, it calls it directly. This ensures both synchronous and asynchronous callbacks are properly handled.
|
I’ve added a validation check for the WebSocket URI in event_svc.py. If the configuration is missing, it now defaults to localhost:8888 instead of failing. Let me know if there's anything else I should adjust. |
Thanks for the tweaks! I did make a suggestion about using a the original default websocket port, but I also noticed that it looks like you might have overwritten the |
|
I will review it from the beginning and create a new PR. It is still giving some errors. |
|
@Alphena-EK Do you want me to wait day or two for new PR? Or am I good to go ahead and mint another minor version? |
|
Feel free to proceed with minting another minor version. We won't be submitting a new PR in the next day or two, so it should be safe. If anything changes, I'll keep you updated. Thanks for checking in! |
Fixed WebSocket config parsing to handle host:port correctly.
Improved error handling and logging for better debugging.
Refactored handle method to prevent path parsing errors.
Why?
Prevents crashes due to misconfigured WebSocket settings.
Enhances debugging with clearer error messages.
Ensures stable and reliable WebSocket connections.