search

MetaMask / metamask-snaps-beta

Fork of MetaMask that supports plugins! Read the Wiki!
https://github.com/MetaMask/metamask-snaps-beta/wiki
MIT License
144 stars 57 forks source link

No stack when evaluating plugin in SES #84

Closed jvluso closed 5 years ago

jvluso commented 5 years ago

I'm trying to add a dependency to a plugin that I'm building, and when I run build I get the error possible html comment syntax rejected around line 30033 I'm pretty sure that this means that there is something in the dependency which isn't able to be run in the ses container, but it would be helpful to have a stack trace so that I know what needs to be fixed. I already made a comment on the realms-shim repository but I also want to confirm that this isn't because of the TODO: mock wallet properly comment.

rekmarks commented 5 years ago

The around line XXX message appears to be the closest thing we can get to a stack when evaluating something in SES. I believe the reason this particular error isn't handled by our build script is because SES did not check for HTML comment syntax when we first wrote it. (They started to do so because of a critical security vulnerability related to HTML comments.)

If you share a link to your repo or invite me as a collaborator, I can help troubleshoot it, and possibly add a handler to our build script.

Otherwise, I recommend that you look inside the bundle around that line and manually remove the HTML comment. If you could at least share a snippet of the offending line(s), we would really appreciate it.

jvluso commented 5 years ago

https://github.com/jvluso/metamask-plugin-beta/tree/transaction-plugin I don't know what the line is that is having the issue because it is buried somewhere in the web3 dependency.

rekmarks commented 5 years ago

What's your intent with using web3 in the plugin? If you require an Ethereum convenience library, we recommend ethers.js, as we believe we'll have an easier time getting it running in SES. We haven't tested that out ourselves yet, but if you encounter any problems, we will help you out.

I also recommend that you:

I did some further digging into your issue, which I am recording here for you (if you are interested), and for our future reference:

If you open the bundle file and go to line 30033, you should see something that looks like an HTML comment. You can also search through the bundle file, and you should find strings of the form <!--, which is syntactically valid JavaScript.

However, when I build your plugin on my machine and search through the bundle, I can only find strings of the form <--. Looking at Agoric's RegEx for finding HTML comments in realms-shim, <-- is not going to get picked up. Instead of a SES eval error, I also get a TypeError: undefined is not a constructor when the plugin is evaluated. The lack of a good stack is unfortunately often a problem with SES, but here I'm unsure where the problem is.

jvluso commented 5 years ago

My overall goal is to include the Aragon API which uses web3 extensively to build a signer for Aragon DAOs. I know it has its problems, but I don't think I'll really be able to get around it soon.

Deleting the dist while fixing the git config seems to have solved the problem I was having. Hopefully agoric can work on the error message that was being generated.

rekmarks commented 5 years ago

Got it. We'd be really excited to see an Aragon plugin!

MetaMask has a great relationship with Agoric, and I'll see if we can do anything to improve the SES error stacks. Meanwhile, I'll close this, and feel free to open a new issue if you encounter any other problems.