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.

Sign up for GitHub

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

Jump to bottom

ethereum.enable() throws error in new builds #5386

Closed
ghost opened this issue Sep 27, 2018 · 4 comments
Closed

ethereum.enable() throws error in new builds #5386

ghost opened this issue Sep 27, 2018 · 4 comments
Assignees
@bitpshr
Labels
needs-reproduction type-bug

Comments

@ghost
Copy link

ghost commented Sep 27, 2018

Describe the bug
After downloading builds 5b5d390 and d496e2d from PR #4703 and testing them against a sample dApp implementation using window.ethereum.enable(), I was unable to successfully access user accounts after approving access, due to an error in the enable call. Denying access worked as expected.

A minimal example of my code looks like:

if (window.ethereum) {
  try {
    await ethereum.enable()
    console.log(window.ethereum)
  } catch (error) {
    console.error(error)
  }
}

The full stack trace of the error is attached.

To Reproduce

  1. Download builds 5b5d390 and d496e2d
  2. Write a sample implementation that looks something like the above code.
  3. Deny access and observe the correctly logged error.
  4. Approve access and observe an uncaught error.

Screenshots
image

Expected behavior
For window.ethereum.enable() to function correctly and grant my dApp access to user accounts.

Browser details (please complete the following information):
Chrome: Version 69.0.3497.100 (Official Build) (64-bit)
OS: macOS High Sierra 10.13.6 (17G65)
MetaMask: 4.9.3 (5b5d390 and d496e2d)
UI: New/Beta

Additional context
I also tested out the Firefox builds, with the same result. I thought that the problem might stem from me using http and localhost, so I briefly spun up an https localhost, but still had the same result.
@ghost ghost mentioned this issue Sep 27, 2018
Merged
7 tasks
@bitpshr
Copy link
Contributor

bitpshr commented Sep 27, 2018 •
edited
Loading

Hi @NoahHydro, thanks for your detailed bug report. After trying the steps listed, we're unable to reproduce the error you described. For reference, here's exactly what we did:

  1. Installed d496e2d for Chrome.
  2. Created a simple HTML page and served it locally: 
        <html>
        <head>
            <script>
                window.addEventListener('load', async () => {
                    if (window.ethereum) {
                        try {
                            await ethereum.enable()
                            console.log(window.ethereum)
                        } catch (error) {
                            console.error(error)
                        }
                    }
                })
            </script>
        </head>
    </html>
  3. Visited the HTML page in the browser:
    preview

Despite not being able to reproduce the issue, the message associated with the error you described is indicative of a failed provider.send({ method: 'eth_accounts' }) call here. To be safe, we updated the logic around this operation.

Could you please try the latest build from 3d47dbd and let us know if you still experience this issue? If you do, would it be possible to provide the relevant initialization code from your dapp? Thanks in advance.

@NoahZinsmeister
Copy link
Contributor

NoahZinsmeister commented Sep 27, 2018 •
edited
Loading

Hi @bitpshr, thanks for the super fast turnaround! I can confirm that build 3d47dbd fixes my issue locally! (@NoahHydro is my other account, sorry for the double identity.)

I'm not sure why the error is not reproducible via your script, it must be something unique to my project. One thought was that I mistakenly tried to initialize a const web3js = new Web3(window.ethereum) instance against web3@1.0.0-beta.35, but I just tested it locally without that line, which didn't fix the issue I was having with the older builds.

Let me know if you'd like me to do some more digging to try to figure out what part of my local project was causing the failure. Possibly something related to me using the new UI?

Regardless, thanks for pushing this fix!

@bitpshr
Copy link
Contributor

bitpshr commented Sep 27, 2018

Glad to hear things are resolved @NoahZinsmeister. Please let us know if you encounter any other issues while testing against the 1102 PR.

@bitpshr bitpshr closed this as completed Sep 27, 2018
@NoahZinsmeister
Copy link
Contributor

Will do, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bitpshr bitpshr

Labels
needs-reproduction type-bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bitpshr @bdresser @NoahZinsmeister