Fixed error messages being wrapped in strings when using ganache or h…

[BelfordZ]

Note: Developers might have code that is similarily parsing out the underlying error in a similar way. This PR improves the returned errors, and breaks peoples code for handling the nested-in-a-string errors.

This was the only place I could find inside of extension that is handling errors that are returned in-band when calling sendtransactionRaw to ganache or harthat.

Fixes #12646

Before:

After:

Before:

After:

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[metamaskbot]

Builds ready [7d57d06]

• builds: chrome, firefox, opera
• builds (beta): chrome, firefox, opera
• builds (flask): chrome, firefox, opera
• build viz: Build System
• code coverage: Report
• storybook: Storybook
• all artifacts
• bundle viz:

  • background: 0, 1, 2, 3, 4, 5, 6, 7, 8
  • common: 0, 1, 2, 3, 4, 5, 6, 7, 8
  • content-script: 0
  • contentscript: contentscript
  • disable-console: disable-console
  • inpage: inpage
  • policy-load: policy-load
  • sentry-install: sentry-install
  • ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1710 ± 52 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	90	158	109	17	8
		domContentLoaded	1553	1904	1698	94	45
		load	1553	1985	1710	109	52
		domInteractive	1553	1904	1698	94	45

[wbt]

Having just today written even more code that parses out nested-in-string errors (lacking any other way to get the information) I am not a fan of the idea of just breaking this suddenly....there is a lot of DApp code with workarounds for this longstanding issue that will probably break.
Could this be done in a nonbreaking way, keeping content nested in the string in addition to in a more easily parseable format, plus a communications strategy to get the word out so people can start converting their code over, before some eventual breaking of lots of existing Dapp code?

[BelfordZ]

@wbt understandable and agreed. Closing this PR since the plan has changed:

1. We will address this inside of ethjs-query.
2. We wont change the error message or code, we will add a data field with the parsed json part of the current error.message.

In doing so, users who haven't yet written the code to parse out the underyling error will at least have the error.data which will contain what they really are after, and those who've already written code against these error messages don't get punished.

We should leave it like this until we have a well-defined process for incurring 'breaking for the better' changes (thx for the comment @wbt). There are some other fixes that have been held back for the same reason, so it's becoming a greater and greater priority to stamp out a process around the lifecycle of a breaking change to metamask public api. Developers should be given incrementally louder signals about upcoming changes so that they have a predictable and ample amount of time to update their code. We want to make sure we get that part right :)