fast-auth-signer
fast-auth-signer copied to clipboard
Listing out possible errors from MPC service
We need MPC to return human readable errors back so that near-discovery can consume and display on toast. Once it is done, we will make following changes on near discovery.
### Tasks
- [ ] List out possible errors from MPC
- [ ] Implement human readable errors on MPC
As we discussed, I have pull it out into a ticket and assigned it to you. In my knowledge, making this change won't break existing flow. (otherwise feel free to discuss here)
We definitely need to organize all of the possible non-500 errors from mpc-recovery and add them to the public API. What would you prefer this to look like though? Would something like this be fine?
{
"type": "error",
"code": 1,
"msg": "Account ID [email protected] is malformed"
}
or would you prefer something more structured so you can come up with a message on your end e.g.
{
"type": "error",
"code": 1,
"account_id": "[email protected]" // trailing fields are dynamic depending on what error code is, in this case we only need `account_id`
}
CC @volovyks looping you in here in case you want to give your opinion on the matter
Implement human readable errors on MPC
Also, just to clarify, I think they are reasonably human readable even right now. For example the most common error we return right now is fail: Error: oath_token <token> has already been used to register an account. You can only register 1 account per oath_token, which seems to not be displayed on the frontend side at all.
We definitely need to organize all of the possible non-500 errors from mpc-recovery and add them to the public API. What would you prefer this to look like though? Would something like this be fine?
{ "type": "error", "code": 1, "msg": "Account ID [email protected] is malformed" }or would you prefer something more structured so you can come up with a message on your end e.g.
{ "type": "error", "code": 1, "account_id": "[email protected]" // trailing fields are dynamic depending on what error code is, in this case we only need `account_id` }CC @volovyks looping you in here in case you want to give your opinion on the matter
Personally, I would prefer first approach because it simplifies the logic on receiving side. Unless we are definitely applying translation, we should go for first approach. Thoughts? @MaximusHaximus @DavidM-D
Implement human readable errors on MPC
Also, just to clarify, I think they are reasonably human readable even right now. For example the most common error we return right now is
fail: Error: oath_token <token> has already been used to register an account. You can only register 1 account per oath_token, which seems to not be displayed on the frontend side at all.
Oh great, didn't know. First of all, is all errors handled like above? Secondly, I think the wording is just fine to me, but to make sure it is more appropriate, these should be passed on to designers + managers to make sure that the wording is friendly enough to general public.
Oh great, didn't know. First of all, is all errors handled like above? Secondly, I think the wording is just fine to me, but to make sure it is more appropriate, these should be passed on to designers + managers to make sure that the wording is friendly enough to general public.
Yes, AFAIR we have human readable errors for all possible responses. Some of them might be more cryptic than the example above though.
@itegulov @hcho112 I think that the first option is fine, but I do not thik that we need "code" field. It's just an unredable duplication of the "type" field. And the type field is human redable BTW. I'm expecting it to be something like "IdTokenValidationFailure". In the end, we will have a finite list of types (strings) that FE can compare against.
{
"errorType": "BadAccountId",
"errorMsg": "Account ID [email protected] is malformed: accountId can only consist of numbers..."
}
@volovyks
By "type": "error" I just meant this is not "type": "ok". For the record, our current deployments return errors like this:
{"type":"err","msg":"failed to verify oidc token: InvalidToken"}
where type is a tag responsible for the enum variants, see this.
Agree with everything else though, we can replace code with something human-readable.