JSONRPCMethodNotFoundError and others doesn't allow to set unique ID

[AndreiPashkin]

Specification says that when error is returned, ID must be set when it's possible to recover it:

id
This member is REQUIRED.
It MUST be the same as the value of 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.

https://www.jsonrpc.org/specification

But such exceptions as JSONRPCMethodNotFoundError or JSONRPCInvalidParamsError do not allow setting the ID. I encountered this problem when using JSONRPCProtocol class separately without other layers provided by the library.

[lnoor]

Hi, thanks for your report.

JSONRPCRequest.error_respond() will copy the id from the request into the error response.
See RPCDispatcher._dispatch() on how to use it.

If this doesn't help please provide more information, a code sample and/or pytest code demonstrating the problem.

Note that the specification is ambivalent in the case of error responses and notifications.
Consequently I chose not to send error responses for notifications.
Notifications are a 'fire and forget' mechanism.

[AndreiPashkin]

@lnoor, but error_respond() doesn't provide a way to set the error code. I'll provide a code snippet tomorrow.
But for now I can simply explain my use case as I just want to have a way to correctly format error response for that I currently use exceptions like JSONRPCMethodNotFoundError. But they do not have interface for setting response ID.

[AndreiPashkin]

@lnoor, here is a code snippet that represents my problem:

class Handler:
    """HTTP handler for some abstract hypothetical web-framework"""
    METHODS = {'method1', 'method2'}
    
    def __init__(self):
        self.__rpc = tinyrpc.protocols.jsonrpc.JSONRPCProtocol()
        
    def handle(self, http_request):
        """Handle HTTP request."""
        try:
            request = self.__rpc.parse_request(http_request.body)
        except tinyrpc.exc.BadRequestError as exc:
            response = exc.error_respond()
            return HTTPResonse(400, response.serialize())

        if request.method not in METHODS:
            # How do I construct a proper JSONRPC response here that would include a response ID?

[lnoor]

Warning: not tested code (just a pointer to get you going).

try:
    if request.method not in METHODS:
        raise JSONRPCMethodNotFoundError()
    # dispatch to method code

except RPCError as err:
    # handle any exception raised by system or your own code
    response = request.error_respond(err)
    return HTTPResponse(400, response.serialize())

Provided in your own code you raise exception derived from FixedErrorMessageMixin (see docs about creating your own exceptions) this should deal with all errors.

As a side note you may want to use HTTP response code 200 since you did return a valid reply and reserve the 40x codes for real HTTP statusses like "403 authorization required" etc.
See http://www.simple-is-better.org/json-rpc/transport_http.html for a proposal to use JSON RPC over HTTP.

Do let me know if this works for you as I'm holding off on the release of 1.0 to have this issue fixed.
After all, what you are doing is exactly how tinyrpc was designed: cooperating stand alone components.
So they better work. Or have better documentation :)

[AndreiPashkin]

@lnoor, in your example how exactly the unique ID is set in the response? JSON RPC spec requires it (at least in my interpretation).

[lnoor]

Yes the spec requires it.
The key MUST be present in the reply and its value MUST match that of the request.
Exceptions: the ID cannot be extracted from the request or the request is a notification.

JSONRPCProtocol.parse_request() sets the ID from the JSON request into the JSONRPCRequest object.
JSONRPCRequest.error_respond() in turn copies it in the JSONRPCErrorResponse object.
JSONRPCErrorResponse.serialize() turns it into JSON containing the ID.

[AndreiPashkin]

@lnoor, it seems like I overlooked existence of JSONRPCRequest.error_respond() method, thank you very much for pointing out! I think that the issue can be closed now.

[lnoor]

I marked it for improved documentation so I keep it open for a while.
But it is good to know it is resolved.