passportxyz / passport

Passport allows users to prove their identity through a secure, decentralized UI
Other
943 stars 451 forks source link

MDB Api Error handling #2849

Open nutrina opened 2 weeks ago

nutrina commented 2 weeks ago

Review & improve error handling in the MDB API

The expected behaviour is that of an error is encountered while processing a request from a user we should return a 500 code with a clean error message.

Right now, the behaviour apparently is that we return a "504 Service down error"

Possible errors:

Also we should document the errors thrown in different scenarios, and what we recommend for each.

Acceptance Criteria

GIVEN I am in the Passport App WHEN I try to retrieve any stamp related to an MDB modal AND and computing the score failed (for example the alchemy API threw an error) THEN I expect an error message telling me that determining the score failed AND I that I should try again in a few minutes

GIVEN I am making an API request for a non-aggregate model score ( http://127.0.0.1:8002/passport/analysis/0x4A13F4394cF05a52128BdA527664429D5376C67f?model_list=ethereum_activity ) WHEN the alchemy request fails THEN I expect an 500 error to be returned with a descriptive error message

GIVEN I am making an API request for an aggregate model score ( http://127.0.0.1:8002/passport/analysis/0x4A13F4394cF05a52128BdA527664429D5376C67f?model_list=aggregate ) WHEN any of the alchemy requests to get the individual model scores fail THEN I expect an 500 error to be returned with a descriptive error message explaining what failed (which model failed)

Tested Scenario - bad alchemy API key

I have configured an invalid alchemy API key

Request:

curl -X 'GET' \
  'http://127.0.0.1:8002/passport/analysis/0x4A13F4394cF05a52128BdA527664429D5376C67f?model_list=ethereum_activity' \
  -H 'accept: application/json' \
  -H 'X-API-Key: XXXX'

Actual

Response 200 with body:

{
  "address": "0x4A13F4394cF05a52128BdA527664429D5376C67f",
  "details": {
    "models": {
      "ethereum_activity": {
        "score": 0
      }
    }
  }
}

Expected

Response 500 with body similar to:

{
  "error": "Failed to retrieve the address history"
}

The problem seems to be the exception caught here is not handled and propagated to the API caller: https://github.com/passportxyz/Passport-Data-Science/blob/main/models/eth_stamp_model_v2/src/data/query_eth_node.py#L236-L238

tim-schultz commented 1 day ago

Here is a link to a cloudwatch query for errors that are being thrown from within the lambdas during batch execution and normal usage.