-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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(*): Support for send 25Mb files #771
Conversation
Thank you @binsee! This PR looks good to me. Please get more approvements from other contributors and I'll be able to merge this PR after we got three approvements. Cheers! |
src/puppet-web/puppet-web.ts
Outdated
@@ -395,43 395,111 @@ export class PuppetWeb extends Puppet { | |||
// Sending video files is not allowed to exceed 20MB | |||
// https://github.com/Chatie/webwx-app-tracker/blob/7c59d35c6ea0cff38426a4c5c912a086c4c512b2/formatted/webwxApp.js#L1115 | |||
const videoMaxSize = 20 * 1024 * 1024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxVideoSize
, largeFileSize
, maxFileSize
would be better variable names
const id = 'WU_FILE_' this.fileId | ||
this.fileId | ||
|
||
const hostname = await this.browser.hostname() | ||
const headers = { | ||
Referer: `https://${hostname}`, | ||
'User-Agent': 'Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious.. Why does the default UA fail here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I did not test other UA.
Awesome, thanks all for the code review which will help the Wechaty better! @binsee Could you please resolve the conflicts in your PR? I'll be able to merge it after that. |
I will deal with this problem as soon as possible. |
I made a mistake... |
Ok. But it's always a good behavior to update one PR till be merged, because close and open a new PR will lose all the comments/reviews. So I'd like to suggest you reopen this PR and keep updating this one. :) |
After you sync with the master branch, I suggest you do a Then I believe this PR will be ok again. Could you have a try with |
1. Upload a file larger than 25Mb. 2. Limit the upload file to no more than 100Mb. 3. According to webwxapp perfect upload processing. 4. Increase the upload and handling of upload exceptions.
1. add MediaMessage.saveFile() , used to save attachment 2. Refactor MediaMessage.forward(), support send to Room and Contact array, add support download big file and send it again.
Because webWx restrictions, more than 25 MB of files can not be downloaded, it can not be forwarded.
Closes 777
@zixia Success, thanks for the reminder. |
@@ -461,7 539,7 @@ export class PuppetWeb extends Puppet { | |||
mediatype, | |||
uploadmediarequest: JSON.stringify(uploadMediaRequest), | |||
webwx_data_ticket: webwxDataTicket, | |||
pass_ticket: passTicket, | |||
pass_ticket: passTicket || '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After testing, this parameter is not required when uploading a file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix #777
* It should be noted that when forwarding ATTACH type message, if the file size is greater than 25Mb, the forwarding will fail. | ||
* The reason is that the server shields the web wx to download more than 25Mb files with a file size of 0. | ||
* | ||
* But if the file is uploaded by you using wechaty, you can forward it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is a new feature that supports forwarding your own uploaded 25Mb files.
* | ||
* ```javasrcipt | ||
* .on('message', async m => { | ||
* if (m.self() && m.rawObj && m.rawObj.Signature) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the code to forward the 25mb file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the awesome fix!
Please let me know when you have been ready for merging.
src/message.ts
Outdated
ret = <boolean>await new Promise((resolve, reject) => { | ||
readStream.pipe(writeStream) | ||
readStream | ||
.on('end', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use once('end')
at here should be better?
src/message.ts
Outdated
readStream | ||
.on('end', () => { | ||
log.silly('MediaMessage', `saveFile() pipe end`) | ||
resolve(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to suggest move resolve()
to the following code:
writeStream.once('close', resolve)
and also add this to monitor failure event:
writeStream.once('error', reject)
src/message.ts
Outdated
* | ||
* @param filePath save file | ||
*/ | ||
public async saveFile(filePath: string): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the return value boolean
:
I'd like to suggest to use void
as the return value. If so, we might want to throw
an Error if the operation failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been modified.
@zixia |
Fantastic! Thank you for the great work! |
Issues: #766