-
-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modify error handling for wallet_WatchAsset in NftController #1406
Conversation
} catch { | ||
errors.ownerFetchError = true; | ||
} catch (error) { | ||
throw rpcErrors.resourceUnavailable(`${error}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gudahtt @shanejonas I've put this unable to verify owner error as a resourceUnavailable
code for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be error.message
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2662aab
to
011613d
Compare
throw rpcErrors.invalidInput( | ||
'Suggested NFT is not owned by the selected account', | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gudahtt @shanejonas I've put this error as a invalidInput
code for now, since as Mark pointed out the dApp should not have sent a request for an asset the user does not own.
011613d
to
d663be0
Compare
* Modify error handling for wallet_WatchAsset in NftController
* Modify error handling for wallet_WatchAsset in NftController
Modifies the error handling pattern for the wallet_watchAsset NFT support introduced in #1173. In that approach we were passing certain errors in the
requestData
to theApprovalController
so that the errors could be displayed in the UI. We have elected to not pursue this approach because it diverges from the pattern of throwing these errors directly to the dApp we use everywhere else.