Refactor websocket logic

[aum-deriv]

Refactored logic for websockets and implemented it in Copier's Dashboard

[review-deriv]

AI Code Review

⚠️ IMPORTANT: This review is AI-generated and should be validated by human reviewers
🔴 BUGS & LOGICAL ISSUES:

1. Dashboard user type logic is overridden:
   • Issue description: In Dashboard.jsx, the conditional logic now always renders CopierDashboard when userType is not "copier". This means TraderDashboard is never rendered.
   • Potential impacts: Users attempting to switch to “Trader” view will remain on the “Copier” view, effectively breaking trader functionality.
   • Reproduction scenarios:
   – Load the Dashboard, click “Trader” button; the UI still displays CopierDashboard instead of TraderDashboard.
   • Fix implementation with code example:
   Replace the else branch with TraderDashboard to restore correct user type handling:

   } else if (userType === "copier") {
   return ;
   } else {
   return ;
   }

2. Removed server-side user type updates:
   • Issue description: The old code sent set_settings with allow_copiers: 1 or 0 to inform the backend that the user switched roles. This logic is removed, causing the server’s state not to match current userType.
   • Potential impacts: The backend may not handle user roles accurately. Server logic relying on allow_copiers could be broken.
   • Reproduction scenarios:
   – Switch role in the UI. The server is never notified, leading to inconsistent states for copies or trades.
   • Fix implementation with code example:
   const handleBecomeTrader = () => {
   updateSettings({ allow_copiers: 1 });
   setUserType("trader");
   };

   const handleBecomeCopier = () => {
   updateSettings({ allow_copiers: 0 });
   setUserType("copier");
   };

3. Potential stuck “isProcessing” in AddTraderForm:
   • Issue description: handleSubmit sets isProcessing=true and only unsets it in a useEffect listening for lastMessage.msg_type === "copy_start". If the server never returns a "copy_start" response or returns another error code, isProcessing can stay stuck.
   • Potential impacts: The user sees a loading or disabled UI indefinitely.
   • Reproduction scenarios:
   – Server returns a generic error or no response. The local form never unlocks.
   • Fix implementation with code example:
   if (lastMessage?.error) {
   setIsProcessing(false);
   setSnackbar(...);
   return;
   }
   if (lastMessage?.msg_type === "copy_start") {
   ...
   }

4. Missing handler for onRemoveTrader in CopierDashboard/trader card:
   • Issue description: The code passes onRemoveTrader={handleRemoveTrader} to TraderCard but does not define handleRemoveTrader in the shown snippet.
   • Potential impacts: Removing a trader might fail silently, or throw a runtime error if handleRemoveTrader is undefined.
   • Reproduction scenarios:
   – Attempt to remove a trader from the dashboard; handler is not found.
   • Fix implementation with code example (in CopierDashboard or top-level):
   const handleRemoveTrader = (trader) => {
   // Implement remove logic or API call here
   ...
   };

   ...
   <TraderCard
   ...
   onRemoveTrader={handleRemoveTrader}
   />

🟡 RELIABILITY CONCERNS:

1. Concurrent message handling:
   • Edge cases identified: Multiple requests in flight could overwrite each other in lastMessage or messageHandlers. If one request is slow, and another arrives, the callback might receive the wrong data.
   • Potential failure scenarios: The wrong callback is triggered for an unrelated response, or late responses override fresh data.
   • Mitigation steps with code examples:
   – Use unique request IDs (already done) and store them in local state to verify you’re handling the correct response:
   sendMessage(requestPayload, (resp) => {
   if (resp.req_id !== currentRequestId) return;
   // proceed
   });

2. Incomplete connection checks:
   • Potential failure scenarios: Code typically checks isConnected and isAuthorized, but the server could drop connection after sending a request. The subsequent logic might assume a valid connection.
   • Mitigation steps: Always handle onClose or check readiness before all subsequent calls. Possibly add reconnection or a fallback if the socket closes mid-request.

3. Ping mechanism vs. server timeouts:
   • Potential failure scenarios: If the server doesn’t respond to ping, the client might stay open indefinitely.
   • Mitigation steps: Provide a maximum retry or an error-handling routine if ping fails repeatedly.

💡 ROBUSTNESS IMPROVEMENTS:

1. Enhanced error handling in AddTraderForm:
   • Error handling enhancements: Check if traderData.token is empty or invalid. Show a user-friendly error immediately.
   • Code example:
   if (!traderData.token) {
   setSnackbar({ ... });
   return;
   }
   sendMessage(...);

2. Expand input validation in handleBecomeTrader/becameCopier:
   • Input validation additions: Confirm the user is actually allowed to switch roles, or that the user has a valid token.
   • Code example:
   if (!defaultAccount.token) {
   throw new Error("No valid token found for user.");
   }
   updateSettings({ allow_copiers: 1 });

3. Improve concurrency control with stable request–response mapping:
   • State management improvements: Store the particular req_id on each request so that lastMessage is only used if it matches that request’s req_id. This avoids collisions in shared lastMessage.
   • Code example:
   const [myRequestId, setMyRequestId] = useState(null);

   const handleSubmit = () => {
   const id = sendMessage({ copy_start: token }, onResponse);
   setMyRequestId(id);
   };

   useEffect(() => {
   if (lastMessage?.req_id === myRequestId) {
   // safe to handle response
   }
   }, [lastMessage, myRequestId]);

4. Consistency between useDerivWebSocket and useAuthorize:
   • Code example for each suggestion:
   – Use only one source of truth for “isAuthorizedGlobal” or let useAuthorize handle the entire auth flow, removing the duplicated logic in useDerivWebSocket.

These changes ensure logical correctness, make the code more reliable under edge conditions, and maintain consistent user experiences for both Trader and Copier roles.

[review-deriv]

AI Code Review

⚠️ IMPORTANT: This review is AI-generated and should be validated by human reviewers
🔴 BUGS & LOGICAL ISSUES:

1. Dashboard userType logic always renders CopierDashboard
   • Issue description: In Dashboard.jsx, the final ternary renders CopierDashboard for both "copier" and "trader" values. The code snippet:
   ) : userType === "copier" ? (
   
   ) : (
   
   )
   never shows TraderDashboard, even when userType is "trader".
   • Potential impacts: Trader-specific functionality (e.g., TraderDashboard) is never accessible, blocking intended workflows and features for trader accounts.
   • Reproduction scenarios:
   – Launch the app, connect, and click “Trader” button. The UI still displays CopierDashboard instead of TraderDashboard.
   • Fix implementation with code example:
   Change the fallback in the ternary to TraderDashboard, for example:
   ) : userType === "copier" ? (
   
   ) : (
   
   );

2. Unused / removed handleCopyClick in CopierDashboard
   • Issue description: The handleCopyClick function is removed or commented out, meaning there is no direct “start copying” action from CopierDashboard. This can confuse users if they expect to start copying a specific trader from within the dashboard cards.
   • Potential impacts: Users may only be able to initiate copying from AddTraderForm (if at all), preventing them from using previously available “Copy” button logic on individual TraderCards.
   • Reproduction scenarios:
   – Attempt to find a “Copy” button in the dashboard. The code logs “Copy clicked for trader:” in the removed function but no longer runs.
   • Fix implementation:
   Restore the handleCopyClick to send the "copy_start" request or remove references to “Copy” in TraderCard if the flow is truly replaced by AddTraderForm. For example:
   const handleCopyClick = (trader) => {
   if (!isConnected || !isAuthorized) {
   setSnackbar({ isVisible: true, message: "Please connect or authorize", status: "fail" });
   return;
   }
   setProcessingTrader(trader);
   sendMessage({ copy_start: trader.token }, response => setWsResponse(response));
   };

🟡 RELIABILITY CONCERNS:

1. No success/failure handling for “copy_stop” responses in CopierDashboard
   • Edge cases identified: handleStopCopy sets processingTrader and calls sendMessage but never checks the server’s response. If the WS call fails, the UI might not reflect this error properly.
   • Potential failure scenarios: User clicks “Stop Copy,” server fails or times out, but the UI remains in a partial state with no notification or fallback path.
   • Mitigation steps with example:
   Add a useEffect to watch wsResponse (or lastMessage) for “copy_stop” messages, similarly to how “copy_start” is handled. For example:
   useEffect(() => {
   if (!wsResponse || !processingTrader) return;
   if (wsResponse.msg_type === "copy_stop") {
   if (wsResponse.error) {
   setSnackbar({ isVisible: true, message: wsResponse.error.message, status: "fail" });
   } else {
   setSnackbar({ isVisible: true, message: Stopped copying ${processingTrader.id}, status: "neutral" });
   }
   setProcessingTrader(null);
   }
   }, [wsResponse, processingTrader]);

2. Potential duplication of authorization logic
   • Edge cases identified: useDerivWebSocket.js and useAuthorize.js both maintain separate isAuthorizedGlobal flags, which can lead to mismatched states if either is updated independently.
   • Potential failure scenarios: One hook might read “isAuthorized = true” while the other toggles it back to false, causing unpredictable flows or rejected requests.
   • Mitigation steps: Consolidate authorization flow in a single hook (useAuthorize), remove isAuthorizedGlobal from useDerivWebSocket, and rely on the standard isAuthorized from useAuthorize exclusively.

💡 ROBUSTNESS IMPROVEMENTS:

1. More explicit error handling during the initial list fetch
   • Error handling enhancements: In useCopyTradersList.js, fetchList silently returns if !isConnected or !isAuthorized. Consider surfacing a clear error/snackbar if the user tries to load the dashboard but is not authorized/connected:
   if (!isConnected || !isAuthorized) {
   setError("Not connected or authorized.");
   setIsLoading(false);
   return;
   }

2. Input validation in AddTraderForm
   • Input validation additions: Before sending { copy_start: traderData.token }, ensure traderData.token is non-empty and valid:
   if (!traderData.token.trim()) {
   setSnackbar({ isVisible: true, message: "Token cannot be empty.", status: "fail" });
   return;
   }

3. Single WebSocket strategy
   • State management improvements: If you intend to deprecate useDerivWebSocket entirely, remove it and unify all requests under useWebSocket to avoid parallel connections or mismatch in data handling. For instance, in existing components that still import useDerivWebSocket, switch them to useWebSocket for consistent messaging flow.

Example refactor (pseudocode):
// Remove or replace all references to useDerivWebSocket:
import useWebSocket from "../hooks/useWebSocket";
// ...
const { sendMessage, lastMessage, isConnected } = useWebSocket();

// Then handle relevant effects and requests based on lastMessage
// and connection status.

These changes will help ensure consistent authorization states, more robust error feedback, and correct rendering branches for Trader vs. Copier dashboards.