Enforce returning a Result from methods (#176)

Author: bcb

jsonrpcserver/dispatcher.py

@@ -25,7 +25,7 @@
     from_result,
     to_serializable,
 )
-from .result import InvalidParams, InternalError, Result
+from .result import InvalidParams, InternalError, Error, Result, Success
 
 default_deserializer = json.loads
 
@@ -55,7 +55,12 @@ def call(method: Callable, args: list, kwargs: dict) -> Result:
         return InvalidParams(errors)
 
     try:
-        return method(*args, **kwargs)
+        result = method(*args, **kwargs)
+        return (
+            InternalError("The method did not return a Result")
+            if not isinstance(result, (Success, Error))
+            else result
+        )
     except Exception as exc:  # Other error inside method - server error
         logging.exception(exc)
         return InternalError(str(exc))

jsonrpcserver/response.py

@@ -16,9 +16,14 @@
 """
 from typing import Any, List, NamedTuple, Union
 
-from . import status
 from .request import NOID
 from .result import Result, Success, UNSPECIFIED
+from .status import (
+    ERROR_INVALID_REQUEST,
+    ERROR_METHOD_NOT_FOUND,
+    ERROR_PARSE_ERROR,
+    ERROR_SERVER_ERROR,
+)
 
 
 class SuccessResponse(NamedTuple):
@@ -48,7 +53,7 @@ def ParseErrorResponse(data: Any) -> ErrorResponse:
     the id member in the Request Object.  If there was an error in detecting the id in
     the Request object (e.g. Parse error/Invalid Request), it MUST be Null."
     """
-    return ErrorResponse(status.JSONRPC_PARSE_ERROR_CODE, "Parse error", data, None)
+    return ErrorResponse(ERROR_PARSE_ERROR, "Parse error", data, None)
 
 
 def InvalidRequestResponse(data: Any) -> ErrorResponse:
@@ -57,19 +62,15 @@ def InvalidRequestResponse(data: Any) -> ErrorResponse:
     the id member in the Request Object.  If there was an error in detecting the id in
     the Request object (e.g. Parse error/Invalid Request), it MUST be Null."
     """
-    return ErrorResponse(
-        status.JSONRPC_INVALID_REQUEST_CODE, "Invalid request", data, None
-    )
+    return ErrorResponse(ERROR_INVALID_REQUEST, "Invalid request", data, None)
 
 
 def MethodNotFoundResponse(data: Any, id: Any) -> ErrorResponse:
-    return ErrorResponse(
-        status.JSONRPC_METHOD_NOT_FOUND_CODE, "Method not found", data, id
-    )
+    return ErrorResponse(ERROR_METHOD_NOT_FOUND, "Method not found", data, id)
 
 
 def ServerErrorResponse(data: Any, id: Any) -> ErrorResponse:
-    return ErrorResponse(status.JSONRPC_SERVER_ERROR_CODE, "Server error", data, id)
+    return ErrorResponse(ERROR_SERVER_ERROR, "Server error", data, id)
 
 
 def from_result(result: Result, id: Any) -> Union[Response, None]:

jsonrpcserver/result.py

@@ -25,7 +25,7 @@ def my_method(request: Request) -> Result:
 """
 from typing import Any, NamedTuple, Optional, Union
 
-from . import status
+from .status import ERROR_INVALID_PARAMS, ERROR_METHOD_NOT_FOUND, ERROR_INTERNAL_ERROR
 
 # This is used to indicate when a value isn't present. We use this instead of
 # None, because None is a valid JSON-serializable type.
@@ -47,12 +47,12 @@ class Error(NamedTuple):
 
 
 def InvalidParams(data: Any = UNSPECIFIED) -> Error:
-    return Error(status.JSONRPC_INVALID_PARAMS_CODE, "Invalid params", data)
+    return Error(ERROR_INVALID_PARAMS, "Invalid params", data)
 
 
 def MethodNotFound(data: Any) -> Error:
-    return Error(status.JSONRPC_METHOD_NOT_FOUND_CODE, "Method not found", data)
+    return Error(ERROR_METHOD_NOT_FOUND, "Method not found", data)
 
 
 def InternalError(data: Any) -> Error:
-    return Error(status.JSONRPC_INTERNAL_ERROR_CODE, "Internal error", data)
+    return Error(ERROR_INTERNAL_ERROR, "Internal error", data)

jsonrpcserver/status.py

@@ -67,9 +67,9 @@
 HTTP_INTERNAL_ERROR = 500
 
 # JSONRPC status codes from http://www.jsonrpc.org/specification#error_object
-JSONRPC_PARSE_ERROR_CODE = -32700
-JSONRPC_INVALID_REQUEST_CODE = -32600
-JSONRPC_METHOD_NOT_FOUND_CODE = -32601
-JSONRPC_INVALID_PARAMS_CODE = -32602
-JSONRPC_INTERNAL_ERROR_CODE = -32603
-JSONRPC_SERVER_ERROR_CODE = -32000
+ERROR_PARSE_ERROR = -32700
+ERROR_INVALID_REQUEST = -32600
+ERROR_METHOD_NOT_FOUND = -32601
+ERROR_INVALID_PARAMS = -32602
+ERROR_INTERNAL_ERROR = -32603
+ERROR_SERVER_ERROR = -32000

tests/test_dispatcher.py

@@ -3,7 +3,6 @@
 from typing import Any
 from unittest.mock import sentinel
 
-from jsonrpcserver import status
 from jsonrpcserver.dispatcher import (
     create_requests,
     default_deserializer,
@@ -13,9 +12,15 @@
     dispatch_to_response_pure,
 )
 from jsonrpcserver.methods import Methods, global_methods
-from jsonrpcserver.result import Result, Success
 from jsonrpcserver.request import Request, NOID
 from jsonrpcserver.response import ErrorResponse, SuccessResponse
+from jsonrpcserver.result import Result, Success, InvalidParams
+from jsonrpcserver.status import (
+    ERROR_INTERNAL_ERROR,
+    ERROR_INVALID_PARAMS,
+    ERROR_INVALID_REQUEST,
+    ERROR_PARSE_ERROR,
+)
 
 
 # def test_dispatch_to_response_pure_invalid_params_notification():
@@ -139,27 +144,29 @@ def test_dispatch_to_response_pure_notification():
     assert response is None
 
 
-def test_dispatch_to_response_pure_notification_invalid_jsonrpc():
+def test_dispatch_to_response_pure_invalid_json():
+    """Unable to parse, must return an error"""
     response = dispatch_to_response_pure(
         methods=Methods(ping),
         context=None,
         schema_validator=default_schema_validator,
         deserializer=default_deserializer,
-        request='{"jsonrpc": "0", "method": "notify"}',
+        request="{",
     )
     assert isinstance(response, ErrorResponse)
+    assert response.code == ERROR_PARSE_ERROR
 
 
-def test_dispatch_to_response_pure_invalid_json():
-    """Unable to parse, must return an error"""
+def test_dispatch_to_response_pure_notification_invalid_jsonrpc():
     response = dispatch_to_response_pure(
         methods=Methods(ping),
         context=None,
         schema_validator=default_schema_validator,
         deserializer=default_deserializer,
-        request="{",
+        request='{"jsonrpc": "0", "method": "notify"}',
     )
     assert isinstance(response, ErrorResponse)
+    assert response.code == ERROR_INVALID_REQUEST
 
 
 def test_dispatch_to_response_pure_invalid_jsonrpc():
@@ -172,6 +179,7 @@ def test_dispatch_to_response_pure_invalid_jsonrpc():
         request="{}",
     )
     assert isinstance(response, ErrorResponse)
+    assert response.code == ERROR_INVALID_REQUEST
 
 
 def test_dispatch_to_response_pure_invalid_params():
@@ -187,6 +195,7 @@ def foo(colour: str) -> Result:
         request='{"jsonrpc": "2.0", "method": "foo", "params": ["blue"], "id": 1}',
     )
     assert isinstance(response, ErrorResponse)
+    assert response.code == ERROR_INVALID_PARAMS
 
 
 def test_dispatch_to_response_pure_invalid_params_count():
@@ -201,7 +210,25 @@ def foo(colour: str, size: str):
         request='{"jsonrpc": "2.0", "method": "foo", "params": {"colour":"blue"}, "id": 1}',
     )
     assert isinstance(response, ErrorResponse)
-    assert response.data == "missing a required argument: 'size'"
+    assert response.code == ERROR_INVALID_PARAMS
+
+
+def test_dispatch_to_response_pure_enforcing_result():
+    """Methods should return a Result, otherwise we get an Internal Error response."""
+
+    def not_a_result():
+        return None
+
+    response = dispatch_to_response_pure(
+        methods=Methods(not_a_result),
+        context=None,
+        schema_validator=default_schema_validator,
+        deserializer=default_deserializer,
+        request='{"jsonrpc": "2.0", "method": "not_a_result", "id": 1}',
+    )
+    assert isinstance(response, ErrorResponse)
+    assert response.code == ERROR_INTERNAL_ERROR
+    assert response.data == "The method did not return a Result"
 
 
 # dispatch_to_response
@@ -306,7 +333,7 @@ def test_examples_invalid_json():
         request='[{"jsonrpc": "2.0", "method": "sum", "params": [1,2,4], "id": "1"}, {"jsonrpc": "2.0", "method"]',
     )
     assert isinstance(response, ErrorResponse)
-    assert response.code == status.JSONRPC_PARSE_ERROR_CODE
+    assert response.code == ERROR_PARSE_ERROR
 
 
 def test_examples_empty_array():
@@ -319,7 +346,7 @@ def test_examples_empty_array():
         deserializer=default_deserializer,
     )
     assert isinstance(response, ErrorResponse)
-    assert response.code == status.JSONRPC_INVALID_REQUEST_CODE
+    assert response.code == ERROR_INVALID_REQUEST
 
 
 def test_examples_invalid_jsonrpc_batch():
@@ -335,7 +362,7 @@ def test_examples_invalid_jsonrpc_batch():
         request="[1]",
     )
     assert isinstance(response, ErrorResponse)
-    assert response.code == status.JSONRPC_INVALID_REQUEST_CODE
+    assert response.code == ERROR_INVALID_REQUEST
 
 
 def test_examples_multiple_invalid_jsonrpc():
@@ -351,7 +378,7 @@ def test_examples_multiple_invalid_jsonrpc():
         request="[1, 2, 3]",
     )
     assert isinstance(response, ErrorResponse)
-    assert response.code == status.JSONRPC_INVALID_REQUEST_CODE
+    assert response.code == ERROR_INVALID_REQUEST
 
 
 def test_examples_mixed_requests_and_notifications():