wharfkit / antelope

Core types, client interfaces, and other tools for working with Antelope-based blockchains.
Other
42 stars 23 forks source link

Missing error details on API error #32

Open theblockstalk opened 1 year ago

theblockstalk commented 1 year ago

The error created from an error response from API calls is missing and not printing extra information in the json.error.details object that would be useful for developers to see. Only the json.error.name and json.error.what is printed.

Error response from v1.chainpush_transaction:

{
      headers: {
        'access-control-allow-headers': '*',
        'access-control-allow-origin': '*',
        connection: 'close',
        'content-length': '531',
        'content-type': 'application/json',
        server: 'WebSocket++/0.7.0'
      },
      status: 500,
      json: {
        code: 500,
        message: 'Internal Service Error',
        error: {
          code: 3090003,
          name: 'unsatisfied_authorization',
          what: 'Provided keys, permissions, and delays do not satisfy declared authorizations',
          details: [
            {
              message: "transaction declares authority '${auth}', but does not have signatures for it under a provided delay of 0 ms, provided permissions ${provided_permissions}, provided keys ${provided_keys}, and a delay max limit of 3888000000 ms",
              file: 'authorization_manager.cpp',
              line_number: 532,
              method: 'check_authorization'
            }
          ]
        }
      },
      text: '{"code":500,"message":"Internal Service Error","error":{"code":3090003,"name":"unsatisfied_authorization","what":"Provided keys, permissions, and delays do not satisfy declared authorizations","details":[{"message":"transaction declares authority \'${auth}\', but does not have signatures for it under a provided delay of 0 ms, provided permissions ${provided_permissions}, provided keys ${provided_keys}, and a delay max limit of 3888000000 ms","file":"authorization_manager.cpp","line_number":532,"method":"check_authorization"}]}}'
    }
theblockstalk commented 1 year ago

The information exists inside the thrown error, just not printed. Maybe consider that it is printed to cater for ignorant devs (oops).

Suggest this issue is closed.

aaroncox commented 1 year ago

Displaying the details as it outputs the error is something I think would be useful as well. It's caught me off guard a few times where I have to manually go try queries or dig deeper into the error messages to find the actual root of the error.

@jnordberg any immediate thoughts on how we could improve this?

jnordberg commented 1 year ago

We should improve this function. It tries to pluck out the relevant into to the error message string but could be a lot better.

ericpassmore commented 1 year ago

I would like to work on this. Current thinking is to return error.details[0].message even when what is defined.

@theblockstalk can you provide more examples to test against?

ericpassmore commented 1 year ago

Delving into the code, and now have a new/better understanding. I dove in and updated the method. It didn't work out too well as falsy values created verbose code, and forced isRejected tests on promises . @jnordberg Proposing a solution here, as always open to other ways of doing things.

Lots of alternatives.

ericpassmore commented 1 year ago

Regarding the scope of APIErrorData, I'm not sure we need to check every permutation. From looking at logs there are only a few data values we care about. 1) Has nodeos Error.code, Error.what, and Error.details[].message 2) No nodeos Error.code, no Error.what, no Error.details[]

We can consider one additional case, to make the code more robust 3) Has Error.code and may or may not have Error.what or Error.details[].message

Simplifying what cases to support will collapse the number of if/else statements. In the case of number two above, APIError would never be created. Thats ok because errors can come from anywhere, there is no guarantee of APIError, and consumers of eosio.core always need to instanceof check the returned error.

ericpassmore commented 1 year ago

Still have this on my queue. Will submit new PR with hopefully, improved Json error parsing and simpler testing.

aaroncox commented 1 year ago

I ran into this again today while working on the @wharfkit session library. When casting an error thrown by the APIClient (e.g. String(error)) it will not show the details, however the details can be accessed through the .response properly on the error. I wanted to record this alternative approach for anyone searching for answers today.

Example:

try {
    // use APIClient for something
} catch (error: any) {
    console.log(error.response.json) // the API response
}

Improving this call like @jnordberg suggested is probably still the right path forward, but is not completed yet. I suspect that the inconsistencies in error responses from nodeos may cause that to be a rather lengthy undertaking to address from the client side.