lbryio / lbry-sdk

The LBRY SDK for building decentralized, censorship resistant, monetized, digital content apps.
https://lbry.com
MIT License
7.19k stars 483 forks source link

[area: jsonrpc] implement JSON-RPC 2.0 `"id"` field in daemon interaction #3493

Open unsuited1 opened 2 years ago

unsuited1 commented 2 years ago

I am currently working on a project in Rust that involves using the LBRY JSON-RPC by using lbrynet. If I am not mistaken the JSON-RPC 2.0 standard is documented here. In there, a field "id" is defined to map responses to requests.

The Server MUST reply with the same value in the Response object if included. This member is used to correlate the context between the two objects.

The lbry JSON-RPC API documentation clearly defines JSON-RPC 2.0 responses, but no "id" field is returned (I guess, the response is constructed here, however I am not really educated in python). The library I use generates an "id" here. My issue now is that I cannot use a strict JSON-RPC 2.0 library/crate to make requests to http://localhost:5279, because this expects a field "id" returned (I think that is serialized here in the Rust crate).

I could just make GET requests to http://localhost:5279 (following the curl example in the API docs), but that does not seem to be the proper way, because, why should there be a JSON-RPC 2.0 API in the first place if I cannot use it?

Am I missing something here? Sincerely, unsuited

EDIT: added serialize link, fixed typo

eukreign commented 2 years ago

This should be an easy fix; if you make a PR for this it will probably get merged (make sure to include tests).

unsuited1 commented 2 years ago

Thanks for your fast answer!

I would love to help, but as I said I cannot write python code. I can only (partially) read it, because it is pretty straight forward to read, but writing it stands on another page... I could try to write code, but I would first learn how to write tests in python etc. Additionally I consider "half-educated code" pretty dangerous though...

unsuited1 commented 2 years ago

Ok, I looked into the code a bit, here's what I guess needs to be done:

I forked the code and made a prototype commit to it. You can find the commit to my fork here.

The function that manages incoming RPC requests needs to pass a potential rpc_request_id to the function that constructs the response.

I changed the function on line 305 to use the rpc_request_id passed via **kwargs to keep the signature the same.

Furthermore I changed the function on line 603 to pass rpc_request_id as a keyword argument to the function jsonrpc_dumps_pretty above.

However, I do not know how to write professional python code or tests. I do not even know whether my code runs in that form. The commit changes just 5 lines, so it should be fairly easy for a more experienced developer to evaluate my changes.

Hoping that this helps you, unsuited

unsuited1 commented 2 years ago

I tried to run my version and (as expected), it did not work. Running ./lbrynet start in one terminal produced some warnings, but that seemed to be a problem with UPNP and my ports.

Running ./lbrynet status (with the daemon running in another terminal, obviously):

Could not process response from server:
Traceback (most recent call last):
  File "/home/user/lbry-sdk/lbry/extras/cli.py", line 33, in execute_command
    data = await resp.json()
  File "/home/user/lbry-sdk/lbry-venv/lib/python3.7/site-packages/aiohttp/client_reqrep.py", line 1027, in json
    headers=self.headers)
aiohttp.client_exceptions.ContentTypeError: 0, message='Attempt to decode JSON with unexpected mimetype: text/plain; charset=utf-8'

Running curl -d'{"method": "status", "params": {}, "id": 1}' http://localhost:5279/ (with the daemon running in another terminal):

500 Internal Server Error

Server got itself in trouble

This error also occurs when using a string, null or an integer as "id" and also when omitting the "id" field entirely.

expected behavior

The daemon returns the expected API response with a field "id" that equals the "id" that came along the request.