wechaty / puppet-padlocal

Puppet PadLocal is a Pad Protocol for WeChat
https://wechaty.js.org/docs/puppet-providers/padlocal
Apache License 2.0
642 stars 88 forks source link

Can not get file for toFileBox() #73

Closed suntong closed 3 years ago

suntong commented 3 years ago

I've got the following error:

(node:80250) UnhandledPromiseRejectionWarning: Error: Can not get file for message: 1297747509315564763
    at PuppetPadlocal.messageFile (/path/to/nodejs/node_modules/wechaty-puppet-padlocal/dist/puppet-padlocal.js:546:23)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async WechatifiedMessage.toFileBox (/.../nodejs/node_modules/wechaty/dist/src/user/message.js:778:25)
    at async msgMediaFile (/.../on-message.js:461:16)
    at async Wechaty.onMessage (/.../on-message.js:303:7)
(Use `node --trace-warnings ...` to show where the warning was created)

on my code (on-message.js:461:16) of:

  const file = await msg.toFileBox()
  let name = saveDir + '/' + file.name

which is used to get the media file from WX.

It used to be working properly with other puppets, but padlocal is failing on images at least:

WechatifiedMessage {
  _events: [Object: null prototype] {},
  _eventsCount: 0,
  _maxListeners: undefined,
  id: '8118076983657142678',
  payload: {
    id: '8118076983657142678',
    timestamp: 1619294531,
    type: 6,
    fromId: 'wxid_dt09..7v22',
    mentionIdList: [],
    roomId: '203..8300@chatroom',
    text: '<?xml version="1.0"?>\n' +
      '<msg>\n' +
      '\t<img aeskey="2bb06930a220ebded73533e5f1f45d67" encryver="0" cdnthumbaeskey="2bb06930a220ebded73533e5f1f45d67" cdnthumburl="3053020100044c304a02010002048b8fde7b02030f5259020426dccdcb02046084783c0425617570696d675f653336373039383165353332386661305f313631393239343236373631330204010d0a020201000400" cdnthumblength="4165" cdnthumbheight="144" cdnthumbwidth="67" cdnmidheight="0" cdnmidwidth="0" cdnhdheight="0" cdnhdwidth="0" cdnmidimgurl="3053020100044c304a02010002048b8fde7b02030f5259020426dccdcb02046084783c0425617570696d675f653336373039383165353332386661305f313631393239343236373631330204010d0a020201000400" length="49142" md5="b98e202a05c82a56b877b90a6cc6d5a4" hevc_mid_size="49142" />\n' +
      '</msg>\n',
    toId: undefined
  },
  [Symbol(kCapture)]: false
}

I'll send you some sample images for you to test...

suntong commented 3 years ago

Found that my two images are actually failing on two different type of images.

The second image failed on the line of

let name = saveDir + '/' + file.name

after getting the file above, and the error is:

(node:103479) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'name' of undefined
    at msgMediaFile (/.../on-message.js:481:35)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Wechaty.onMessage (/.../on-message.js:315:7)
suntong commented 3 years ago

You can test my two images with

https://github.com/wechaty/wechaty-getting-started/blob/master/examples/advanced/media-file-bot.js

(OK that the first one not fixed, but the 2nd image should be easy to fix)

suntong commented 3 years ago

after getting the file above, and the error is:

(node:103479) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'name' of undefined

I.e., the msg.toFileBox() is returning undefined when it is an image.

Just to make it clear what the second case is for toFileBox().

(as I misread the message and thought let name = saveDir + '/' + (file.name || 'default') should have fixed the problem)

suntong commented 3 years ago

Here is another case:

Error: Can not get file for message: 6872153355253203246 caught at msg.toFileBox() in House.
WechatifiedMessage {
  _events: [Object: null prototype] {},
  _eventsCount: 0,
  _maxListeners: undefined,
  id: '6872153355253203246',
  payload: {
    id: '68721...253203246',
    timestamp: 1620145211,
    type: 6,
    fromId: 'ge...gsz',
    mentionIdList: [],
    roomId: '60...884@chatroom',
    text: '<?xml version="1.0"?>\n' +
      '<msg>\n' +
      '\t<img aeskey="2dd56b19aa84a2ae73752c953c1431cd" encryver="0" cdnthumbaeskey="2dd56b19aa84a2ae73752c953c1431cd" cdnthumburl="30580201000451304f0201000204fe09633a02030f5259020425dccdcb02046091743a042a777875706c6f61645f363032333133343838344063686174726f6f6d3833355f313632303134353231300204010d0a020201000400" cdnthumblength="3882" cdnthumbheight="120" cdnthumbwidth="67" cdnmidheight="0" cdnmidwidth="0" cdnhdheight="0" cdnhdwidth="0" cdnmidimgurl="30580201000451304f0201000204fe09633a02030f5259020425dccdcb02046091743a042a777875706c6f61645f363032333133343838344063686174726f6f6d3833355f313632303134353231300204010d0a020201000400" length="1" md5="e2c83d68a055ce661a1f184a8c0f25cb" hevc_mid_size="73562" />\n' +
      '</msg>\n',
    toId: undefined
  },
  [Symbol(kCapture)]: false
}
padlocal commented 3 years ago

For image messages, use await message.toImage() to get the image.

If allowing toFileBox to return the image, but which type of image should it be?

export enum ImageType {
  Unknown = 0,
  Thumbnail,
  HD,
  Artwork,
}

So, we recommend to get image by toImage only.

suntong commented 3 years ago

Hi bro, sorry I have to disagree with you at this issue,

  1. Foremost, I think @huan would agree me that all Wechaty puppet have to comply to a common API standard. The reason I stress so because I found that the message.toImage() is not in the Wechaty common API standard https://wechaty.js.org/docs/api/message.
  2. I tend to agree with Wechaty's standard way of dealing with Media Files. I.e., take a look at https://github.com/wechaty/wechaty-getting-started/blob/5e67a44bf5d4db7ee09aba486f86542c6ef6c182/examples/advanced/media-file-bot.js#L50-L55
  if (msg.type() !== Message.Type.Text) {
    const file = await msg.toFileBox()
    const name = file.name
    console.log('Save file to: ' + name)
    file.toFile(name)
  }

This is how Wechaty's standard way of saving Media Files, be it Audio, Video, Attachment or Image. The code is exactly the same. I've used basically all Wechaty puppets that ever existed, and they all behave the same and the above code always works.

I think padlocal doing it differently than anybody else is not a good practice and should not be encouraged, right @huan ?

padlocal commented 3 years ago

Will get hd image for image message with toFileBox()

suntong commented 3 years ago

That works like a charm my dear brother. Thanks a lot!!!

huan commented 3 years ago

I think padlocal doing it differently than anybody else is not a good practice and should not be encouraged, right @huan ?

I agree with you that we should keep all the behavior be the same as possible as we can for the whole Wechaty ecosystem so that our developers can get consistent experiences crossing the puppet provider/services.

For this case, I agree that the toFileBox() should be supported for the image type message and return the HD size image. If users want to specify the different sizes for the image, then they should use toImage for maximum flexibility.

suntong commented 3 years ago

Thanks for your support @huan.

Our brother had already fixed that according to Wechaty ecosystem's common behavior. Thanks @padlocal.