Skip to content
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

refactor: kill extrinsic errors, use metadata error matching instead #452

Merged
merged 7 commits into from
Jan 19, 2022

Conversation

rflechtner
Copy link
Contributor

@rflechtner rflechtner commented Jan 11, 2022

fixes KILTProtocol/ticket#1652

This makes use of module error matching based on chain metadata info.
Our internal extrinsic error matching is removed.
Minor changes necessary to get this done relate to the recovery by resign functionality, and the SubscriptionPromise maker.

How to test:

test & integration tests suffice

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)

@rflechtner rflechtner requested review from tjwelde and LeonFLK January 12, 2022 15:18
@rflechtner rflechtner self-assigned this Jan 12, 2022
Copy link
Contributor

@tjwelde tjwelde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks SO MUCH better!
Just some nitpicks

@rflechtner
Copy link
Contributor Author

while working on this I wrote this little gadget which I had included in the ErrorHandler.ts but not committed (yet):

export function throwExtrinsicError(extrinsicResult: ISubmittableResult): void {
  const { dispatchError } = extrinsicResult
  if (!dispatchError) return
  if (dispatchError.isModule) {
    const moduleError = dispatchError.registry.findMetaError(
      dispatchError.asModule
    )
    const { section, index, name, docs } = moduleError
    throw new ExtrinsicError(
      `Module Error ${section}.${name}(${index}): ${docs.join(' ')}`
    )
  }
  if (dispatchError.isToken || dispatchError.isArithmetic) {
    throw new ExtrinsicError(
      `${dispatchError.type} Error: ${(dispatchError.value as TokenError).type}`
    )
  }
  throw new ExtrinsicError(`${dispatchError.type} Error`)
}

It's not used or needed anywhere, do you think that's something users would want to use?

@rflechtner rflechtner requested a review from tjwelde January 12, 2022 19:20
@tjwelde
Copy link
Contributor

tjwelde commented Jan 18, 2022

Regarding #452 (comment):
Not sure about the usefullness. I imagine people will either want to show a generic message, when something goes wrong, or want to look at errors more in depth instead of just parsing an error message, if they want to handle something specific. I would leave it out of this PR.

Copy link
Contributor

@tjwelde tjwelde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rflechtner rflechtner merged commit b9d8882 into develop Jan 19, 2022
@rflechtner rflechtner deleted the rf-#1652 branch January 19, 2022 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants