Some cleanup on the JSONRPC class.

3 files changed, 56 insertions, 123 deletions

diff --git a/indra/llcorehttp/lljsonrpcws.cpp b/indra/llcorehttp/lljsonrpcws.cpp
index 5de595c595..93e38a8397 100644
--- a/indra/llcorehttp/lljsonrpcws.cpp
+++ b/indra/llcorehttp/lljsonrpcws.cpp
@@ -118,18 +118,22 @@ void LLJSONRPCConnection::processMessage(const LLSD& message_obj)
         if (message_obj.has("method"))
         {
             // This is a request or notification
-            validateMessage(message_obj, true);
-            processRequest(message_obj);
+            if (validateMessage(message_obj, true))
+            {
+                processRequest(message_obj);
+            }
         }
         else if (message_obj.has("result") || message_obj.has("error"))
         {
             // This is a response
-            validateMessage(message_obj, false);
-            processResponse(message_obj);
+            if (validateMessage(message_obj, false))
+            {
+                processResponse(message_obj);
+            }
         }
         else
         {
-            throw InvalidRequest("Message must contain 'method' or 'result'/'error'");
+            LL_WARNS("JSONRPC") << "Message must contain 'method' or 'result'/'error'" << LL_ENDL;
         }
     }
     catch (const RPCError& e)
@@ -165,7 +169,6 @@ void LLJSONRPCConnection::processRequest(const LLSD& request)
         // Call the method handler with method name, ID, and parameters
         LLSD result = it->second(method, id, params);
 
-        // Send response (only for requests, not notifications)
         if (!is_notification)
         {
             sendResponse(id, result);
@@ -225,12 +228,13 @@ void LLJSONRPCConnection::processResponse(const LLSD& response)
     }
 }
 
-void LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
+bool LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
 {
     // Check JSON-RPC version
     if (!message.has("jsonrpc") || message["jsonrpc"].asString() != "2.0")
     {
-        throw InvalidRequest("Missing or invalid jsonrpc version");
+        LL_WARNS("JSONRPC") << "Missing or invalid jsonrpc version" << LL_ENDL;
+        return false;
     }
 
     if (is_request)
@@ -238,12 +242,14 @@ void LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
         // Request/notification validation
         if (!message.has("method"))
         {
-            throw InvalidRequest("Missing method field");
+            LL_WARNS("JSONRPC") << "Missing method field" << LL_ENDL;
+            return false;
         }
 
         if (!message["method"].isString())
         {
-            throw InvalidRequest("Method must be a string");
+            LL_WARNS("JSONRPC") << "Method must be a string" << LL_ENDL;
+            return false;
         }
 
         // Params are optional but must be array or object if present
@@ -251,7 +257,8 @@ void LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
         {
             if (!message["params"].isArray() && !message["params"].isMap())
             {
-                throw InvalidParams("Params must be array or object");
+                LL_WARNS("JSONRPC") << "Params must be array or object" << LL_ENDL;
+                return false;
             }
         }
     }
@@ -260,7 +267,8 @@ void LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
         // Response validation
         if (!message.has("id"))
         {
-            throw InvalidRequest("Response missing id field");
+            LL_WARNS("JSONRPC") << "Response missing id field" << LL_ENDL;
+            return false;
         }
 
         // Must have either result or error, but not both
@@ -269,12 +277,14 @@ void LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
 
         if (!has_result && !has_error)
         {
-            throw InvalidRequest("Response must have result or error");
+            LL_WARNS("JSONRPC") << "Response must have result or error" << LL_ENDL;
+            return false;
         }
 
         if (has_result && has_error)
         {
-            throw InvalidRequest("Response cannot have both result and error");
+            LL_WARNS("JSONRPC") << "Response cannot have both result and error" << LL_ENDL;
+            return false;
         }
 
         // Error must be an object with code and message
@@ -283,14 +293,15 @@ void LLJSONRPCConnection::validateMessage(const LLSD& message, bool is_request)
             LLSD error = message["error"];
             if (!error.isMap())
             {
-                throw InvalidRequest("Error must be an object");
+                LL_WARNS("JSONRPC") << "Error must be an object" << LL_ENDL;
             }
             if (!error.has("code") || !error.has("message"))
             {
-                throw InvalidRequest("Error must have code and message");
+                LL_WARNS("JSONRPC") << "Error must have code and message" << LL_ENDL;
             }
         }
     }
+    return true;
 }
 
 LLSD LLJSONRPCConnection::generateId()
@@ -307,13 +318,13 @@ LLSD LLJSONRPCConnection::generateId()
 void LLJSONRPCConnection::registerMethod(const std::string& method, MethodHandler handler)
 {
     mMethodHandlers[method] = handler;
-    LL_INFOS("JSONRPC") << "Registered method: " << method << LL_ENDL;
+    LL_DEBUGS("JSONRPC") << "Registered method: " << method << LL_ENDL;
 }
 
 void LLJSONRPCConnection::unregisterMethod(const std::string& method)
 {
     mMethodHandlers.erase(method);
-    LL_INFOS("JSONRPC") << "Unregistered method: " << method << LL_ENDL;
+    LL_DEBUGS("JSONRPC") << "Unregistered method: " << method << LL_ENDL;
 }
 
 LLSD LLJSONRPCConnection::call(const std::string& method, const LLSD& params, ResponseCallback callback)
@@ -344,14 +355,15 @@ LLSD LLJSONRPCConnection::call(const std::string& method, const LLSD& params, Re
         {
             mPendingRequests.erase(id.asString());
         }
-        throw InternalError("Failed to send request");
+        LL_WARNS("JSONRPC") << "Failed to send request" << LL_ENDL;
+        return LLSD();
     }
 
     LL_DEBUGS("JSONRPC") << "Sent request: " << method << " with id: " << id.asString() << LL_ENDL;
     return id;
 }
 
-void LLJSONRPCConnection::notify(const std::string& method, const LLSD& params)
+bool LLJSONRPCConnection::notify(const std::string& method, const LLSD& params)
 {
     LLSD notification;
     notification["jsonrpc"] = "2.0";
@@ -366,13 +378,15 @@ void LLJSONRPCConnection::notify(const std::string& method, const LLSD& params)
 
     if (!sendMessage(LlsdToJson(notification)))
     {
-        throw InternalError("Failed to send notification");
+        LL_WARNS("JSONRPC") << "Failed to send notification" << LL_ENDL;
+        return false;
     }
 
     LL_DEBUGS("JSONRPC") << "Sent notification: " << method << LL_ENDL;
+    return true;
 }
 
-void LLJSONRPCConnection::sendResponse(const LLSD& id, const LLSD& result)
+bool LLJSONRPCConnection::sendResponse(const LLSD& id, const LLSD& result)
 {
     LLSD response;
     response["jsonrpc"] = "2.0";
@@ -382,14 +396,13 @@ void LLJSONRPCConnection::sendResponse(const LLSD& id, const LLSD& result)
     if (!sendMessage(LlsdToJson(response)))
     {
         LL_WARNS("JSONRPC") << "Failed to send response for id: " << id.asString() << LL_ENDL;
+        return false;
     }
-    else
-    {
-        LL_DEBUGS("JSONRPC") << "Sent response for id: " << id.asString() << LL_ENDL;
-    }
+    LL_DEBUGS("JSONRPC") << "Sent response for id: " << id.asString() << LL_ENDL;
+    return true;
 }
 
-void LLJSONRPCConnection::sendError(const LLSD& id, const RPCError& error)
+bool LLJSONRPCConnection::sendError(const LLSD& id, const RPCError& error)
 {
     LLSD response;
     response["jsonrpc"] = "2.0";
@@ -409,18 +422,18 @@ void LLJSONRPCConnection::sendError(const LLSD& id, const RPCError& error)
     if (!sendMessage(LlsdToJson(response)))
     {
         LL_WARNS("JSONRPC") << "Failed to send error response" << LL_ENDL;
+        return false;
     }
-    else
-    {
-        LL_DEBUGS("JSONRPC") << "Sent error response: " << error.what() << LL_ENDL;
-    }
+    LL_DEBUGS("JSONRPC") << "Sent error response: " << error.what() << LL_ENDL;
+    return true;
 }
 
-void LLJSONRPCConnection::sendBatch(const LLSD& batch, ResponseCallback callback)
+bool LLJSONRPCConnection::sendBatch(const LLSD& batch, ResponseCallback callback)
 {
     if (!batch.isArray() || batch.size() == 0)
     {
-        throw InvalidRequest("Batch must be non-empty array");
+        LL_WARNS("JSONRPC") << "Batch must be non-empty array" << LL_ENDL;
+        return false;
     }
 
     // For batch requests with callbacks, we need to track multiple responses
@@ -433,10 +446,12 @@ void LLJSONRPCConnection::sendBatch(const LLSD& batch, ResponseCallback callback
 
     if (!sendMessage(LlsdToJson(batch)))
     {
-        throw InternalError("Failed to send batch");
+        LL_WARNS("JSONRPC") << "Failed to send batch" << LL_ENDL;
+        return false;
     }
 
     LL_DEBUGS("JSONRPC") << "Sent batch with " << batch.size() << " messages" << LL_ENDL;
+    return true;
 }
 
 //========================================================================
diff --git a/indra/llcorehttp/lljsonrpcws.h b/indra/llcorehttp/lljsonrpcws.h
index 54578c0e93..bd9939aa33 100644
--- a/indra/llcorehttp/lljsonrpcws.h
+++ b/indra/llcorehttp/lljsonrpcws.h
@@ -230,6 +230,7 @@ public:
             : RPCError(INVALID_SESSION, details) {}
     };
 
+
     LLJSONRPCConnection(const LLWebsocketMgr::WSServer::ptr_t server,
                        const LLWebsocketMgr::connection_h& handle)
         : LLWebsocketMgr::WSConnection(server, handle) {}
@@ -269,28 +270,28 @@ public:
      * @param method The method name
      * @param params The parameters to pass
      */
-    void notify(const std::string& method, const LLSD& params = LLSD());
+    bool notify(const std::string& method, const LLSD& params = LLSD());
 
     /**
      * @brief Send a successful response to a request
      * @param id The request ID from the original request
      * @param result The result to return
      */
-    void sendResponse(const LLSD& id, const LLSD& result);
+    bool sendResponse(const LLSD& id, const LLSD& result);
 
     /**
      * @brief Send an error response to a request
      * @param id The request ID from the original request (can be null)
      * @param error The RPCError to send
      */
-    void sendError(const LLSD& id, const RPCError& error);
+    bool sendError(const LLSD& id, const RPCError& error);
 
     /**
      * @brief Send a batch of requests/notifications
      * @param batch Array of request/notification objects
      * @param callback Callback for batch response (optional)
      */
-    void sendBatch(const LLSD& batch, ResponseCallback callback = nullptr);
+    bool sendBatch(const LLSD& batch, ResponseCallback callback = nullptr);
 
 protected:
     /**
@@ -315,9 +316,8 @@ protected:
      * @brief Validate a JSON-RPC message structure
      * @param message The message to validate
      * @param is_request True if validating a request, false for response
-     * @throw InvalidRequest if validation fails
      */
-    void validateMessage(const LLSD& message, bool is_request = true);
+    bool validateMessage(const LLSD& message, bool is_request = true);
 
     /**
      * @brief Generate the next unique request ID
diff --git a/indra/llcorehttp/llwebsocketmgr.cpp b/indra/llcorehttp/llwebsocketmgr.cpp
index 09eb728500..addd108b71 100644
--- a/indra/llcorehttp/llwebsocketmgr.cpp
+++ b/indra/llcorehttp/llwebsocketmgr.cpp
@@ -181,38 +181,8 @@ void LLWebsocketMgr::stopAllServers()
 }
 
 //------------------------------------------------------------------------
-/**
- * @struct Server_impl
- * @brief Internal implementation wrapper for websocketpp server functionality
- *
- * This structure serves as a PIMPL (Pointer to Implementation) wrapper around
- * the websocketpp::server template, providing a clean interface between the
- * high-level WSServer class and the low-level websocketpp library. It handles
- * all direct websocketpp interactions including server lifecycle management,
- * event handling, and connection management.
- *
- * The Server_impl follows Linden Lab conventions while maintaining compatibility
- * with the websocketpp library. It provides thread-safe operations where possible
- * and integrates with the existing logging infrastructure.
- *
- * @note This class uses the websocketpp::config::asio configuration which provides
- * ASIO-based networking without TLS/SSL support.
- */
 struct Server_impl
 {
-    /**
-     * @brief Constructor - Initializes the websocketpp server with configuration
-     * @param owner Pointer to the owning WSServer instance (must not be null)
-     * @param port The port number to bind the server to (1-65535)
-     * @param local_only If true, binds only to localhost; if false, binds to all interfaces
-     *
-     * Sets up the websocketpp server instance and registers lambda-based event handlers
-     * for connection open, close, and message events. The handlers delegate back to the
-     * owning WSServer instance for processing, maintaining the abstraction layer.
-     *
-     * @warning The owner pointer must remain valid for the lifetime of this object
-     * @pre port must be a valid port number (typically > 1024 for non-privileged access)
-     */
     Server_impl(LLWebsocketMgr::WSServer *owner, U16 port, bool local_only) :
         mOwner(owner),
         mPort(port),
@@ -234,10 +204,6 @@ struct Server_impl
      * on the specified port. The binding behavior depends on the mLocalOnly flag:
      * - If mLocalOnly is true: binds to "127.0.0.1" (localhost only)
      * - If mLocalOnly is false: binds to all available network interfaces
-     *
-     * @note This method must be called before attempting to start the server
-     * @pre The server must not already be initialized
-     * @post The server is ready to accept connections when start() is called
      */
     void init()
     {
@@ -257,16 +223,6 @@ struct Server_impl
     /**
      * @brief Start the websocket server and begin accepting connections
      * @return true if server started successfully, false on error
-     *
-     * Runs a controlled event loop that periodically checks the stop flag for clean shutdown.
-     * Instead of calling run() once and blocking indefinitely, this implementation uses
-     * run_for() with a timeout to process events in chunks, checking mOwner->mShouldStop between
-     * iterations to allow for responsive termination.
-     *
-     * @note This method blocks the calling thread until the server stops
-     * @pre init() must have been called successfully
-     * @post On success, the server is actively accepting connections
-     * @warning Any exceptions during startup are caught and logged as warnings
      */
     bool start()
     {
@@ -317,21 +273,6 @@ struct Server_impl
         }
     }
 
-    /**
-     * @brief Stop the websocket server and cease accepting new connections
-     *
-     * Gracefully shuts down the server by first stopping the listener to prevent
-     * new connections, then stopping the ASIO event loop. Existing connections
-     * may remain active briefly during the shutdown process.
-     *
-     * The method performs a safe shutdown by checking if the server is already
-     * stopped before attempting shutdown operations. Any exceptions during shutdown
-     * are caught and logged but do not propagate.
-     *
-     * @note This method is non-blocking and safe to call multiple times
-     * @post The server will no longer accept new connections
-     * @post Existing connections will be cleanly terminated
-     */
     void stop()
     {
         if (mServer.stopped())
@@ -356,10 +297,6 @@ struct Server_impl
      * Called automatically by the websocketpp library when a new client connection
      * is successfully established. This method serves as a bridge between the
      * low-level websocketpp callback and the high-level WSServer interface.
-     *
-     * @note This is an internal callback method called by websocketpp
-     * @pre mOwner must be valid (assertion will fail if null)
-     * @post The owning WSServer will be notified of the new connection
      */
     void onOpen(websocketpp::connection_hdl hdl) const
     {
@@ -371,14 +308,6 @@ struct Server_impl
     /**
      * @brief Handle connection closure event
      * @param hdl WebSocket connection handle from websocketpp
-     *
-     * Called automatically by the websocketpp library when a client connection
-     * is closed, either by the client, server, or due to a network error.
-     * This method delegates the event to the owning WSServer for processing.
-     *
-     * @note This is an internal callback method called by websocketpp
-     * @pre mOwner must be valid (assertion will fail if null)
-     * @post The owning WSServer will be notified of the connection closure
      */
     void onClose(websocketpp::connection_hdl hdl) const
     {
@@ -395,15 +324,7 @@ struct Server_impl
      * received from a client. This method validates the connection exists, extracts
      * the message payload, and forwards it to the appropriate connection handler.
      *
-     * Currently handles text messages only. Binary message support and message
-     * fragmentation handling are noted as TODO items for future implementation.
-     *
-     * @note This is an internal callback method called by websocketpp
-     * @pre mOwner must be valid (assertion will fail if null)
-     * @post If connection exists, the message is forwarded for processing
-     * @todo Add support for binary messages
-     * @todo Implement fragmented message handling
-     * @todo Process connection close codes for graceful closure
+     * Currently handles text messages only.
      */
     void onMessage(websocketpp::connection_hdl hdl, Server_t::message_ptr msg) const
     {
@@ -417,7 +338,6 @@ struct Server_impl
 
         // TODO: check the FIN bit and handle fragmented messages if needed
         // TODO: check terminal and close codes and handle connection closure if needed
-        // TODO: handle binary messages if needed
         mOwner->handleMessage(hdl, msg->get_payload());
     }
 
@@ -505,14 +425,12 @@ void LLWebsocketMgr::WSServer::stop()
 
         LL_INFOS("WebSocket") << "Stopping WebSocket server: " << mServerName << LL_ENDL;
 
-        // Signal the thread to stop
         mShouldStop = true;
 
         // Stop the websocket server (this will cause the controlled run loop to exit)
         mImpl->stop();
     } // Release the lock here
 
-    // Wait for the thread to finish (outside the lock to avoid deadlock)
     if (mServerThread.joinable())
     {
         mServerThread.join();