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

display custom error details for injected web3 #4059

Merged
merged 9 commits into from
Sep 18, 2023
Merged

Conversation

Aniket-Engg
Copy link
Collaborator

fixes #3765

@netlify
Copy link

netlify bot commented Sep 11, 2023

Deploy Preview for remixproject ready!

Name Link
🔨 Latest commit 5e3cc77
🔍 Latest deploy log https://app.netlify.com/sites/remixproject/deploys/650822fd6363b3000811b8ae
😎 Deploy Preview https://deploy-preview-4059--remixproject.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Aniket-Engg Aniket-Engg requested a review from yann300 September 11, 2023 14:35
@Aniket-Engg Aniket-Engg added ready-to-review PR ready to review and removed WIP labels Sep 12, 2023
@@ -173,3 +173,77 @@ export function checkVMError (execResult, compiledContracts) {
ret.message = `${error}\n${exceptionError}\n${msg}\nDebug the transaction to get more information.`
return ret
}

export function checkInjectedError (errorObj, compiledContracts) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you check the function checkVMError? could these be merged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. Please check

@@ -57,7 +57,7 @@ export function callFunction (from, to, data, value, gasLimit, funAbi, txRunner,
* @param {Object} execResult - execution result given by the VM
Copy link
Contributor

Choose a reason for hiding this comment

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

could you update the doc?

@@ -75,20 +75,20 @@ export function checkVMError (execResult, compiledContracts) {
error: false,
message: ''
}
if (!execResult.exceptionError) {
if (!execResult.exceptionError && !isInjectedWeb3) {
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like error and returnData are both the same but present under different properties... could you add a type execResult, so you don't have to do stuff like isInjectedWeb3 ? execResult.error : exceptionError

Copy link
Contributor

Choose a reason for hiding this comment

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

that way you may not need to have the isInjected

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think execResult is quite different for VM and injected Web3. For Injected web3, it is just an object having error details

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

@Aniket-Engg Aniket-Engg requested a review from yann300 September 14, 2023 16:19
@Aniket-Engg Aniket-Engg merged commit b72551e into master Sep 18, 2023
@Aniket-Engg Aniket-Engg deleted the cuserrInject branch September 18, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-review PR ready to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom error not working correctly.
2 participants