Skip to content
This repository has been archived by the owner on Jun 1, 2020. It is now read-only.

Add implementation for room-invite event #107

Merged
merged 7 commits into from Jul 24, 2018
Merged

Add implementation for room-invite event #107

merged 7 commits into from Jul 24, 2018

Conversation

windmemory
Copy link
Member

Currently this is a prototype, not finished version.

@windmemory windmemory requested a review from huan July 20, 2018 05:29
@windmemory
Copy link
Member Author

I was using axios to do a post with the url returned back from WXGetRequestToken call. But it triggered websocket close event, then throw out an exception. The exception as below:

TypeError: Cannot read property 'request' of undefined
    at RedirectableRequest._performRequest (/Users/yuangao/git/wechaty/wechaty-puppet-padchat/node_modules/follow-redirects/index.js:132:24)
    at RedirectableRequest._processResponse (/Users/yuangao/git/wechaty/wechaty-puppet-padchat/node_modules/follow-redirects/index.js:222:10)
    at ClientRequest.RedirectableRequest._onNativeResponse (/Users/yuangao/git/wechaty/wechaty-puppet-padchat/node_modules/follow-redirects/index.js:39:10)
    at Object.onceWrapper (events.js:273:13)
    at ClientRequest.emit (events.js:182:13)
    at ClientRequest.EventEmitter.emit (domain.js:442:20)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:546:21)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:109:17)
    at TLSSocket.socketOnData (_http_client.js:432:20)
    at TLSSocket.emit (events.js:182:13)
    at TLSSocket.EventEmitter.emit (domain.js:442:20)
    at addChunk (_stream_readable.js:283:12)
    at readableAddChunk (_stream_readable.js:264:11)
    at TLSSocket.Readable.push (_stream_readable.js:219:10)
    at TLSWrap.onread (net.js:635:20)

Seems like error with axios, but I can not find anyone talked about this in axios repo. Then I decide to try with request-promise, but after I installed request-promise, all the npm link I created before seems got reset, not sure why, but setup npm link is time consuming, so I decide to have the change in wechaty and wechaty-puppet get published, then work on the impl.

Anyway, here is the workable prototype you asked, please let me know what do you think.

@huan
Copy link
Member

huan commented Jul 20, 2018

Could you please deploy your workable prototype on a c9 IDE, so that we can run it? Because this PR can not workable without a runnable environment.

If you have any trouble with your c9 account, we have a public c9 ide at https://ide.c9.io/zixia/wechaty-bug-reproducer and you can apply a write permission for that. But please be careful that it's public so everyone can read it.

@windmemory
Copy link
Member Author

c9 IDE is closed for new signup, I am using @lijiarui 's account, and will have . I think it would be better to have a new tool in the future to replace the c9 since not everyone has c9 account.

Here is the c9 workspace: https://ide.c9.io/lijiarui/room-invite-test

@huan
Copy link
Member

huan commented Jul 20, 2018

Yes, I agree with you.

Amazon sucks.

@huan
Copy link
Member

huan commented Jul 20, 2018

Is there any documentation, or at least a note of how to run your code?

@windmemory
Copy link
Member Author

@zixia Please let me know what do you think about this approach :)

@windmemory
Copy link
Member Author

@zixia Do a ts-node bot.ts in the root folder.

Copy link
Member

@huan huan left a comment

Choose a reason for hiding this comment

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

Need to support English language as well

@windmemory
Copy link
Member Author

windmemory commented Jul 20, 2018

English version will also return back in Chinese, I've tested that.

Well, I think I should revoke this message. I will add English language as well.

Is there any other comments besides the language?

@windmemory
Copy link
Member Author

@zixia ping

@huan
Copy link
Member

huan commented Jul 21, 2018

Will look into it for you this evening.

@windmemory
Copy link
Member Author

Hi @zixia did you look into it?

@huan
Copy link
Member

huan commented Jul 22, 2018

Please follow reply at wechaty/wechaty#1492 (comment)

Copy link
Member

@huan huan left a comment

Choose a reason for hiding this comment

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

Your change is a BREAKING CHANGE and we have to bumper the version of wechaty-puppet-padchat from v0.8 to v0.9 to prevent breaking the old users.

package.json Outdated
@@ -69,7 +69,7 @@
"tslint": "^5.10.0",
"tslint-config-standard": "^7.1.0",
"typescript": "^2.9.2",
"wechaty": "^0.17.133"
"wechaty": "^0.19.107"
Copy link
Member

Choose a reason for hiding this comment

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

I have to notify you that this is a BREAKING change and you should bump your MINOR version number when you changing the dependence to wechaty 0.17 to 0.19

@windmemory
Copy link
Member Author

Bumped

@windmemory windmemory merged commit ce7cfd1 into master Jul 24, 2018
@windmemory windmemory deleted the dev branch July 24, 2018 07:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants