From f8cbf8b91953fc76f392ae530b42252b5664e5c4 Mon Sep 17 00:00:00 2001 From: Huan LI Date: Sun, 27 May 2018 11:10:41 +0800 Subject: [PATCH] NPM Switch: `promise-retry` to replace `retry-promise` (#1235) --- package.json | 3 +- src/friend-request.ts | 30 +++++++++--- src/misc.spec.ts | 58 +++++++++++++++++++++- src/misc.ts | 25 ++++++++++ src/puppet-puppeteer/bridge.spec.ts | 76 ++++++++++++++--------------- src/puppet-puppeteer/bridge.ts | 40 +++++++++------ src/puppet-puppeteer/firer.ts | 59 +++++++++++----------- 7 files changed, 197 insertions(+), 94 deletions(-) diff --git a/package.json b/package.json index 623dfc9a..f488dc76 100644 --- a/package.json +++ b/package.json @@ -101,11 +101,11 @@ "hot-import": "^0.2.1", "mime": "^2.2.0", "normalize-package-data": "^2.4.0", + "promise-retry": "^1.1.1", "puppeteer": "^1.2.0", "raven": "^2.6.2", "read-pkg-up": "^3.0.0", "request": "^2.87.0", - "retry-promise": "^1.0.0", "rx-queue": "^0.4.7", "rxjs": "^6.1.0", "state-switch": "^0.4.5", @@ -125,6 +125,7 @@ "@types/mime": "^2.0.0", "@types/node": "^10.1.2", "@types/normalize-package-data": "^2.4.0", + "@types/promise-retry": "^1.1.1", "@types/puppeteer": "^1.0.0", "@types/raven": "^2.1.0", "@types/read-pkg-up": "^3.0.0", diff --git a/src/friend-request.ts b/src/friend-request.ts index 39d92cfd..b8630544 100644 --- a/src/friend-request.ts +++ b/src/friend-request.ts @@ -19,8 +19,7 @@ */ /* tslint:disable:no-var-requires */ -const retryPromise = require('retry-promise').default - +// const retryPromise = require('retry-promise').default import { instanceToClass } from 'clone-class' import { PuppetAccessory } from './puppet-accessory' @@ -30,6 +29,7 @@ import { Contact } from './contact' import { log, } from './config' +import { Misc } from './misc' export enum FriendRequestType { Unknown = 0, @@ -166,17 +166,31 @@ export class FriendRequest extends PuppetAccessory { await this.puppet.friendRequestAccept(this.payload.contactId, this.payload.ticket) - const max = 20 - const backoff = 300 - const timeout = max * (backoff * max) / 2 + // const max = 20 + // const backoff = 300 + // const timeout = max * (backoff * max) / 2 // 20 / 300 => 63,000 // max = (2*totalTime/backoff) ^ (1/2) // timeout = 11,250 for {max: 15, backoff: 100} // refresh to wait contact ready - await retryPromise({ max: max, backoff: backoff }, async (attempt: number) => { - log.silly('PuppeteerFriendRequest', 'accept() retryPromise() attempt %d with timeout %d', attempt, timeout) + // await retryPromise({ max: max, backoff: backoff }, async (attempt: number) => { + // log.silly('PuppeteerFriendRequest', 'accept() retryPromise() attempt %d with timeout %d', attempt, timeout) + + // await this.contact().ready() + + // if (this.contact().isReady()) { + // log.verbose('PuppeteerFriendRequest', 'accept() with contact %s ready()', this.contact().name()) + // return + // } + // throw new Error('FriendRequest.accept() content.ready() not ready') + + // }).catch((e: Error) => { + // log.warn('PuppeteerFriendRequest', 'accept() rejected for contact %s because %s', this.contact, e && e.message || e) + // }) + await Misc.retry(async (retry, attempt) => { + log.silly('PuppeteerFriendRequest', 'accept() retryPromise() attempt %d', attempt) await this.contact().ready() @@ -184,7 +198,7 @@ export class FriendRequest extends PuppetAccessory { log.verbose('PuppeteerFriendRequest', 'accept() with contact %s ready()', this.contact().name()) return } - throw new Error('FriendRequest.accept() content.ready() not ready') + retry(new Error('FriendRequest.accept() content.ready() not ready')) }).catch((e: Error) => { log.warn('PuppeteerFriendRequest', 'accept() rejected for contact %s because %s', this.contact, e && e.message || e) diff --git a/src/misc.spec.ts b/src/misc.spec.ts index c0c0a2f7..ea6e7b9b 100755 --- a/src/misc.spec.ts +++ b/src/misc.spec.ts @@ -19,13 +19,18 @@ */ // tslint:disable:no-shadowed-variable import * as test from 'blue-tape' -// import * as sinon from 'sinon' +import * as sinon from 'sinon' // const sinonTest = require('sinon-test')(sinon) import * as http from 'http' import * as express from 'express' -import Misc from './misc' +import promiseRetry = require('promise-retry') + +import Misc from './misc' +import { + log, +} from './config' test('stripHtml()', async t => { const HTML_BEFORE_STRIP = 'OuterInner' @@ -164,3 +169,52 @@ test('getPort() for an available socket port', async t => { } t.pass('should has no exception after loop test') }) + +test('promiseRetry()', async t => { + const EXPECTED_RESOLVE = 'Okey' + const EXPECTED_REJECT = 'NotTheTime' + + function delayedFactory(timeout: number) { + const startTime = Date.now() + return function() { + const nowTime = Date.now() + log.silly('MiscTest', 'promiseRetry() delayedFactory(' + timeout + '): ' + (nowTime - startTime)) + if (nowTime - startTime > timeout) { + return Promise.resolve(EXPECTED_RESOLVE) + } + return Promise.reject(EXPECTED_REJECT) + } + } + + const thenSpy = sinon.spy() + + const delay500 = delayedFactory(500) + await promiseRetry( + { + minTimeout : 1, + retries : 1, + }, + function(retry) { + return delay500().catch(retry) + }, + ).catch((e: any) => { + thenSpy(e) + }) + t.true(thenSpy.withArgs(EXPECTED_REJECT).calledOnce, 'should got EXPECTED_REJECT when wait not enough') + + thenSpy.resetHistory() + const anotherDelay50 = delayedFactory(50) + await promiseRetry( + { + minTimeout: 1, + retries: 100, + }, + function(retry) { + return anotherDelay50().catch(retry) + }, + ) + .then((r: string) => { + thenSpy(r) + }) + t.true(thenSpy.withArgs(EXPECTED_RESOLVE).calledOnce, 'should got EXPECTED_RESOLVE when wait enough') +}) diff --git a/src/misc.ts b/src/misc.ts index e3864e92..235fa9c1 100644 --- a/src/misc.ts +++ b/src/misc.ts @@ -25,6 +25,9 @@ import { } from 'stream' import * as url from 'url' +import promiseRetry = require('promise-retry') +import { WrapOptions } from 'retry' + import { log } from './config' export class Misc { @@ -269,6 +272,28 @@ export class Misc { // return 'application/octet-stream' // } // } + + public static async retry( + retryableFn: ( + retry: (error: Error) => never, + attempt: number, + ) => Promise, + ): Promise { + const factor = 3 + const minTimeout = 10 + const maxTimeout = 20 * 1000 + const retries = 9 + const unref = true + + const retryOptions: WrapOptions = { + minTimeout, + maxTimeout, + retries, + unref, + factor, + } + return await promiseRetry(retryOptions, retryableFn) + } } export default Misc diff --git a/src/puppet-puppeteer/bridge.spec.ts b/src/puppet-puppeteer/bridge.spec.ts index f70af22a..797e5b75 100755 --- a/src/puppet-puppeteer/bridge.spec.ts +++ b/src/puppet-puppeteer/bridge.spec.ts @@ -26,7 +26,7 @@ import * as test from 'blue-tape' import { launch, } from 'puppeteer' -import { spy } from 'sinon' +// import { spy } from 'sinon' import Profile from '../profile' @@ -180,43 +180,43 @@ test('clickSwitchAccount()', async t => { }) }) -test('retryPromise()', async t => { - const EXPECTED_RESOLVE = 'Okey' - const EXPECTED_REJECT = 'NotTheTime' - - function delayedFactory(timeout: number) { - const startTime = Date.now() - return function() { - const nowTime = Date.now() - if (nowTime - startTime > timeout) { - return Promise.resolve(EXPECTED_RESOLVE) - } - return Promise.reject(EXPECTED_REJECT) - } - } - - const thenSpy = spy() - - const retryPromise = require('retry-promise').default - - const delay500 = delayedFactory(500) - await retryPromise({ max: 1, backoff: 1 }, function() { - return delay500() - }).catch((e: any) => { - thenSpy(e) - }) - t.true(thenSpy.withArgs(EXPECTED_REJECT).calledOnce, 'should got EXPECTED_REJECT when wait not enough') - - thenSpy.resetHistory() - const anotherDelay50 = delayedFactory(50) - await retryPromise({ max: 6, backoff: 10 }, function() { - return anotherDelay50() - }) - .then((r: string) => { - thenSpy(r) - }) - t.true(thenSpy.withArgs(EXPECTED_RESOLVE).calledOnce, 'should got EXPECTED_RESOLVE when wait enough') -}) +// test('retryPromise()', async t => { +// const EXPECTED_RESOLVE = 'Okey' +// const EXPECTED_REJECT = 'NotTheTime' + +// function delayedFactory(timeout: number) { +// const startTime = Date.now() +// return function() { +// const nowTime = Date.now() +// if (nowTime - startTime > timeout) { +// return Promise.resolve(EXPECTED_RESOLVE) +// } +// return Promise.reject(EXPECTED_REJECT) +// } +// } + +// const thenSpy = spy() + +// const retryPromise = require('retry-promise').default + +// const delay500 = delayedFactory(500) +// await retryPromise({ max: 1, backoff: 1 }, function() { +// return delay500() +// }).catch((e: any) => { +// thenSpy(e) +// }) +// t.true(thenSpy.withArgs(EXPECTED_REJECT).calledOnce, 'should got EXPECTED_REJECT when wait not enough') + +// thenSpy.resetHistory() +// const anotherDelay50 = delayedFactory(50) +// await retryPromise({ max: 6, backoff: 10 }, function() { +// return anotherDelay50() +// }) +// .then((r: string) => { +// thenSpy(r) +// }) +// t.true(thenSpy.withArgs(EXPECTED_RESOLVE).calledOnce, 'should got EXPECTED_RESOLVE when wait enough') +// }) test('WechatyBro.ding()', async t => { const profile = new Profile(Math.random().toString(36).substr(2, 5)) diff --git a/src/puppet-puppeteer/bridge.ts b/src/puppet-puppeteer/bridge.ts index 27b34e4b..9e27115d 100644 --- a/src/puppet-puppeteer/bridge.ts +++ b/src/puppet-puppeteer/bridge.ts @@ -31,7 +31,7 @@ import StateSwitch from 'state-switch' import { parseString } from 'xml2js' /* tslint:disable:no-var-requires */ -const retryPromise = require('retry-promise').default +// const retryPromise = require('retry-promise').default import { log } from '../config' import Profile from '../profile' @@ -530,19 +530,16 @@ export class Bridge extends EventEmitter { } public async getContact(id: string): Promise { - const max = 35 - const backoff = 500 + // const max = 35 + // const backoff = 500 - // max = (2*totalTime/backoff) ^ (1/2) - // timeout = 11,250 for {max: 15, backoff: 100} - // timeout = 45,000 for {max: 30, backoff: 100} - // timeout = 30,6250 for {max: 35, backoff: 500} - const timeout = max * (backoff * max) / 2 + // const timeout = max * (backoff * max) / 2 try { - return await retryPromise({ max: max, backoff: backoff }, async (attempt: number) => { - log.silly('PuppetPuppeteerBridge', 'getContact() retryPromise: attampt %d/%d time for timeout %d', - attempt, max, timeout) + return await Misc.retry(async (retry, attempt) => { + log.silly('PuppetPuppeteerBridge', 'getContact() promiseRetry: attempt %d', + attempt, + ) try { const r = await this.proxyWechaty('getContact', id) if (r) { @@ -551,11 +548,26 @@ export class Bridge extends EventEmitter { throw new Error('got empty return value at attempt: ' + attempt) } catch (e) { log.silly('PuppetPuppeteerBridge', 'proxyWechaty(getContact, %s) exception: %s', id, e.message) - throw e + retry(e) } }) - } catch (e) { - log.warn('PuppetPuppeteerBridge', 'retryPromise() getContact() finally FAIL: %s', e.message) + + // return await retryPromise({ max: max, backoff: backoff }, async (attempt: number) => { + // log.silly('PuppetPuppeteerBridge', 'getContact() retryPromise: attampt %d/%d time for timeout %d', + // attempt, max, timeout) + // try { + // const r = await this.proxyWechaty('getContact', id) + // if (r) { + // return r + // } + // throw new Error('got empty return value at attempt: ' + attempt) + // } catch (e) { + // log.silly('PuppetPuppeteerBridge', 'proxyWechaty(getContact, %s) exception: %s', id, e.message) + // throw e + // } + // }) + } catch (e) { + log.warn('PuppetPuppeteerBridge', 'promiseRetry() getContact() finally FAIL: %s', e.message) throw e } ///////////////////////////////// diff --git a/src/puppet-puppeteer/firer.ts b/src/puppet-puppeteer/firer.ts index 4eb66fa6..f7008c65 100644 --- a/src/puppet-puppeteer/firer.ts +++ b/src/puppet-puppeteer/firer.ts @@ -18,11 +18,12 @@ */ /* tslint:disable:no-var-requires */ -const retryPromise = require('retry-promise').default +// const retryPromise = require('retry-promise').default import { log, } from '../config' +import { Misc } from '../misc' import { WebRecomendInfo, @@ -239,7 +240,9 @@ async function checkRoomJoin( const text = msg.text() - let inviteeList: string[], inviter: string + let inviteeList : string[] + let inviter : string + try { [inviteeList, inviter] = parseRoomJoin.call(this, text) } catch (e) { @@ -247,57 +250,52 @@ async function checkRoomJoin( return false // not a room join message } log.silly('PuppetPuppeteerFirer', 'checkRoomJoin() inviteeList: %s, inviter: %s', - inviteeList.join(','), - inviter, - ) + inviteeList.join(','), + inviter, + ) let inviterContact: null | Contact = null let inviteeContactList: Contact[] = [] try { - if (inviter === 'You' || inviter === '你' || inviter === 'you') { + if (/^You|你$/i.test(inviter)) { // === 'You' || inviter === '你' || inviter === 'you' inviterContact = this.userSelf() } - const max = 20 - const backoff = 300 - const timeout = max * (backoff * max) / 2 + // const max = 20 + // const backoff = 300 + // const timeout = max * (backoff * max) / 2 // 20 / 300 => 63,000 // max = (2*totalTime/backoff) ^ (1/2) // timeout = 11,250 for {max: 15, backoff: 100} - await retryPromise({ max: max, backoff: backoff }, async (attempt: number) => { - log.silly('PuppetPuppeteerFirer', 'fireRoomJoin() retryPromise() attempt %d with timeout %d', attempt, timeout) + await Misc.retry(async (retry, attempt) => { + log.silly('PuppetPuppeteerFirer', 'fireRoomJoin() retry() attempt %d', attempt) - await room.refresh() + await room.sync() let inviteeListAllDone = true for (const i in inviteeList) { - const loaded = inviteeContactList[i] instanceof Contact - - if (!loaded) { - const c = room.member(inviteeList[i]) - if (!c) { + const inviteeContact = inviteeContactList[i] + if (inviteeContact instanceof Contact) { + if (!inviteeContact.isReady()) { + log.warn('PuppetPuppeteerFirer', 'fireRoomJoin() retryPromise() isReady false for contact %s', inviteeContact.id) inviteeListAllDone = false + await inviteeContact.refresh() continue } - - await c.ready() - inviteeContactList[i] = c - - const isReady = c.isReady() - if (!isReady) { + } else { + const member = room.member(inviteeList[i]) + if (!member) { inviteeListAllDone = false continue } - } - if (inviteeContactList[i] instanceof Contact) { - const isReady = inviteeContactList[i].isReady() - if (!isReady) { - log.warn('PuppetPuppeteerFirer', 'fireRoomJoin() retryPromise() isReady false for contact %s', inviteeContactList[i].id) + await member.ready() + inviteeContactList[i] = member + + if (!member.isReady()) { inviteeListAllDone = false - await inviteeContactList[i].refresh() continue } } @@ -317,8 +315,7 @@ async function checkRoomJoin( } log.error('PuppetPuppeteerFirer', 'fireRoomJoin() not found(yet)') - return false - // throw new Error('not found(yet)') + return retry(new Error('fireRoomJoin() not found(yet)')) }).catch((e: Error) => { log.warn('PuppetPuppeteerFirer', 'fireRoomJoin() reject() inviteeContactList: %s, inviterContact: %s, error %s', -- GitLab