fix: make multiple api errors spec compliant

[har777]

Motivation

Api's returning multiple errors are not formatted according to the spec. This PR resolves that.

Description

Closes #6293

• [x] Add new IndexedError type to handle multiple errors
• [x] Update server to handle possible IndexedError
• [x] Update httpClient to handle possible IndexedError
• [x] Update submitPoolAttestations to use IndexedError
• [x] Update submitPoolBlsToExecutionChange to use IndexedError
• [x] Update submitPoolSyncCommitteeSignatures to use IndexedError
• [x] e2e tests checking response

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.72%. Comparing base (06210ef) to head (0acfc71). Report is 422 commits behind head on unstable.

Additional details and impacted files

@@            Coverage Diff            @@
##           unstable    #6479   +/-   ##
=========================================
  Coverage     61.72%   61.72%           
=========================================
  Files           555      555           
  Lines         58204    58206    +2     
  Branches       1839     1843    +4     
=========================================
+ Hits          35925    35928    +3     
+ Misses        22240    22239    -1     
  Partials         39       39           

[har777]

I'm thinking we can add something like an extra field to an Error object. And the handler checks and destructures the value of that key if it exists into the response object. That way if any error wants to return extra data, it can simply add an extra key to its Error object.

[nflaig]

I'm thinking we can add something like an extra field to an Error object.

I think more explicit solution is better as we don't have that many cases

• ApiError
• IndexedError
• Ajv error (err.validation)
• Any other Error

We could also consider handling errors thrown by ssz types e.g. JSON expected key ${jsonKey} is undefined in container.ts but unless ssz throws a specific validation error this will be hard to handle but definitely something we could do as well.

Also I noticed my suggestion to validate objects more strictly at the schema (ajv) level instead of just doing Schema.Object or Schema.ObjectArray is not ideal because we will be supporting ssz payloads (binary / bytes) for all APIs and in that case the validation has to be done by the ssz types.

[har777]

We could also consider handling errors thrown by ssz types e.g. JSON expected key ${jsonKey} is undefined in container.ts but unless ssz throws a specific validation error this will be hard to handle but definitely something we could do as well.

Yeah we should handle this in server.setErrorHandler. I think it's better to fix this + the error response keys together as part of tackling #6480 in a new PR.

[philknows]

Hi @har777 , are you still working on this PR?

[har777]

Hi @har777 , are you still working on this PR?

@philknows sorry I completely forgot about this one. I'll work on the comments this weekend and request a review if thats ok!

[matthewkeil]

Hi there @har777!! We are looking at including this code but wanted to see if you are still interested in taking it across the finish line. Feel free to reach out if you want to, or if you want us to finish this up. Thanks!

[nflaig]

Closed in favor of https://github.com/ChainSafe/lodestar/pull/7113