Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Wallet creation fails on C65 build #13366

Closed
srirambv opened this issue Mar 2, 2018 · 8 comments
Closed

Wallet creation fails on C65 build #13366

srirambv opened this issue Mar 2, 2018 · 8 comments
Assignees
Labels
0.21.x issue first seen in 0.21.x bug feature/rewards priority/P1 Blocks development or testing. Product cannot run. Must be fixed immediately, shipped next release. QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified regression release/blocking release-notes/exclude

Comments

@srirambv
Copy link
Collaborator

srirambv commented Mar 2, 2018

Description

Wallet creation fails on C65 build

Steps to Reproduce

  1. Clean install 0.21.654
  2. Launch from console and enable payments
  3. Gets stuck in wallet creation, reproduced by @bsclifton as well

Actual result:
Wallet creation fails with the following error on console

properties error: Error: Ledger client initialization incomplete.
(node:3896) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 removed listeners added. Use emitter.setMaxListeners() to increase limit
MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 removed listeners added. Use emitter.setMaxListeners() to increase limit
    at _addListener (events.js:262:19)
    at DownloadItem.addListener (events.js:279:10)
    at DownloadItem.<anonymous> (C:\Users\SriramBV\AppData\Local\BraveBeta\app-0.21.654\resources\app.asar\app\filtering.js:602:12)
    at emitTwo (events.js:106:13)
    at DownloadItem.emit (events.js:194:7)
properties error: Error: Ledger client initialization incomplete.
<<< GET https://mercury-proxy.privateinternetaccess.com/v3/publisher/identity?publisher=google.com
<<< content-type: application/json; charset=utf-8
<<< user-agent: Brave/0.21.654 Chrome/65.0.3325.88 Muon/5.1.3 Microsoft Windows x64
<<< accept-encoding:
<<<
[3896:1228:0302/105054.807:ERROR:platform_sensor_reader_win.cc(239)] NOT IMPLEMENTED
[ response for GET https://mercury-proxy.privateinternetaccess.com/v3/publisher/identity?publisher=google.com ]

Expected result:
Wallet should get created when payments is enabled

Reproduces how often:
100% on 0.21.654 build

Brave Version

about:brave info:

Brave 0.21.654
OS Release 10.0.16299
Muon 5.1.3
V8 6.5.254.26
libchromiumcontent 65.0.3325.88
Node.js 7.9.0
rev bc6272c
Update Channel Beta
OS Platform Microsoft Windows
Brave Sync v1.4.2
OS Architecture x64

Reproducible on current live release:
No

Additional Information

@srirambv srirambv added bug feature/rewards regression priority/P1 Blocks development or testing. Product cannot run. Must be fixed immediately, shipped next release. 0.21.x issue first seen in 0.21.x labels Mar 2, 2018
@srirambv srirambv added this to the 0.21.x w/ Chromium 65 (Beta Channel) milestone Mar 2, 2018
@NejcZdovc NejcZdovc self-assigned this Mar 2, 2018
@NejcZdovc
Copy link
Contributor

https://github.com/brave-intl/bat-client/blob/573a72ed13aa6b60a8994d848a0f67919d18113e/index.js#L1358-L1390

this block is problematic worker.onmessage is not triggered, where worker.postMessage is triggered

@NejcZdovc NejcZdovc removed their assignment Mar 2, 2018
@bsclifton
Copy link
Member

@darkdh can you help check this out?

@darkdh darkdh self-assigned this Mar 5, 2018
@hferreiro
Copy link
Contributor

hferreiro commented Mar 7, 2018

Following the worker.postMessage call, it looks like the onmessage callback isn't working on the muon side. In OnMessageInternal() (https://github.com/brave/muon/blob/master/brave/common/workers/worker_bindings.cc#L34), the call onmessage_fun->Call() (https://github.com/brave/muon/blob/master/brave/common/workers/worker_bindings.cc#L54) is skipped because the condition onmessage->IsFunction() is false.

@darkdh
Copy link
Member

darkdh commented Mar 7, 2018

right, and you can assign other property in JS like string and you still can't read the changes
even if you typed somethings that will cause syntax check failure, there will be no exceptions thrown.
and I tried other way to read it

extensions::v8_helpers::GetProperty(context, global,
        v8::String::NewFromUtf8(isolate, "onmessage",
            v8::NewStringType::kNormal).ToLocalChecked(),
        &onmessage);

but it still doesn't work

@darkdh
Copy link
Member

darkdh commented Mar 7, 2018

The root cause is env()->module_system()->Require(module_name_) failure
https://github.com/brave/muon/blob/C65/brave/common/workers/v8_worker_thread.cc#L130

@darkdh
Copy link
Member

darkdh commented Mar 8, 2018

and the exception it throws by https://chromium.googlesource.com/chromium/src.git/+/65.0.3325.146/extensions/renderer/module_system.cc#788 is

ReferenceError: define is not defined
    at Object.commonjs [as require] (extensions::muon/module_system/commonjs:54:3)
    at extensions::bat-client/worker.js:46:43

@darkdh
Copy link
Member

darkdh commented Mar 10, 2018

real root cause is this commit
https://chromium.googlesource.com/chromium/src.git/+/312031eec9c97d5cb9dea801f4f3cefe2f3ad937%5E%21/

they removed

-      // AMD.
-      GetPropertyUnsafe(v8_context, define_object, "define"),

@bsclifton
Copy link
Member

Fixed by upgrading Muon with 39573e4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.21.x issue first seen in 0.21.x bug feature/rewards priority/P1 Blocks development or testing. Product cannot run. Must be fixed immediately, shipped next release. QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified regression release/blocking release-notes/exclude
Projects
None yet
Development

No branches or pull requests

7 participants