fix: resolve critical race conditions in WebSocket proxy

[Alex-dev-angel]

Summary by cubic

Fixes critical race conditions in the WebSocket proxy using locks and reference counting to stabilize multi-client streaming. Also corrects Angel OHLC fields and returns structured API errors; adds tests and an audit report.

• Bug Fixes

  • Guard subscribe/unsubscribe with a subscription_lock and global reference counting.
  • Prevent duplicate adapter/user races with adapter_lock and user_lock.
  • Pre-register subscriptions with rollback on failure; move success logs after broker subscribe; validate before unsubscribe.
  • Snapshot subscriptions for the ZMQ listener and use immutable dict copies during iteration; add consistent error handling and rollback.

• Refactors

  • Correct Angel adapter OHLC field names (open/high/low/close).
  • Return structured error objects for empty or invalid API responses in order_api.
  • Add tests for multi-client subscriptions and the improved ZMQ server; include an audit report documenting fixes.

[cubic-dev-ai]

5 issues found across 6 files

Prompt for AI agents (all 5 issues)

Understand the root cause of the following 5 issues and fix them.


<file name="websocket_proxy/server.py">

<violation number="1" location="websocket_proxy/server.py:671">
Indexing mode_to_str with the raw client mode will raise a KeyError when the value isn’t 1/2/3 (e.g., clients sending the string &quot;3&quot; worked before but now blows up). Please fall back gracefully when the key is missing so subscriptions keep working.</violation>
</file>

<file name="docs/zmq_new_audit_report.md">

<violation number="1" location="docs/zmq_new_audit_report.md:1191">
The audit report claims the added file is `ZMQ_Audit_Report.md`, but the repository file is named `docs/zmq_new_audit_report.md`, so this reference is inaccurate.</violation>

<violation number="2" location="docs/zmq_new_audit_report.md:1192">
Listing `COMMIT_MESSAGE.md` as an added file is inaccurate because no file with that name is present in the repository.</violation>
</file>

<file name="test/test_improved_zmq_server.py">

<violation number="1" location="test/test_improved_zmq_server.py:984">
This test only counts messages containing &quot;ERROR&quot;, but the helper methods record unauthenticated request failures as &quot;...FAILED&quot;, so error_count never increases and the test always reports failure even when behavior is correct.</violation>
</file>

<file name="test/test_multi_client_fix.py">

<violation number="1" location="test/test_multi_client_fix.py:146">
The test only logs message counts instead of asserting that other clients continue receiving data after one unsubscribes, so it will silently succeed even when the regression reappears. Please add explicit assertions that detect regressions.</violation>
</file>

---

Since this is your first cubic review, here's how it works:

• cubic automatically reviews your code and comments on bugs and improvements
• Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
• Ask questions if you need clarification on any suggestion

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

[cubic-dev-ai]

Indexing mode_to_str with the raw client mode will raise a KeyError when the value isn’t 1/2/3 (e.g., clients sending the string "3" worked before but now blows up). Please fall back gracefully when the key is missing so subscriptions keep working.

Prompt for AI agents

Address the following comment on websocket_proxy/server.py at line 671:

<comment>Indexing mode_to_str with the raw client mode will raise a KeyError when the value isn’t 1/2/3 (e.g., clients sending the string &quot;3&quot; worked before but now blows up). Please fall back gracefully when the key is missing so subscriptions keep working.</comment>

<file context>
@@ -614,22 +610,82 @@ async def subscribe_client(self, client_id, data):
+                            })
+                            continue
+                        else:
+                            logger.info(f&quot;First client subscribed to {symbol}.{exchange}.{mode_to_str[mode]}, broker subscribe successful&quot;)
+                    except Exception as e:
+                        self._remove_global_subscription(client_id, user_id, symbol, exchange, mode)
</file context>

Suggested change

	logger.info(f"First client subscribed to {symbol}.{exchange}.{mode_to_str[mode]}, broker subscribe successful")
	logger.info(f"First client subscribed to {symbol}.{exchange}.{mode_to_str.get(mode, mode)}, broker subscribe successful")

✅ Addressed in 963e9de

[cubic-dev-ai]

Listing COMMIT_MESSAGE.md as an added file is inaccurate because no file with that name is present in the repository.

Prompt for AI agents

Address the following comment on docs/zmq_new_audit_report.md at line 1192:

<comment>Listing `COMMIT_MESSAGE.md` as an added file is inaccurate because no file with that name is present in the repository.</comment>

<file context>
@@ -0,0 +1,1328 @@
+
+### Files Added
+- `ZMQ_Audit_Report.md` - This comprehensive audit report
+- `COMMIT_MESSAGE.md` - Detailed commit message
+
+### Dependencies
</file context>

✅ Addressed in 963e9de

[cubic-dev-ai]

The audit report claims the added file is ZMQ_Audit_Report.md, but the repository file is named docs/zmq_new_audit_report.md, so this reference is inaccurate.

Prompt for AI agents

Address the following comment on docs/zmq_new_audit_report.md at line 1191:

<comment>The audit report claims the added file is `ZMQ_Audit_Report.md`, but the repository file is named `docs/zmq_new_audit_report.md`, so this reference is inaccurate.</comment>

<file context>
@@ -0,0 +1,1328 @@
+- `server.py` - Core WebSocket proxy server (128 lines changed)
+
+### Files Added
+- `ZMQ_Audit_Report.md` - This comprehensive audit report
+- `COMMIT_MESSAGE.md` - Detailed commit message
+
</file context>

Suggested change

	- `ZMQ_Audit_Report.md` - This comprehensive audit report
	- `docs/zmq_new_audit_report.md` - This comprehensive audit report

✅ Addressed in 963e9de

[cubic-dev-ai]

This test only counts messages containing "ERROR", but the helper methods record unauthenticated request failures as "...FAILED", so error_count never increases and the test always reports failure even when behavior is correct.

Prompt for AI agents

Address the following comment on test/test_improved_zmq_server.py at line 984:

<comment>This test only counts messages containing &quot;ERROR&quot;, but the helper methods record unauthenticated request failures as &quot;...FAILED&quot;, so error_count never increases and the test always reports failure even when behavior is correct.</comment>

<file context>
@@ -0,0 +1,1364 @@
+            
+            time.sleep(1)
+            results = client.get_test_results()
+            error_count = sum(1 for result in results if &quot;ERROR&quot; in result)
+            
+            if error_count &gt; 0:
</file context>

Suggested change

	error_count = sum(1 for result in results if "ERROR" in result)
	error_count = sum(1 for result in results if "ERROR" in result or "FAILED" in result)

✅ Addressed in 963e9de

[cubic-dev-ai]

The test only logs message counts instead of asserting that other clients continue receiving data after one unsubscribes, so it will silently succeed even when the regression reappears. Please add explicit assertions that detect regressions.

Prompt for AI agents

Address the following comment on test/test_multi_client_fix.py at line 146:

<comment>The test only logs message counts instead of asserting that other clients continue receiving data after one unsubscribes, so it will silently succeed even when the regression reappears. Please add explicit assertions that detect regressions.</comment>

<file context>
@@ -0,0 +1,198 @@
+        
+        # Check that all clients are receiving data
+        for i, client in enumerate(clients):
+            market_data_count = len([msg for msg in client.received_messages if msg.get(&#39;type&#39;) == &#39;market_data&#39;])
+            logger.info(f&quot;Client {client.client_id} received {market_data_count} market data messages&quot;)
+        
</file context>

✅ Addressed in 963e9de