Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/webdriver/src/request/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export const COMMANDS_WITHOUT_RETRY = [
findCommandPathByName('performActions'),
]
const MAX_RETRY_TIMEOUT = 100 // 100ms
const RETRY_429_BASE_DELAY = 5000 // 5s
const RETRY_429_MAX_DELAY = 30000 // 30s
const DEFAULT_HEADERS = {
'Content-Type': 'application/json; charset=utf-8',
'Connection': 'keep-alive',
Expand Down Expand Up @@ -190,11 +192,12 @@ export default abstract class WebDriverRequest extends EventEmitter {
* @param {Error} error error object that causes the retry
* @param {string} msg message that is being shown as warning to user
*/
const retry = (error: Error, msg: string) => {
const retry = async (error: Error, msg: string) => {
/**
* stop retrying if totalRetryCount was exceeded or there is no reason to
* retry, e.g. if sessionId is invalid
*/
const is429 = !(response instanceof Error) && response.statusCode === 429
if (retryCount >= totalRetryCount || error.message.includes('invalid session id')) {
log.error(`Request failed with status ${response.statusCode} due to ${error}`)
this.emit('response', { error })
Expand All @@ -207,6 +210,13 @@ export default abstract class WebDriverRequest extends EventEmitter {
this.emit('performance', { request: fullRequestOptions, durationMillisecond, success: false, error, retryCount })
log.warn(msg)
log.info(`Retrying ${retryCount}/${totalRetryCount}`)

if (is429) {
const delay = Math.min(Math.round(RETRY_429_BASE_DELAY * 2 ** (retryCount - 1) + Math.random() * 1000), RETRY_429_MAX_DELAY)
log.debug(`Request rate-limited (429), retrying in ${delay}ms`)
await new Promise<void>(resolve => setTimeout(resolve, delay))
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if tomorrow we let the user manage connectionRetryCount option? And he set value 10. How much we should wait? I'm saying that we need some kind of maximum limit.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set this on testplane side, and maybe we have to set max limit there and do it when we provide it to config for users?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usually, when implementing retires logic, jitter and the maximum limit are always used


return this._request(fullRequestOptions, transformResponse, customWdRequestAgent, totalRetryCount, retryCount)
}

Expand Down
Loading