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

Fix switch account #811

Merged
merged 7 commits into from Sep 22, 2017

Conversation

Projects
None yet
4 participants
@binsee
Member

binsee commented Sep 16, 2017

  1. Support for find elements on pages of different languages
  2. fix uncaughtException in onServerLogin()

uncaughtException log:

17:33:29 VERB StateSwitch Wechaty:current(ready,true) <- (ready,false)
ReferenceError: arguments is not defined
    at Timeout.setTimeout (E:\CodeWork\fork\wechaty\src\puppet-web\event.ts:249:35)
    at ontimeout (timers.js:488:11)
    at tryOnTimeout (timers.js:323:5)
    at Timer.listOnTimeout (timers.js:283:5)
17:33:30 ERR Bot uncaughtException: ReferenceError: arguments is not defined
Wechaty exit 99 because of uncaughtException

fix #772

Show outdated Hide outdated src/puppet-web/event.ts Outdated
@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Sep 16, 2017

Member

It seems we should use arguments inside the arrow function carefully.

See: https://stackoverflow.com/questions/33298315/why-arguments-is-not-defined-in-arrow-function-in-es6

Member

zixia commented Sep 16, 2017

It seems we should use arguments inside the arrow function carefully.

See: https://stackoverflow.com/questions/33298315/why-arguments-is-not-defined-in-arrow-function-in-es6

@zixia zixia requested review from hczhcz, lijiarui and ax4 Sep 16, 2017

@@ -244,6 +244,10 @@ async function onServerLogin(this: PuppetWeb, data, attempt = 0): Promise<void>
// if `login` event fired before this.bridge inited, we delay the event for 1 second.

This comment has been minimized.

@zixia

zixia Sep 16, 2017

Member

const args = Array.prototype.slice.call(arguments)

@zixia

zixia Sep 16, 2017

Member

const args = Array.prototype.slice.call(arguments)

This comment has been minimized.

@binsee

binsee Sep 16, 2017

Member

OK, has used args to replace arguments.

@binsee

binsee Sep 16, 2017

Member

OK, has used args to replace arguments.

Show outdated Hide outdated src/puppet-web/event.ts Outdated
Show outdated Hide outdated src/puppet-web/event.ts Outdated
@zixia

zixia approved these changes Sep 16, 2017

The code looks good to me, however I have no environment to test it.

Please get at least 3 approvements from the contributors then I'll be able to merge it.

Thank you very much!

@hczhcz

hczhcz approved these changes Sep 16, 2017

Good job!

@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Sep 17, 2017

Member

Thanks! I tried this code, but always get the following result:

12:29:54 VERB PuppetWeb initBridge() done
12:29:54 VERB PuppetWebBrowser clickSwitchAccount()
12:29:54 SILL PuppetWebBrowser clickSwitchAccount() button not found: no such element: Unable to locate element: {"method":"xpath","selector":"//div[contains(@class,'association') and contains(@class,'show')]/a[@ng-click='qrcodeLogin()']"}
  (Session info: chrome=60.0.3112.113)
  (Driver info: chromedriver=2.24.417412 (ac882d3ce7c0d99292439bf3405780058fcca0a6),platform=Mac OS X 10.11.6 x86_64)
12:29:54 VERB StateSwitch Puppet:current(live,true) <- (live,false)

Cannot get auto click silly log.

Just here fyi

Member

lijiarui commented Sep 17, 2017

Thanks! I tried this code, but always get the following result:

12:29:54 VERB PuppetWeb initBridge() done
12:29:54 VERB PuppetWebBrowser clickSwitchAccount()
12:29:54 SILL PuppetWebBrowser clickSwitchAccount() button not found: no such element: Unable to locate element: {"method":"xpath","selector":"//div[contains(@class,'association') and contains(@class,'show')]/a[@ng-click='qrcodeLogin()']"}
  (Session info: chrome=60.0.3112.113)
  (Driver info: chromedriver=2.24.417412 (ac882d3ce7c0d99292439bf3405780058fcca0a6),platform=Mac OS X 10.11.6 x86_64)
12:29:54 VERB StateSwitch Puppet:current(live,true) <- (live,false)

Cannot get auto click silly log.

Just here fyi

@binsee

This comment has been minimized.

Show comment
Hide comment
@binsee

binsee Sep 17, 2017

Member

@lijiarui
I tried this code to work properly.
When using the cookie file to start again, if the interval is very short, do not need to re-scan landing.
If you modify the key value in the cookie file to make it invalid, you will find that when you start again, the switch account button will be automatically clicked.
Because the normal landing did not switch account button, it will lead to abnormal, but it was captured, will not affect the operation.

Member

binsee commented Sep 17, 2017

@lijiarui
I tried this code to work properly.
When using the cookie file to start again, if the interval is very short, do not need to re-scan landing.
If you modify the key value in the cookie file to make it invalid, you will find that when you start again, the switch account button will be automatically clicked.
Because the normal landing did not switch account button, it will lead to abnormal, but it was captured, will not affect the operation.

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Sep 17, 2017

Member

In order to make the log clearer, I suggest we change the log code like this:

- log.silly('PuppetWebBrowser', 'clickSwitchAccount() button not found: %s', e && e.message || e) 
+ log.silly('PuppetWebBrowser', 'clickSwitchAccount() no button found')

This change might be able to make the log less confusing.

Member

zixia commented Sep 17, 2017

In order to make the log clearer, I suggest we change the log code like this:

- log.silly('PuppetWebBrowser', 'clickSwitchAccount() button not found: %s', e && e.message || e) 
+ log.silly('PuppetWebBrowser', 'clickSwitchAccount() no button found')

This change might be able to make the log less confusing.

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Sep 21, 2017

Member

@lijiarui could you please review this pr again and approve it if it lgty?

Member

zixia commented Sep 21, 2017

@lijiarui could you please review this pr again and approve it if it lgty?

@lijiarui

Although we didn't find the way to reproduce, but the code looks good to me.

Thanks! @binsee

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Sep 22, 2017

Member

Thanks all for pr/review the code!

Member

zixia commented Sep 22, 2017

Thanks all for pr/review the code!

@zixia zixia merged commit 459dbea into Chatie:master Sep 22, 2017

5 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
codacy/pr Good work! A positive pull request.
Details
codeclimate 1 fixed issue
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
security/snyk No new issues
Details

@binsee binsee deleted the binsee:fix-switch-account branch Sep 22, 2017

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