Fixed JSONRPC error handling

include/fc/exception/exception.hpp

@@ -35,7 +35,8 @@ namespace fc
        aes_error_code                    = 18,
        overflow_code                     = 19,
        underflow_code                    = 20,
-       divide_by_zero_code               = 21
+       divide_by_zero_code               = 21,
+       method_not_found_exception_code   = 22
    };
 
    /**
@@ -273,6 +274,7 @@ namespace fc
   FC_DECLARE_EXCEPTION( key_not_found_exception, key_not_found_exception_code, "Key Not Found" );
   FC_DECLARE_EXCEPTION( bad_cast_exception, bad_cast_exception_code, "Bad Cast" );
   FC_DECLARE_EXCEPTION( out_of_range_exception, out_of_range_exception_code, "Out of Range" );
+  FC_DECLARE_EXCEPTION( method_not_found_exception, method_not_found_exception_code, "Method Not Found" );
 
   /** @brief if an operation is unsupported or not valid this may be thrown */
   FC_DECLARE_EXCEPTION( invalid_operation_exception,

include/fc/network/http/connection.hpp

@@ -25,6 +25,7 @@ namespace fc {
         enum status_code {
             OK                  = 200,
             RecordCreated       = 201,
+            NoContent           = 204,
             BadRequest          = 400,
             NotAuthorized       = 401,
             NotFound            = 404,
@@ -35,6 +36,7 @@ namespace fc {
         int                     status;
         std::vector<header>      headers;
         std::vector<char>        body;
+        std::string              body_as_string;
      };
      
      struct request 
@@ -79,4 +81,4 @@ namespace fc {
 
 #include <fc/reflect/reflect.hpp>
 FC_REFLECT( fc::http::header, (key)(val) )
-
+FC_REFLECT( fc::http::reply, (status)(headers)(body)(body_as_string) )

include/fc/network/http/websocket.hpp

@@ -4,6 +4,7 @@
 #include <string>
 #include <boost/any.hpp>
 #include <fc/network/ip.hpp>
+#include <fc/network/http/connection.hpp>
 #include <fc/signals.hpp>
 
 namespace fc { namespace http {
@@ -12,7 +13,7 @@ namespace fc { namespace http {
       class websocket_tls_server_impl;
       class websocket_client_impl;
       class websocket_tls_client_impl;
-   } // namespace detail;
+   } // namespace detail
 
    class websocket_connection
    {
@@ -21,10 +22,10 @@ namespace fc { namespace http {
          virtual void send_message( const std::string& message ) = 0;
          virtual void close( int64_t code, const std::string& reason  ){};
          void on_message( const std::string& message ) { _on_message(message); }
-         string on_http( const std::string& message ) { return _on_http(message); }
+         fc::http::reply on_http( const std::string& message ) { return _on_http(message); }
 
          void on_message_handler( const std::function<void(const std::string&)>& h ) { _on_message = h; }
-         void on_http_handler( const std::function<std::string(const std::string&)>& h ) { _on_http = h; }
+         void on_http_handler( const std::function<fc::http::reply(const std::string&)>& h ) { _on_http = h; }
 
          void        set_session_data( boost::any d ){ _session_data = std::move(d); }
          boost::any& get_session_data() { return _session_data; }
@@ -35,7 +36,7 @@ namespace fc { namespace http {
       private:
          boost::any                                _session_data;
          std::function<void(const std::string&)>   _on_message;
-         std::function<string(const std::string&)> _on_http;
+         std::function<fc::http::reply(const std::string&)> _on_http;
    };
    typedef std::shared_ptr<websocket_connection> websocket_connection_ptr;
 

include/fc/rpc/api_connection.hpp

@@ -112,13 +112,17 @@ namespace fc {
          variant call( const string& name, const variants& args )
          {
             auto itr = _by_name.find(name);
-            FC_ASSERT( itr != _by_name.end(), "no method with name '${name}'", ("name",name)("api",_by_name) );
+            if( itr == _by_name.end() )
+               FC_THROW_EXCEPTION( method_not_found_exception, "No method with name '${name}'",
+                                   ("name",name)("api",_by_name) );
             return call( itr->second, args );
          }
 
          variant call( uint32_t method_id, const variants& args )
          {
-            FC_ASSERT( method_id < _methods.size() );
+            if( method_id >= _methods.size() )
+               FC_THROW_EXCEPTION( method_not_found_exception, "No method with id '${id}'",
+                                   ("id",method_id)("api",_by_name) );
             return _methods[method_id](args);
          }
 

include/fc/rpc/state.hpp

@@ -6,9 +6,10 @@
 namespace fc { namespace rpc {
    struct request
    {
-      optional<uint64_t>  id;
+      optional<variant>   id;
       std::string         method;
       variants            params;
+      optional<std::string> jsonrpc;
    };
 
    struct error_object
@@ -20,12 +21,14 @@ namespace fc { namespace rpc {
 
    struct response
    {
-      response(){}
+      response() {}
-      response( int64_t i, fc::variant r ):id(i),result(r){}
+      response( const optional<variant>& _id, const variant& _result,
-      response( int64_t i, error_object r ):id(i),error(r){}
+                const optional<string>& version = optional<string>() )
-      response( int64_t i, fc::variant r, string j ):id(i),jsonrpc(j),result(r){}
+         : id(_id), jsonrpc(version), result(_result) {}
-      response( int64_t i, error_object r, string j ):id(i),jsonrpc(j),error(r){}
+      response( const optional<variant>& _id, const error_object& error,
-      int64_t                id = 0;
+                const optional<string>& version = optional<string>() )
+         : id(_id), jsonrpc(version), error(error) {}
+      optional<variant>      id;
       optional<std::string>  jsonrpc;
       optional<fc::variant>  result;
       optional<error_object> error;
@@ -44,7 +47,7 @@ namespace fc { namespace rpc {
          void    handle_reply( const response& response );
 
          request start_remote_call( const string& method_name, variants args );
-         variant wait_for_response( uint64_t request_id );
+         variant wait_for_response( const variant& request_id );
 
          void close();
 
@@ -52,12 +55,12 @@ namespace fc { namespace rpc {
 
       private:
          uint64_t                                                   _next_id = 1;
-         std::unordered_map<uint64_t, fc::promise<variant>::ptr>    _awaiting;
+         std::map<variant, fc::promise<variant>::ptr>               _awaiting;
          std::unordered_map<std::string, method>                    _methods;
          std::function<variant(const string&,const variants&)>      _unhandled;
    };
 } }  // namespace  fc::rpc
 
-FC_REFLECT( fc::rpc::request, (id)(method)(params) );
+FC_REFLECT( fc::rpc::request, (id)(method)(params)(jsonrpc) );
 FC_REFLECT( fc::rpc::error_object, (code)(message)(data) )
 FC_REFLECT( fc::rpc::response, (id)(jsonrpc)(result)(error) )

include/fc/rpc/websocket_api.hpp

@@ -1,9 +1,7 @@
 #pragma once
+#include <fc/network/http/websocket.hpp>
 #include <fc/rpc/api_connection.hpp>
 #include <fc/rpc/state.hpp>
-#include <fc/network/http/websocket.hpp>
-#include <fc/io/json.hpp>
-#include <fc/reflect/variant.hpp>
 
 namespace fc { namespace rpc {
 
@@ -26,9 +24,9 @@ namespace fc { namespace rpc {
             variants args = variants() ) override;
 
       protected:
-         std::string on_message(
+         response on_message( const std::string& message );
-            const std::string& message,
+         response on_request( const variant& message );
-            bool send_message = true );
+         void     on_response( const variant& message );
 
          std::shared_ptr<fc::http::websocket_connection>  _connection;
          fc::rpc::state                                   _rpc_state;

include/fc/variant.hpp

@@ -658,11 +658,11 @@ namespace fc
    variant operator - ( const variant& a, const variant& b );
    variant operator * ( const variant& a, const variant& b );
    variant operator / ( const variant& a, const variant& b );
-   variant operator == ( const variant& a, const variant& b );
+   bool    operator == ( const variant& a, const variant& b );
-   variant operator != ( const variant& a, const variant& b );
+   bool    operator != ( const variant& a, const variant& b );
-   variant operator < ( const variant& a, const variant& b );
+   bool    operator < ( const variant& a, const variant& b );
-   variant operator > ( const variant& a, const variant& b );
+   bool    operator > ( const variant& a, const variant& b );
-   variant operator ! ( const variant& a );
+   bool    operator ! ( const variant& a );
 } // namespace fc
 
 #include <fc/reflect/reflect.hpp>

src/network/http/websocket.cpp

@@ -12,7 +12,10 @@
 #include <websocketpp/extensions/permessage_deflate/disabled.hpp>
 #endif
 
+#include <fc/io/json.hpp>
 #include <fc/optional.hpp>
+#include <fc/reflect/variant.hpp>
+#include <fc/rpc/websocket_api.hpp>
 #include <fc/variant.hpp>
 #include <fc/thread/thread.hpp>
 #include <fc/asio.hpp>
@@ -239,10 +242,10 @@ namespace fc { namespace http {
                        wdump(("server")(request_body));
 
                        fc::async([current_con, request_body, con] {
-                          std::string response = current_con->on_http(request_body);
+                          fc::http::reply response = current_con->on_http(request_body);
-                          idump((response));
+                          idump( (response) );
-                          con->set_body( response );
+                          con->set_body( std::move( response.body_as_string ) );
-                          con->set_status( websocketpp::http::status_code::ok );
+                          con->set_status( websocketpp::http::status_code::value(response.status) );
                           con->send_http_response();
                           current_con->closed();
                        }, "call on_http");
@@ -364,8 +367,8 @@ namespace fc { namespace http {
                           wdump(("server")(con->get_request_body()));
                           auto response = current_con->on_http( con->get_request_body() );
                           idump((response));
-                          con->set_body( response );
+                          con->set_body( std::move( response.body_as_string ) );
-                          con->set_status( websocketpp::http::status_code::ok );
+                          con->set_status( websocketpp::http::status_code::value( response.status ) );
                        } catch ( const fc::exception& e )
                        {
                          edump((e.to_detail_string()));

src/rpc/state.cpp

@@ -29,7 +29,8 @@ variant state::local_call( const string& method_name, const variants& args )
 
 void  state::handle_reply( const response& response )
 {
-   auto await = _awaiting.find( response.id );
+   FC_ASSERT( response.id, "Response without ID: ${response}", ("response",response) );
+   auto await = _awaiting.find( *response.id );
    FC_ASSERT( await != _awaiting.end(), "Unknown Response ID: ${id}", ("id",response.id)("response",response) );
    if( response.result ) 
       await->second->set_value( *response.result );
@@ -48,7 +49,7 @@ request state::start_remote_call( const string& method_name, variants args )
    _awaiting[*request.id] = fc::promise<variant>::ptr( new fc::promise<variant>("json_connection::async_call") );
    return request;
 }
-variant state::wait_for_response( uint64_t request_id )
+variant state::wait_for_response( const variant& request_id )
 {
    auto itr = _awaiting.find(request_id);
    FC_ASSERT( itr != _awaiting.end() );

src/rpc/websocket_api.cpp

@@ -1,5 +1,6 @@
-
+#include <fc/reflect/variant.hpp>
 #include <fc/rpc/websocket_api.hpp>
+#include <fc/io/json.hpp>
 
 namespace fc { namespace rpc {
 
@@ -49,8 +50,30 @@ websocket_api_connection::websocket_api_connection( const std::shared_ptr<fc::ht
       return this->receive_call( 0, method_name, args );
    } );
 
-   _connection->on_message_handler( [this]( const std::string& msg ){ on_message(msg,true); } );
+   _connection->on_message_handler( [this]( const std::string& msg ){
-   _connection->on_http_handler( [this]( const std::string& msg ){ return on_message(msg,false); } );
+       response reply = on_message(msg);
+       if( _connection && ( reply.id || reply.result || reply.error || reply.jsonrpc ) )
+          _connection->send_message( fc::json::to_string( reply, fc::json::stringify_large_ints_and_doubles,
+                                                          _max_conversion_depth ) );
+   } );
+   _connection->on_http_handler( [this]( const std::string& msg ){
+       response reply = on_message(msg);
+       fc::http::reply result;
+       if( reply.error )
+       {
+          if( reply.error->code == -32603 )
+             result.status = fc::http::reply::InternalServerError;
+          else if( reply.error->code <= -32600 )
+             result.status = fc::http::reply::BadRequest;
+       }
+       if( reply.id || reply.result || reply.error || reply.jsonrpc )
+          result.body_as_string = fc::json::to_string( reply, fc::json::stringify_large_ints_and_doubles,
+                                                       _max_conversion_depth );
+       else
+          result.status = fc::http::reply::NoContent;
+
+       return result;
+   } );
    _connection->closed.connect( [this](){ closed(); } );
 }
 
@@ -96,21 +119,74 @@ void websocket_api_connection::send_notice(
                                                    _max_conversion_depth ) );
 }
 
-std::string websocket_api_connection::on_message(
+response websocket_api_connection::on_message( const std::string& message )
-   const std::string& message,
-   bool send_message /* = true */ )
 {
+   variant var;
    try
    {
-      auto var = fc::json::from_string(message, fc::json::legacy_parser, _max_conversion_depth);
+      var = fc::json::from_string( message, fc::json::legacy_parser, _max_conversion_depth );
-      const auto& var_obj = var.get_object();
+   }
+   catch( const fc::exception& e )
+   {
+      return response( variant(), { -32700, "Invalid JSON message", variant( e, _max_conversion_depth ) }, "2.0" );
+   }
+
+   if( var.is_array() )
+      return response( variant(), { -32600, "Batch requests not supported" }, "2.0" );
+
+   if( !var.is_object() )
+      return response( variant(), { -32600, "Invalid JSON request" }, "2.0" );
+
+   variant_object var_obj = var.get_object();
+
+   if( var_obj.contains( "id" )
+       && !var_obj["id"].is_string() && !var_obj["id"].is_numeric() && !var_obj["id"].is_null() )
+      return response( variant(), { -32600, "Invalid id" }, "2.0" );
+
+   if( var_obj.contains( "method" ) && ( !var_obj["method"].is_string() || var_obj["method"].get_string() == "" ) )
+      return response( variant(), { -32602, "Missing or invalid method" }, "2.0" );
+
+   if( var_obj.contains( "jsonrpc" ) && ( !var_obj["jsonrpc"].is_string() || var_obj["jsonrpc"] != "2.0" ) )
+      return response( variant(), { -32600, "Unsupported JSON-RPC version" }, "2.0" );
 
    if( var_obj.contains( "method" ) )
    {
-         auto call = var.as<fc::rpc::request>(_max_conversion_depth);
+      if( var_obj.contains( "params" ) && var_obj["params"].is_object() )
-         exception_ptr optexcept;
+         return response( variant(), { -32600, "Named parameters not supported" }, "2.0" );
-         try
+
+      if( var_obj.contains( "params" ) && !var_obj["params"].is_array() )
+         return response( variant(), { -32600, "Invalid parameters" }, "2.0" );
+
+      return on_request( std::move( var ) );
+   }
+
+   if( var_obj.contains( "result" ) || var_obj.contains("error") )
    {
+      if( !var_obj.contains( "id" ) || ( var_obj["id"].is_null() && !var_obj.contains( "jsonrpc" ) ) )
+         return response( variant(), { -32600, "Missing or invalid id" }, "2.0" );
+
+      on_response( std::move( var ) );
+
+      return response();
+   }
+
+   return response( variant(), { -32602, "Missing method or result or error" }, "2.0" );
+}
+
+void websocket_api_connection::on_response( const variant& var )
+{
+   _rpc_state.handle_reply( var.as<fc::rpc::response>(_max_conversion_depth) );
+}
+
+response websocket_api_connection::on_request( const variant& var )
+{
+   request call = var.as<fc::rpc::request>( _max_conversion_depth );
+   if( var.get_object().contains( "id" ) )
+      call.id = var.get_object()["id"]; // special handling for null id
+
+   // null ID is valid in JSONRPC-2.0 but signals "no id" in JSONRPC-1.0
+   bool has_id = call.id.valid() && ( call.jsonrpc.valid() || !call.id->is_null() );
+
    try
    {
 #ifdef LOG_LONG_API
@@ -130,47 +206,33 @@ std::string websocket_api_connection::on_message(
                ("m",call.method)("p",call.params)("t", end - start) );
 #endif
 
-               if( call.id )
+      if( has_id )
+         return response( call.id, result, call.jsonrpc );
+   }
+   catch ( const fc::method_not_found_exception& e )
    {
-                  auto reply = fc::json::to_string( response( *call.id, result, "2.0" ),
+      if( has_id )
-                                                    fc::json::stringify_large_ints_and_doubles,
+         return response( call.id, error_object{ -32601, "Method not found",
-                                                    _max_conversion_depth );
+                          variant( (fc::exception) e, _max_conversion_depth ) }, call.jsonrpc );
-                  if( send_message && _connection )
-                     _connection->send_message( reply );
-                  return reply;
-               }
-            }
-            FC_CAPTURE_AND_RETHROW( (call.method)(call.params) )
    }
    catch ( const fc::exception& e )
    {
-            if( call.id )
+      if( has_id )
+         return response( call.id, error_object{ e.code(), "Execution error", variant( e, _max_conversion_depth ) },
+                          call.jsonrpc );
+   }
+   catch ( const std::exception& e )
    {
-               optexcept = e.dynamic_copy_exception();
+      elog( "Internal error - ${e}", ("e",e.what()) );
+      return response( call.id, error_object{ -32603, "Internal error", variant( e.what(), _max_conversion_depth ) },
+                       call.jsonrpc );
    }
-         }
+   catch ( ... )
-         if( optexcept ) {
-
-               auto reply = fc::json::to_string( variant(response( *call.id, error_object{ 1, optexcept->to_string(), fc::variant(*optexcept, _max_conversion_depth)}, "2.0" ), _max_conversion_depth ),
-                                                 fc::json::stringify_large_ints_and_doubles, _max_conversion_depth );
-               if( send_message && _connection )
-                  _connection->send_message( reply );
-
-               return reply;
-         }
-      }
-      else
    {
-         auto reply = var.as<fc::rpc::response>(_max_conversion_depth);
+      elog( "Internal error while processing RPC request" );
-         _rpc_state.handle_reply( reply );
+      throw;
    }
-   }
+   return response();
-   catch ( const fc::exception& e )
-   {
-      wdump((e.to_detail_string()));
-      return e.to_detail_string();
-   }
-   return string();
 }
 
 } } // namespace fc::rpc

src/variant.cpp

@@ -678,7 +678,7 @@ void from_variant( const variant& var, std::vector<char>& vo, uint32_t max_depth
    void to_variant( unsigned long long int s, variant& v, uint32_t max_depth ) { v = variant( uint64_t(s)); }
 #endif
 
-   variant operator == ( const variant& a, const variant& b )
+   bool operator == ( const variant& a, const variant& b )
    {
       if( a.is_string()  || b.is_string() ) return a.as_string() == b.as_string();
       if( a.is_double()  || b.is_double() ) return a.as_double() == b.as_double();
@@ -687,7 +687,7 @@ void from_variant( const variant& var, std::vector<char>& vo, uint32_t max_depth
       return false;
    }
 
-   variant operator != ( const variant& a, const variant& b )
+   bool operator != ( const variant& a, const variant& b )
    {
       if( a.is_string()  || b.is_string() ) return a.as_string() != b.as_string();
       if( a.is_double()  || b.is_double() ) return a.as_double() != b.as_double();
@@ -696,12 +696,12 @@ void from_variant( const variant& var, std::vector<char>& vo, uint32_t max_depth
       return false;
    }
 
-   variant operator ! ( const variant& a )
+   bool operator ! ( const variant& a )
    {
       return !a.as_bool();
    }
 
-   variant operator < ( const variant& a, const variant& b )
+   bool operator < ( const variant& a, const variant& b )
    {
       if( a.is_string()  || b.is_string() ) return a.as_string() < b.as_string();
       if( a.is_double()  || b.is_double() ) return a.as_double() < b.as_double();
@@ -710,7 +710,7 @@ void from_variant( const variant& var, std::vector<char>& vo, uint32_t max_depth
       FC_ASSERT( false, "Invalid operation" );
    }
 
-   variant operator > ( const variant& a, const variant& b )
+   bool operator > ( const variant& a, const variant& b )
    {
       if( a.is_string()  || b.is_string() ) return a.as_string() > b.as_string();
       if( a.is_double()  || b.is_double() ) return a.as_double() > b.as_double();
@@ -719,7 +719,7 @@ void from_variant( const variant& var, std::vector<char>& vo, uint32_t max_depth
       FC_ASSERT( false, "Invalid operation" );
    }
 
-   variant operator <= ( const variant& a, const variant& b )
+   bool operator <= ( const variant& a, const variant& b )
    {
       if( a.is_string()  || b.is_string() ) return a.as_string() <= b.as_string();
       if( a.is_double()  || b.is_double() ) return a.as_double() <= b.as_double();

tests/api_tests.cpp

@@ -1,6 +1,7 @@
 #include <boost/test/unit_test.hpp>
 
 #include <fc/api.hpp>
+#include <fc/io/json.hpp>
 #include <fc/log/logger.hpp>
 #include <fc/rpc/api_connection.hpp>
 #include <fc/rpc/websocket_api.hpp>