I hit the same issue, and took a day to investigate. This should be a regression due to the puppet refactoring.
There are two issues need to be resolved:
a crash on 'wechaty-bro.js' due to the factory instance not passed in as this. it can be fixed by this code
oldRecalledMsgProcess.call(chatFactory, msg) I can help to submit a fix.
A design change needs to be discussed before it can be fixed. When a recalled message is hooked, the 'message' event is trigger only with the message id as here, the messagePayload is called to load the message payload from cache, which causes the original 'RECALLED' type (10002) is lost. There are two solutions but they may all slightly effect the whole 'puppet' design, and I am not sure how it effects other types of puppet implementations,
a. trigger the 'message' event with additional parameters/metadata, so the message type can be changed by this information after the message is loaded.
b. trigger a new 'recall' event with the cached message.
@huan I would like to contribute the fix after we discuss the design. suggestions?
@seanxlliu Thank you very much for taking the time to investigate this issue!
I totally agree with you with your analysis, and you did a great PR to fix this.
About the topic 2, after I thinking about your plan A and plan B, I have a new idea that the RECALLED action needs a pseudo message to help us to understand what was happening in the message event.
Which means I have a plan C as the following:
C. when a message was recalled, we:
get the message id from the message that had been recalled. let's say, the id is id_real_message_be_recalled
generate a message id for a new RECALL message, let's say, the id is id_pseudo_recall_message
create a message payload for id_presudo_recall_message, set the MessageType to RECALL, and save the recalled message id id_real_message_be_recalled to this payload(for a dirty example, we save the id in the text property)
emit(id_pseudo_recall_message)
With this design, I believe there's no need to modify any puppet design, and we can easily deal with the RECALLED message in the on('message') event.
Please free free to let me know what you think, and you are welcome to implement it with a new PR!
Moved from https://github.com/Chatie/wechaty/issues/1065 author @WiseClock
Provide Your Network Information
Run
npm run doctor
orwechaty run doctor
(for docker user), paste output hereExpected behavior
Recalling messages trigger the .on('message') method with a message of type RECALLED and the original message wrapped in.
It should be fixed months ago in: https://github.com/Chatie/wechaty/commit/174b6775c4e9242e5f003094b2f9e10953c978f2 Am I missing something in the setup or is the API changed?
Actual behavior
Recalling a message does not trigger the .on('message') method and shows nothing in the console output.
Steps to reproduce the behavior (and fixes, if any)
Full Output Logs
Show Logs
### Paste the full output logs here with `WECHATY_LOG=silly` set ```shell D:\CODE\node\rin_wechaty>set WECHATY_LOG=silly D:\CODE\node\rin_wechaty>node index.js WECHATY_LOG=silly 22:50:24 SILL Brolog WECHATY_LOG set level to silly 22:50:24 INFO Config registering process.on("unhandledRejection") for development/debug 22:50:24 VERB Config constructor() 22:50:25 SILL StateSwitch constructor(Wechaty) 22:50:25 VERB Wechaty contructor() 22:50:25 VERB Profile constructor(rin) 22:50:25 VERB Wechaty on(scan, function) registered 22:50:25 VERB Wechaty onFunction(scan) 22:50:25 VERB Wechaty on(login, function) registered 22:50:25 VERB Wechaty onFunction(login) 22:50:25 VERB Wechaty on(message, function) registered 22:50:25 VERB Wechaty onFunction(message) 22:50:25 SILL Wechaty version() form development environment is not availble: ENOENT: no such file o r directory, stat 'D:\CODE\node\rin_wechaty\node_modules\wechaty\dist\.git' 22:50:25 INFO Wechaty v0.13.42 starting... 22:50:25 VERB Wechaty puppet: web 22:50:25 VERB Wechaty profile: rin 22:50:25 VERB Wechaty uuid: 2adc06ea-47a2-4bee-8769-88953859c350 22:50:25 SILL StateSwitchI hit the same issue, and took a day to investigate. This should be a regression due to the puppet refactoring.
There are two issues need to be resolved:
a crash on 'wechaty-bro.js' due to the factory instance not passed in as this. it can be fixed by this code
oldRecalledMsgProcess.call(chatFactory, msg)
I can help to submit a fix.A design change needs to be discussed before it can be fixed. When a recalled message is hooked, the 'message' event is trigger only with the message id as here, the messagePayload is called to load the message payload from cache, which causes the original 'RECALLED' type (10002) is lost. There are two solutions but they may all slightly effect the whole 'puppet' design, and I am not sure how it effects other types of puppet implementations,
a. trigger the 'message' event with additional parameters/metadata, so the message type can be changed by this information after the message is loaded. b. trigger a new 'recall' event with the cached message.
@huan I would like to contribute the fix after we discuss the design. suggestions?
Submitted a PR for the first issue.
@seanxlliu Thank you very much for taking the time to investigate this issue!
I totally agree with you with your analysis, and you did a great PR to fix this.
About the topic 2, after I thinking about your plan A and plan B, I have a new idea that the
RECALLED
action needs a pseudo message to help us to understand what was happening in themessage
event.Which means I have a plan C as the following:
C. when a message was recalled, we:
id_real_message_be_recalled
id_pseudo_recall_message
id_presudo_recall_message
, set the MessageType toRECALL
, and save the recalled message idid_real_message_be_recalled
to this payload(for a dirty example, we save the id in thetext
property)id_pseudo_recall_message
)With this design, I believe there's no need to modify any puppet design, and we can easily deal with the RECALLED message in the on('message') event.
Please free free to let me know what you think, and you are welcome to implement it with a new PR!
Agree. Will work on that.