Fix HTTP transport codes

[hholst80]

I think it is incorrect to specify HTTP error code 4xx on a JSON RPC error, say APIError raised.

In this example https://github.com/bcb/jsonrpcserver/blob/master/examples/flask_server.py we will see status codes in the 400 range for JSON RPC errors even though the transport did not fail in any way.

I think this should be a 200 as the HTTP TRANSPORT did not fail. JSON-RPC 2.0 is designed to be agnostic to the transport protocol so if the transport did its job but the RPC call itself failed, the error should be communicated inside the HTTP payload and in JSON RPC, not relaying on transport error handling mechanisms.

https://www.simple-is-better.org/json-rpc/transport_http.html

Additional motivation:

External authentication handling. I return a 403 if the authentication failed. Anything that is not a 200 is not assumed to be a JSON RPC payload but something deferred outside the concern of the JSON RPC server endpoint. One exception is 202 which could be a reasonable status code for a JSON RPC notify. The dispatch method could flag this as a "notification event" that did not respond with a JSON RPC response.

Over all I think the status_code field should be removed as it mentally connects the two things, http transport and json rpc and that should be avoided.

[bcb]

Yes, you’re right. This should be fixed in a future major release.

Can you clarify your last point though, if we return http 2xx for everything, shouldn’t a status_code be included as part of the json-rpc (messaging) layer?

[bcb]

Somewhat unrelated, but I feel the “ApiError” exception should be changed as well, because we shouldn’t be raising exceptions to control flow, and to continue with the functional style of the rest of the library, instead we should be returning an error response. This can go in the same major release as fixing the http status codes.

[hholst80]

Can you clarify your last point though, if we return http 2xx for everything, shouldn’t a status_code be included as part of the json-rpc (messaging) layer?

That was maybe a bit unclear, sorry. I think it should be communicated back to the transport handling logic that there is no response from the RPC layer, like a flag ("RPC call was a notification") and that flag can be used to reply with 202 instead of a regular 200 for a normal RPC call with a response body. The 202 should of course not be communicated in itself because it is very specific to HTTP.

Having a helpful Flask example loop illustrating recommended use would probably be good!

[hholst80]

Somewhat unrelated, but I feel the “ApiError” exception should be changed as well, because we shouldn’t be raising exceptions to control flow, and to continue with the functional style of the rest of the library, instead we should be returning an error response. This can go in the same major release as fixing the http status codes.

I do not mind the ApiError raise TBH. But maybe it just as well could return a Response object that is sub-classed by "OK" and "Error" objects. Follow what is most common ("what would Standard Library; or Flask do?"). But I have no strong opinions on that one. I do think that the exposure of the status code object can influence implementors of JSON-RPC into implementing the "wrong" semantics.

[bcb]

I see what you mean. and it probably did influence the implementer in this case.

For ApiError, I've created a separate issue for that: #134

[bcb]

I found this (old) page on jsonrpc.org which has some suggested HTTP status codes for each JSON-RPC response - are these ok @hholst80 ?

https://www.jsonrpc.org/historical/json-rpc-over-http.html#response-codes

Also a long discussion here
https://groups.google.com/g/json-rpc/c/VNIH0WaxH5U

[bcb]

Is it better to leave this decision to the implementation?

We could remove the http_status_code attribute and let the user work it out.

[bcb]

Leaving http status code out of version 5 for now.

Adding it later won’t be a breaking change.

[hholst80]

Sorry for the slow response time on my part.I think it is reasonable to leave HTTP codes out for at least JSON-RPC 2.0.
I think the "spirit" of JSON-RPC 2.0 aligns well with having status codes deferred:

JSON-RPC 2.0 doesn't define any transport-specific issues, since transport and RPC are independent.

https://www.simple-is-better.org/rpc/#differences-between-1-0-and-2-0