Wechaty: Discussion of switching GetContactPayload to SearchContactPayload

[windmemory]

contactRawPayload in padchat-manager.ts is using WXGetContactPayload method to get the contact. Sometimes, the contactId passed to contactRawPayload might not be the friend of the user.

Per my investigation, when message.ts call ready method, it will also call ready method on the from and to Contact. If this message is from a room, the from might not be the friend of the bot, then ready in Contact will try to load the data for the contact, but since the contact is not a friend, with GetContactPayload, there will be no results for it. After that, this payload will be passed to contactRawPayloadParser, and the parser can not read out the user_name, thus cause breakage of the bot.

Do you think we should change the method in contactRawPayload method?

[windmemory]

Related to #1358

[huan]

Yes, Message needs the sender contact to be ready-ed, which means that Contact must be able to load the ContactPayload for itself.

Because in PuppetPadchat it will fail when getting a contact payload for a stranger in the room, so your discussion is very valuable for us to deal with the Pachat protocol well.

My suggestion would be:

1. Yes, you can try SearchContact, and to see if it could solve this problem. But to my understanding of this API(of the name _search_), I don't think it would be the solution for us;
2. Consider to load the sender payload from roomMemberPayload, which I feels should be the best data source for the room members.

https://github.com/Chatie/wechaty/blob/1de61f430f381905fc486750be6ac23c429b4eed/src/puppet-padchat/padchat-manager.ts#L750-L758

[lijiarui]

@windmemory I change my code as follows here in padchat-rpc.ts, when get undefined user_name then change to WXSearchContact()

  /**
   * Get contact by contact id
   * @param {any} id        user_name
   */
  public async WXGetContactPayload(id: string): Promise<PadchatContactPayload> {
    if (!isContactId(id)) { // /@chatroom$/.test(id)) {
      throw Error(`should use WXGetRoomPayload because get a room id :${id}`)
    }
    let result = await this.WXGetContact(id) as PadchatContactPayload

    if (!result.user_name) {
      result = await this.WXSearchContact(id) as PadchatContactPayload
    }

    return result
  }

Well, the API is blocked... Related log as follows:

01:45:53 INFO PadchatRpc WXSearchContact wxid: wxid_pmct091suwyt12 cannot be searched
01:45:53 SILL PuppetPadchat contactRawPayloadParser({user_name=""})
01:45:53 ERR Contact ready() this.puppet.contactPayload(Contact) exception: cannot get user_name from raw payload: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[找不到相关帐号或内容]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 SILL PadchatRpc WXSearchContact result: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[找不到相关帐号或内容]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 INFO PadchatRpc WXSearchContact wxid: wxid_o9yzflim2f0n12 cannot be searched
01:45:53 SILL PuppetPadchat contactRawPayloadParser({user_name=""})
01:45:53 ERR Contact ready() this.puppet.contactPayload(Contact) exception: cannot get user_name from raw payload: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[找不到相关帐号或内容]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 SILL PadchatRpc WXSearchContact result: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[操作过于频繁，请稍后再试]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 INFO PadchatRpc WXSearchContact wxid: yyf2010555 cannot be searched
01:45:53 SILL PuppetPadchat contactRawPayloadParser({user_name=""})
01:45:53 ERR Contact ready() this.puppet.contactPayload(Contact) exception: cannot get user_name from raw payload: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[操作过于频繁，请稍后再试]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 SILL PadchatRpc WXSearchContact result: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[操作过于频繁，请稍后再试]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 INFO PadchatRpc WXSearchContact wxid: wxid_hjjqix6dojdr12 cannot be searched
01:45:53 SILL PuppetPadchat contactRawPayloadParser({user_name=""})
01:45:53 ERR Contact ready() this.puppet.contactPayload(Contact) exception: cannot get user_name from raw payload: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[操作过于频繁，请稍后再试]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 SILL PadchatRpc WXSearchContact result: {"big_head":"","city":"","country":"","message":"\n�\u0002<e>\n<ShowType>1</ShowType>\n<Content><![CDATA[操作过于频繁，请稍后再试]]></Content>\n<Url><![CDATA[]]></Url>\n<DispSec>30</DispSec>\n<Title><![CDATA[]]></Title>\n<Action>2</Action>\n<DelayConnSec>0</DelayConnSec>\n<Countdown>0</Countdown>\n<Ok><![CDATA[]]></Ok>\n<Cancel><![CDATA[]]></Cancel>\n</e>\n","nick_name":"","provincia":"","py_initial":"","quan_pin":"","sex":0,"signature":"","small_head":"","status":-24,"stranger":"","user_name":""}
01:45:53 INFO PadchatRpc WXSearchContact wxid: wxid_v6hwv4tc5na921 cannot be searched
01:45:53 SILL PuppetPadchat contactRawPayloadParser({user_name=""})

Maybe this is because WXSearchContact() is related to add a friend, this should not be used to frequently. But WXGetContact() can be called frequently more than WXSearchContact().

So maybe we shouldn't change WXGetContact to WXSearchContact...

[windmemory]

Thanks @lijiarui for your investigation. You make it easy to answer @zixia 's comment. We need to load the data from roomMember instead of contact.

Then I got another question. I think @zixia could answer this well.

I think it's better to keep align with the original design so we don't break anyone.

What's the original design for room member? Are these information also supposed to be stored in Contact? If so, we need to fill the member data into Contact when we sync the room member, otherwise, we should change the find logic here for the room member.

[huan]

In Wechaty, the original design for room member is to get the room related information for the contact, for example, the room alias.

So that information is all stored in the Contact, exception the room alias.

Please see the related commits and I had made today, and feel free to let me know if you have any suggestions/questions, and any further improvements are welcome.

[windmemory]

After reading code carefully, I find that there are two places that writes the contact information to cache.
One is here in SyncContactsAndRooms
https://github.com/Chatie/wechaty/blob/d89f77f4bb98c83f4b91c8c419d4a154014a3f41/src/puppet-padchat/padchat-manager.ts#L883-L895
The other one is in contactRawPayload
https://github.com/Chatie/wechaty/blob/d89f77f4bb98c83f4b91c8c419d4a154014a3f41/src/puppet-padchat/padchat-manager.ts#L947-L954

Code in SyncContactsAndRooms seems to be safe and will always write valid data into cache, but the code in contactRawPayload is not safe. There was a check before we writes data into cache, but got commented out after. I think we should always check the payload before we set the payload into the cache. At least check the payload is not empty.

Besides, seems like the contactRawPayload is the deepest datasource in padchat for contact, so I wonder whether it makes more sense to move the logic of checking roomMemberList from puppet-padchat to this method?

If this makes sense to you @zixia , I will submit a PR later. Let me know what do you think.

[huan]

Thank you for reading code carefully.

I believe we should create a new issue because the discussion right now is not related to the current title of SearchContactPayload.

And also please notice that I had moved all the code base related to the wechaty-puppet-padchat to it's new home at https://github.com/lijiarui/wechaty-puppet-padchat, all the future development and discussion should be moved over there.

About the PR: Please feel free to submit PR at https://github.com/lijiarui/wechaty-puppet-padchat instead here, because all the code related to the padchat had been frozen in this repository to prevent source code inconsistent. Make sure you had tested it locally, and submit with unit tests will be recommend.

[windmemory]

Cool, I will close this issue.