Skip to content
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 request error when image sending too fast #185

Merged
merged 11 commits into from
Dec 14, 2021

Conversation

System233
Copy link
Contributor

Finally I figured out where wx2.qq.comundefined come from:

/**
*
* Help Functions which Proxy to WXAPP AngularJS Scope & Factory
* getMsgImg(message.MsgId,'slave')
* getMsgImg(message.MsgId,'big',message)
*/
function getMsgImg (id, type, message) {
const contentChatScope = WechatyBro.glue.contentChatScope
if (!contentChatScope) {
throw new Error('getMsgImg() contentChatScope not found')
}
const path = contentChatScope.getMsgImg(id, type, message)
return window.location.origin + path
// https://wx.qq.com/?&lang=en_US/cgi-bin/mmwebwx-bin/webwxgetmsgimg?&MsgID=4520385745174034093&skey=%40crypt_f9cec94b_a3aa5c868466d81bc518293eb292926e
// https://wx.qq.com/cgi-bin/mmwebwx-bin/webwxgetmsgimg?&MsgID=8454987316459381112&skey=%40crypt_f9cec94b_bd210b2224f217afeab8d462af70cf53
}

https://github.com/wechaty/webwx-app-tracker/blob/a12c78fb8bd7186c0f3bb0e18dd611151e6b8aac/formatted/webwxApp.js#L2100-L2106
some getMsgImg call returned undefined and FileBox requested the url.

If I send images one by one and save them, everything is fine, if I sending images too fast, an error will be occurred and crash the process when exception be passed to wechaty.

Error: getaddrinfo ENOTFOUND wx2.qq.comundefined
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:71:26)

It should be fixed in #183, but I made a mistake, I thought $watch will works like Vue watch API, but it's not, the watch handler will be called immediately in first time even though MMStatus doesn't change.

#183 should always fail in my manual tests, but it does work, and it should not happen in #184 tests(I think the error cause by the default value of MMStatus change to 'SENDING' in #183), which is weird.

Other Enhancements

  1. I want to increase the priority of file.mediaType to ensure that user behavior is not overwritten
    const contentType = mime.getType(ext) || file.mediaType || undefined
const contentType = file.mediaType || mime.getType(ext)  || undefined
  1. Can we add an error handler to catch exceptions from user listeners to ensure that process dose not crash?
import {WechatyBuilder} from "wechaty"
import {generate} from "qrcode-terminal"
const bot=WechatyBuilder.build({puppet:'wechaty-puppet-wechat'});
bot.on('scan',code=>generate(code,{small:true}));
bot.on('message',()=>{
    throw new Error('crash')
})
bot.start();

src/puppet-wechat.ts Outdated Show resolved Hide resolved
src/puppet-wechat.ts Outdated Show resolved Hide resolved
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.

Please keep the tests/puppeteer-attachment.spec.ts file permission to be 100755 so that it can be executed directly under Linux.

@huan
Copy link
Member

huan commented Dec 14, 2021

Can we add an error handler to catch exceptions from user listeners to ensure that the process does not crash?

If an error on wechaty instance has not been caught then the process will crash.

In order to catch it, we can use:

wechaty.on('error', e => {
  console.error(e)
})

to catch it.

Please let me know if you have more questions, thank you very much.

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.

LGTM

Please feel free to help yourself to merge this PR when you believe everything is OK with it, thank you very much!

@System233
Copy link
Contributor Author

LGTM

Please feel free to help yourself to merge this PR when you believe everything is OK with it, thank you very much!

Thanks, I will.

@System233 System233 merged commit da20828 into wechaty:main Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants