Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beacon API v2.5.0 spec compliance #5710

Open
10 of 20 tasks
nflaig opened this issue Jun 26, 2023 · 2 comments
Open
10 of 20 tasks

Beacon API v2.5.0 spec compliance #5710

nflaig opened this issue Jun 26, 2023 · 2 comments
Labels
Epic Issues used as milestones and tracking multiple issues.

Comments

@nflaig
Copy link
Member

nflaig commented Jun 26, 2023

To be compliant to latest beacon API spec (v2.5.0) several updates are required.

v2.5.0

v2.4.2

v2.4.1

Nothing

v2.4.0

All the above issues can be tackled independently. A PR that implements a new API or adds new parameters needs to make sure to update the corresponding ignore list.

const ignoredOperations = [

const ignoredProperties: Record<string, IgnoredProperty> = {

@nflaig nflaig added the Epic Issues used as milestones and tracking multiple issues. label Jun 26, 2023
@jeluard jeluard changed the title Beacon API v2.4.1 spec compliance Beacon API v2.4.2 spec compliance Dec 7, 2023
@ensi321
Copy link
Contributor

ensi321 commented Dec 18, 2023

On a related note, at some point we should update the api error messages to be compliant with the Beacon API spec.

eg.

Expected

curl http://localhost:9596/eth/v1/beacon/states/current/root | jq
{
  "code": 400,
  "message": "Invalid state ID: current"
}

Actual

{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "Invalid block id 'current'"
}

Expected

curl "http://localhost:9596/eth/v1/beacon/pool/attestations?slot=current&committee_index=123" | jq
{
  "code": 400,
  "message": "Invalid slot: current"
}

Actual

[
  {
    "instancePath": "/slot",
    "schemaPath": "#/properties/slot/type",
    "keyword": "type",
    "params": {
      "type": "integer"
    },
    "message": "must be integer"
  }
]

Expected

curl http://localhost:9596/eth/v1/beacon/headers/123 | jq
{
  "code": 404,
  "message": "Block not found"
}

Actual

{
  "statusCode": 404,
  "error": "Not Found",
  "message": "No block found for id '123'"
}

@nflaig
Copy link
Member Author

nflaig commented Dec 18, 2023

On a related note, at some point we should update the api error messages to be compliant with the Beacon API spec.

I have looked at this before, it should be possible to transform the error before we send it

server.setErrorHandler((err, req, res) => {

Regarding the statusCode property, this is something fastify sets internally as well I did not find a good way to customize this. e.g. is even hard coded here, I don't think we can override those (other than adding custom default error handler)
fastify.js#L687-L711

body = `{"error":"${errorStatus}","message":"Client Timeout","statusCode":408}`

In case of default json schema error, would have to compile it in a single messages maybe something like

const { instancePath, message } = errors[0]
const message = `${instancePath.substring(instancePath.lastIndexOf("/") + 1)} ${message}`

Even though it would be good if we can support all properties I don't think anyone is actually looking at the json body of an error (unless API is used manually). Most important is the http status code and also res.text as this is used in Lodestar to display the error message.

const errBody = await res.text();
throw new HttpError(`${res.statusText}: ${getErrorMessage(errBody)}`, res.status, url.toString());

@nflaig nflaig changed the title Beacon API v2.4.2 spec compliance Beacon API v2.5.0 spec compliance Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Issues used as milestones and tracking multiple issues.
Projects
None yet
Development

No branches or pull requests

2 participants