Response code when aggregate can't be created

[ajsutton]

/eth/v1/validator/aggregate_attestation may not be able to create an aggregate if the beacon node doesn't have any attestations with the specified attestation data root. In that case, currently both Teku and Lighthouse are returning a 404 response code but it's not actually listed as one of the possible response codes in the OpenAPI docs.

Also, while 404 isn't an unreasonable option, it has led to a bunch of confusion about whether the URL being requested was wrong (typically 4xx response codes mean the caller did something wrong). In this case the call was valid but there was nothing to return. I wonder if 204 No Content would be a better response code.

In any case, the OpenAPI docs should be updated to explicitly list what response code is used when no known attestations matching that data root are found.

[mpetrunic]

We could return 400 with some error code/description

[dapplion]

Not sure if this is still an issue? Lodestar returns 500 in the attestation data is not known.

The caller has agency to request an attestation data that it previously produced (correct spec), or request a random attestation data root. In the second case, it's the callers fault by requesting bad data. However I see @ajsutton point internal conditions of the beacon node can cause the aggregate to not be producible so should be a 500 code.

I think it's worth it to standarize either way, going for 500 for simplicity on the caller side

[ajsutton]

500 is an internal server error so I wouldn't think that's appropriate for the case to just not having data to return. 500 response indicates a bug or similar fault in the beacon node.

It's also entirely within spec to request an aggregate from a beacon node based on data that you haven't previously sent to that specific node. You might have published the attestation via one node and then requested the aggregate from a different one (for any number of reasons).

[dapplion]

Sometimes I wish we had a JSON RPC so we don't need to do semantical debates to choose a random status number lmao. Should it return status 418?

[ajsutton]

JSON-RPC has error codes too and they are even more poorly defined and lack standardisation. :)

I'd say either 404 or 204 are the appropriate responses. 404 is more technically accurate given that the request for an aggregate matching the specified data and nothing matching that request was found. Assuming the majority of clients are already returning 404 I'd just add that to the spec and be done with it. It's just unfortunate that 404 could mean "you typed the URL wrong" or "valid request but I don't have any data" but that's just the way the HTTP spec goes.

[mcdee]

I think that 204 is more technically accurate in this case. If the parameters were in the path a 404 would be reasonable, but the endpoint does indeed exist there is just no response to provide.

But returning 404 wouldn't be the end of the world, as you say if clients are already doing it we can live with it. There's little outside of validator clients that will hit this endpoint, and they'll be pretty robust in terms of handling the error whatever it is. So if that's what majority clients do let's standardize on that.

[nflaig]

I think that 204 is more technically accurate in this case. If the parameters were in the path a 404 would be reasonable, but the endpoint does indeed exist there is just no response to provide.

Looking at this purely from an implementation point of view, if the aggregate attestation can not be served it should be considered an error (4xx-5xx) as the server did not fulfill what the client asked for, and not a success by returning a 2xx status code. In general, I think it's not helpful if APIs return multiple success cases as it makes it harder to reason about and implement a generic http client, e.g. should the client retry the request on another node if a 204 is returned, for this API it's a good idea as another node might be able to serve the aggregate but for other APIs 204 might actually be a success and should not be retried.

So I think it's much easier to just have one success case (2xx) and multiple error cases (4xx-5xx). Having different behavior per status code that the client needs to handle just makes things more difficult and in an error case there are also barely any cases where it matters what kind of status code is returned outside from really specific ones like 415, 429 and maybe 503 (if node is syncing).

Unfortunately, for the getHeader, it's too late to get it right but would be good to avoid this in the future.

A note has been added in https://github.com/ethereum/beacon-APIs/pull/380 to return a 404 if aggregate can't be served, can we consider this closed?

[rolfyone]

Sounds like a fair way to resolve...