polygon-edge
polygon-edge copied to clipboard
Inconsistent response from JSON-RPC GET method
Inconsistent response from JSON-RPC GET method
Description
While testing the JSON-RPC endpoints I decided to run
curl -X GET -H 'content-type: application/json' http://YOUR_NODE_URL
I was expecting a JSON response in a middleware I built but what I got was a plain text (not parseable to a JSON object), I went directly to repo's code and found this > https://github.com/0xPolygon/polygon-edge/blob/develop/jsonrpc/jsonrpc.go#L248-L252
This is not a big Issue but I'd suggest standardizing the response, and document it here > https://docs.polygon.technology/docs/edge/get-started/json-rpc-commands
Your environment
- OS and version: Kubernetes GCP/EKS
- version of the Polygon Edge: v0.4.1
- branch that causes this issue:
develop
Steps to reproduce
- Create a network with 1 Validator Node.
- From a terminal run
curl -X GET -H 'content-type: application/json' http://YOUR_NODE_URL
You will see the response is a String that is not parseable to a JSON Object.
Expected behaviour
If the Content-Type is set to application/json > https://github.com/0xPolygon/polygon-edge/blob/develop/jsonrpc/jsonrpc.go#L237 I would expect a JSON Object.
Actual behaviour
The response is just a plain text.
Logs
NA
Proposed solution
I'd suggest standardizing the response, and document it here > https://docs.polygon.technology/docs/edge/get-started/json-rpc-commands
Just to enrich the conversation. I think you could consider not handling HTTP GET method because in fact JSON-RPC expects a data object, meaning that it must be an HTTP POST.
Here's the spec https://www.jsonrpc.org/specification just in case you want to review it, but I'm sure you know it very well.
Hey @inglkruiz,
Thank you for opening up the issue.
Would you find it useful if the GET request to the root JSON-RPC url returns a JSON response that includes the name of your blockchain network (the one you set in the genesis.json)?
Hey @inglkruiz,
Thank you for opening up the issue.
Would you find it useful if the
GETrequest to the root JSON-RPC url returns a JSON response that includes the name of your blockchain network (the one you set in thegenesis.json)?
Hi @zivkovicmilos
Yes, I would. Possibly, other details might come handy, like the node's version. TBH I'm fine with anything as long as the response is a parseable JSON Object because the Header Content-Type is set to application/json.
Got it @inglkruiz, thank you for the quick reply.
I've talked with the team and decided that this makes sense - we've added it to the development pipeline.
Letting you know here as soon as we have it up on a PR 🙏
GET / is a very useful standard endpoint for health checks for load balances and downstream services. Is it really necessary to remove it?
Hey @mrwillis,
Not necessary at all to remove it. Seeing as we can't reach a conclusion on the PR, I will keep the GET endpoint, but make sure it returns a proper JSON response, instead of a string. This option requires some code gymnastics, but @ZeljkoBenovic also expressed concerns on removing it prematurely so it's worth looking into it
@mrwillis I would not say that GET / is a standard endpoint for health checks. As a node operator in the K8s space, I've found multiple services implement /healthz as the health check endpoint and mostly it goes on a different port.
As a reference, you could check Hyperledger Fabric common library and here the healthz module > https://github.com/hyperledger/fabric-lib-go/tree/main/healthz Also, here are the docs > https://github.com/hyperledger/fabric/blob/v2.4.4/docs/source/operations_service.rst#health-checks and for getting some inspiration on how they use it you can check Fabric's codebase > https://github.com/hyperledger/fabric/