From 51d7a8fba08bea2a712e1dd2cea8a8816a500ed2 Mon Sep 17 00:00:00 2001 From: Harjot Gill Date: Fri, 28 Apr 2023 16:27:04 -0700 Subject: [PATCH] use p-retry (#265) --- dist/index.js | 422 +++++++++++++++++++++++++++++++++++++++++++--- package-lock.json | 31 +++- package.json | 3 +- src/bot.ts | 10 +- src/review.ts | 18 +- src/utils.ts | 19 --- 6 files changed, 449 insertions(+), 54 deletions(-) delete mode 100644 src/utils.ts diff --git a/dist/index.js b/dist/index.js index ea664643..70d36992 100644 --- a/dist/index.js +++ b/dist/index.js @@ -1,7 +1,7 @@ /******/ (() => { // webpackBootstrap /******/ var __webpack_modules__ = ({ -/***/ 4909: +/***/ 6737: /***/ ((__unused_webpack_module, __webpack_exports__, __nccwpck_require__) => { "use strict"; @@ -3388,23 +3388,107 @@ var ChatGPTUnofficialProxyAPI = class { }; //# sourceMappingURL=index.js.map -;// CONCATENATED MODULE: ./lib/utils.js +// EXTERNAL MODULE: ./node_modules/retry/index.js +var retry = __nccwpck_require__(4347); +;// CONCATENATED MODULE: ./node_modules/p-retry/index.js -const retry = async (fn, args, times) => { - for (let i = 0; i < times; i++) { - try { - return await fn(...args); - } - catch (error) { - if (i === times - 1) { - throw error; - } - (0,core.warning)(`Function failed on try ${i + 1}, retrying...`); - continue; - } - } + +const networkErrorMsgs = new Set([ + 'Failed to fetch', // Chrome + 'NetworkError when attempting to fetch resource.', // Firefox + 'The Internet connection appears to be offline.', // Safari + 'Network request failed', // `cross-fetch` + 'fetch failed', // Undici (Node.js) +]); + +class p_retry_AbortError extends Error { + constructor(message) { + super(); + + if (message instanceof Error) { + this.originalError = message; + ({message} = message); + } else { + this.originalError = new Error(message); + this.originalError.stack = this.stack; + } + + this.name = 'AbortError'; + this.message = message; + } +} + +const decorateErrorWithCounts = (error, attemptNumber, options) => { + // Minus 1 from attemptNumber because the first attempt does not count as a retry + const retriesLeft = options.retries - (attemptNumber - 1); + + error.attemptNumber = attemptNumber; + error.retriesLeft = retriesLeft; + return error; }; +const isNetworkError = errorMessage => networkErrorMsgs.has(errorMessage); + +const p_retry_getDOMException = errorMessage => globalThis.DOMException === undefined + ? new Error(errorMessage) + : new DOMException(errorMessage); + +async function pRetry(input, options) { + return new Promise((resolve, reject) => { + options = { + onFailedAttempt() {}, + retries: 10, + ...options, + }; + + const operation = retry.operation(options); + + operation.attempt(async attemptNumber => { + try { + resolve(await input(attemptNumber)); + } catch (error) { + if (!(error instanceof Error)) { + reject(new TypeError(`Non-error was thrown: "${error}". You should only throw errors.`)); + return; + } + + if (error instanceof p_retry_AbortError) { + operation.stop(); + reject(error.originalError); + } else if (error instanceof TypeError && !isNetworkError(error.message)) { + operation.stop(); + reject(error); + } else { + decorateErrorWithCounts(error, attemptNumber, options); + + try { + await options.onFailedAttempt(error); + } catch (error) { + reject(error); + return; + } + + if (!operation.retry(error)) { + reject(operation.mainError()); + } + } + } + }); + + if (options.signal && !options.signal.aborted) { + options.signal.addEventListener('abort', () => { + operation.stop(); + const reason = options.signal.reason === undefined + ? p_retry_getDOMException('The operation was aborted.') + : options.signal.reason; + reject(reason instanceof Error ? reason : p_retry_getDOMException(reason)); + }, { + once: true, + }); + } + }); +} + ;// CONCATENATED MODULE: ./lib/bot.js @@ -3467,7 +3551,9 @@ Current date: ${currentDate}`; opts.parentMessageId = ids.parentMessageId; } try { - response = await retry(this.api.sendMessage.bind(this.api), [message, opts], this.options.openaiRetries); + response = await pRetry(() => this.api.sendMessage(message, opts), { + retries: this.options.openaiRetries + }); } catch (e) { if (e instanceof ChatGPTError) { @@ -4105,7 +4191,7 @@ __nccwpck_require__.a(module, async (__webpack_handle_async_dependencies__, __we __nccwpck_require__.r(__webpack_exports__); /* harmony import */ var _actions_core__WEBPACK_IMPORTED_MODULE_0__ = __nccwpck_require__(2186); /* harmony import */ var _actions_core__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__nccwpck_require__.n(_actions_core__WEBPACK_IMPORTED_MODULE_0__); -/* harmony import */ var _bot__WEBPACK_IMPORTED_MODULE_1__ = __nccwpck_require__(4909); +/* harmony import */ var _bot__WEBPACK_IMPORTED_MODULE_1__ = __nccwpck_require__(6737); /* harmony import */ var _options__WEBPACK_IMPORTED_MODULE_2__ = __nccwpck_require__(8870); /* harmony import */ var _prompts__WEBPACK_IMPORTED_MODULE_5__ = __nccwpck_require__(4272); /* harmony import */ var _review__WEBPACK_IMPORTED_MODULE_3__ = __nccwpck_require__(2612); @@ -7392,8 +7478,8 @@ function parseReview(response, patches, debug = false) { comment: sanitizedComment.trim() }; let withinPatch = false; - let bestPatchStartLine = patches[0][0]; - let bestPatchEndLine = patches[0][1]; + let bestPatchStartLine = -1; + let bestPatchEndLine = -1; let maxIntersection = 0; for (const [startLine, endLine] of patches) { const intersectionStart = Math.max(review.startLine, startLine); @@ -7410,11 +7496,20 @@ function parseReview(response, patches, debug = false) { break; } if (!withinPatch) { - review.comment = `> Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [${review.startLine}-${review.endLine}] + if (bestPatchStartLine !== -1 && bestPatchEndLine !== -1) { + review.comment = `> Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [${review.startLine}-${review.endLine}] + +${review.comment}`; + review.startLine = bestPatchStartLine; + review.endLine = bestPatchEndLine; + } + else { + review.comment = `> Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [${review.startLine}-${review.endLine}] ${review.comment}`; - review.startLine = bestPatchStartLine; - review.endLine = bestPatchEndLine; + review.startLine = patches[0][0]; + review.endLine = patches[0][1]; + } } reviews.push(review); (0,core.info)(`Stored comment for line range ${currentStartLine}-${currentEndLine}: ${currentComment.trim()}`); @@ -27051,6 +27146,289 @@ function onceStrict (fn) { } +/***/ }), + +/***/ 4347: +/***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { + +module.exports = __nccwpck_require__(6244); + +/***/ }), + +/***/ 6244: +/***/ ((__unused_webpack_module, exports, __nccwpck_require__) => { + +var RetryOperation = __nccwpck_require__(5369); + +exports.operation = function(options) { + var timeouts = exports.timeouts(options); + return new RetryOperation(timeouts, { + forever: options && (options.forever || options.retries === Infinity), + unref: options && options.unref, + maxRetryTime: options && options.maxRetryTime + }); +}; + +exports.timeouts = function(options) { + if (options instanceof Array) { + return [].concat(options); + } + + var opts = { + retries: 10, + factor: 2, + minTimeout: 1 * 1000, + maxTimeout: Infinity, + randomize: false + }; + for (var key in options) { + opts[key] = options[key]; + } + + if (opts.minTimeout > opts.maxTimeout) { + throw new Error('minTimeout is greater than maxTimeout'); + } + + var timeouts = []; + for (var i = 0; i < opts.retries; i++) { + timeouts.push(this.createTimeout(i, opts)); + } + + if (options && options.forever && !timeouts.length) { + timeouts.push(this.createTimeout(i, opts)); + } + + // sort the array numerically ascending + timeouts.sort(function(a,b) { + return a - b; + }); + + return timeouts; +}; + +exports.createTimeout = function(attempt, opts) { + var random = (opts.randomize) + ? (Math.random() + 1) + : 1; + + var timeout = Math.round(random * Math.max(opts.minTimeout, 1) * Math.pow(opts.factor, attempt)); + timeout = Math.min(timeout, opts.maxTimeout); + + return timeout; +}; + +exports.wrap = function(obj, options, methods) { + if (options instanceof Array) { + methods = options; + options = null; + } + + if (!methods) { + methods = []; + for (var key in obj) { + if (typeof obj[key] === 'function') { + methods.push(key); + } + } + } + + for (var i = 0; i < methods.length; i++) { + var method = methods[i]; + var original = obj[method]; + + obj[method] = function retryWrapper(original) { + var op = exports.operation(options); + var args = Array.prototype.slice.call(arguments, 1); + var callback = args.pop(); + + args.push(function(err) { + if (op.retry(err)) { + return; + } + if (err) { + arguments[0] = op.mainError(); + } + callback.apply(this, arguments); + }); + + op.attempt(function() { + original.apply(obj, args); + }); + }.bind(obj, original); + obj[method].options = options; + } +}; + + +/***/ }), + +/***/ 5369: +/***/ ((module) => { + +function RetryOperation(timeouts, options) { + // Compatibility for the old (timeouts, retryForever) signature + if (typeof options === 'boolean') { + options = { forever: options }; + } + + this._originalTimeouts = JSON.parse(JSON.stringify(timeouts)); + this._timeouts = timeouts; + this._options = options || {}; + this._maxRetryTime = options && options.maxRetryTime || Infinity; + this._fn = null; + this._errors = []; + this._attempts = 1; + this._operationTimeout = null; + this._operationTimeoutCb = null; + this._timeout = null; + this._operationStart = null; + this._timer = null; + + if (this._options.forever) { + this._cachedTimeouts = this._timeouts.slice(0); + } +} +module.exports = RetryOperation; + +RetryOperation.prototype.reset = function() { + this._attempts = 1; + this._timeouts = this._originalTimeouts.slice(0); +} + +RetryOperation.prototype.stop = function() { + if (this._timeout) { + clearTimeout(this._timeout); + } + if (this._timer) { + clearTimeout(this._timer); + } + + this._timeouts = []; + this._cachedTimeouts = null; +}; + +RetryOperation.prototype.retry = function(err) { + if (this._timeout) { + clearTimeout(this._timeout); + } + + if (!err) { + return false; + } + var currentTime = new Date().getTime(); + if (err && currentTime - this._operationStart >= this._maxRetryTime) { + this._errors.push(err); + this._errors.unshift(new Error('RetryOperation timeout occurred')); + return false; + } + + this._errors.push(err); + + var timeout = this._timeouts.shift(); + if (timeout === undefined) { + if (this._cachedTimeouts) { + // retry forever, only keep last error + this._errors.splice(0, this._errors.length - 1); + timeout = this._cachedTimeouts.slice(-1); + } else { + return false; + } + } + + var self = this; + this._timer = setTimeout(function() { + self._attempts++; + + if (self._operationTimeoutCb) { + self._timeout = setTimeout(function() { + self._operationTimeoutCb(self._attempts); + }, self._operationTimeout); + + if (self._options.unref) { + self._timeout.unref(); + } + } + + self._fn(self._attempts); + }, timeout); + + if (this._options.unref) { + this._timer.unref(); + } + + return true; +}; + +RetryOperation.prototype.attempt = function(fn, timeoutOps) { + this._fn = fn; + + if (timeoutOps) { + if (timeoutOps.timeout) { + this._operationTimeout = timeoutOps.timeout; + } + if (timeoutOps.cb) { + this._operationTimeoutCb = timeoutOps.cb; + } + } + + var self = this; + if (this._operationTimeoutCb) { + this._timeout = setTimeout(function() { + self._operationTimeoutCb(); + }, self._operationTimeout); + } + + this._operationStart = new Date().getTime(); + + this._fn(this._attempts); +}; + +RetryOperation.prototype.try = function(fn) { + console.log('Using RetryOperation.try() is deprecated'); + this.attempt(fn); +}; + +RetryOperation.prototype.start = function(fn) { + console.log('Using RetryOperation.start() is deprecated'); + this.attempt(fn); +}; + +RetryOperation.prototype.start = RetryOperation.prototype.try; + +RetryOperation.prototype.errors = function() { + return this._errors; +}; + +RetryOperation.prototype.attempts = function() { + return this._attempts; +}; + +RetryOperation.prototype.mainError = function() { + if (this._errors.length === 0) { + return null; + } + + var counts = {}; + var mainError = null; + var mainErrorCount = 0; + + for (var i = 0; i < this._errors.length; i++) { + var error = this._errors[i]; + var message = error.message; + var count = (counts[message] || 0) + 1; + + counts[message] = count; + + if (count >= mainErrorCount) { + mainError = error; + mainErrorCount = count; + } + } + + return mainError; +}; + + /***/ }), /***/ 9318: diff --git a/package-lock.json b/package-lock.json index fc6a0d49..eb1943ae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,8 @@ "@octokit/plugin-throttling": "^5.1.1", "minimatch": "^9.0.0", "node-fetch": "^3.3.1", - "p-limit": "^4.0.0" + "p-limit": "^4.0.0", + "p-retry": "^5.1.2" }, "devDependencies": { "@jest/globals": "^29.5.0", @@ -2174,6 +2175,11 @@ "integrity": "sha512-KufADq8uQqo1pYKVIYzfKbJfBAc0sOeXqGbFaSpv8MRmC/zXgowNZmFcbngndGk922QDmOASEXUZCaY48gs4cg==", "dev": true }, + "node_modules/@types/retry": { + "version": "0.12.1", + "resolved": "https://registry.npmjs.org/@types/retry/-/retry-0.12.1.tgz", + "integrity": "sha512-xoDlM2S4ortawSWORYqsdU+2rxdh4LRW9ytc3zmT37RIKQh6IHyKwwtKhKis9ah8ol07DCkZxPt8BBvPjC6v4g==" + }, "node_modules/@types/semver": { "version": "7.3.13", "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.3.13.tgz", @@ -8894,6 +8900,21 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/p-retry": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/p-retry/-/p-retry-5.1.2.tgz", + "integrity": "sha512-couX95waDu98NfNZV+i/iLt+fdVxmI7CbrrdC2uDWfPdUAApyxT4wmDlyOtR5KtTDmkDO0zDScDjDou9YHhd9g==", + "dependencies": { + "@types/retry": "0.12.1", + "retry": "^0.13.1" + }, + "engines": { + "node": "^12.20.0 || ^14.13.1 || >=16.0.0" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/p-timeout": { "version": "6.1.1", "resolved": "https://registry.npmjs.org/p-timeout/-/p-timeout-6.1.1.tgz", @@ -9423,6 +9444,14 @@ "node": ">=10" } }, + "node_modules/retry": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/retry/-/retry-0.13.1.tgz", + "integrity": "sha512-XQBQ3I8W1Cge0Seh+6gjj03LbmRFWuoszgK9ooCpwYIrhhoO80pfq4cUkU5DkknwfOfFteRwlZ56PYOGYyFWdg==", + "engines": { + "node": ">= 4" + } + }, "node_modules/reusify": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/reusify/-/reusify-1.0.4.tgz", diff --git a/package.json b/package.json index 56d82a45..7c2580ef 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,8 @@ "@octokit/plugin-throttling": "^5.1.1", "minimatch": "^9.0.0", "node-fetch": "^3.3.1", - "p-limit": "^4.0.0" + "p-limit": "^4.0.0", + "p-retry": "^5.1.2" }, "devDependencies": { "@jest/globals": "^29.5.0", diff --git a/src/bot.ts b/src/bot.ts index a37fdf8e..054138a0 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -8,8 +8,8 @@ import { SendMessageOptions // eslint-disable-next-line import/no-unresolved } from 'chatgpt' +import pRetry from 'p-retry' import {OpenAIOptions, Options} from './options' -import {retry} from './utils' // define type to save parentMessageId and conversationId export interface Ids { @@ -83,11 +83,9 @@ Current date: ${currentDate}` opts.parentMessageId = ids.parentMessageId } try { - response = await retry( - this.api.sendMessage.bind(this.api), - [message, opts], - this.options.openaiRetries - ) + response = await pRetry(() => this.api!.sendMessage(message, opts), { + retries: this.options.openaiRetries + }) } catch (e: unknown) { if (e instanceof ChatGPTError) { info( diff --git a/src/review.ts b/src/review.ts index 915c2c98..137f1b67 100644 --- a/src/review.ts +++ b/src/review.ts @@ -842,8 +842,8 @@ function parseReview( } let withinPatch = false - let bestPatchStartLine = patches[0][0] - let bestPatchEndLine = patches[0][1] + let bestPatchStartLine = -1 + let bestPatchEndLine = -1 let maxIntersection = 0 for (const [startLine, endLine] of patches) { @@ -866,11 +866,19 @@ function parseReview( } if (!withinPatch) { - review.comment = `> Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [${review.startLine}-${review.endLine}] + if (bestPatchStartLine !== -1 && bestPatchEndLine !== -1) { + review.comment = `> Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [${review.startLine}-${review.endLine}] ${review.comment}` - review.startLine = bestPatchStartLine - review.endLine = bestPatchEndLine + review.startLine = bestPatchStartLine + review.endLine = bestPatchEndLine + } else { + review.comment = `> Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [${review.startLine}-${review.endLine}] + +${review.comment}` + review.startLine = patches[0][0] + review.endLine = patches[0][1] + } } reviews.push(review) diff --git a/src/utils.ts b/src/utils.ts deleted file mode 100644 index ad8e330f..00000000 --- a/src/utils.ts +++ /dev/null @@ -1,19 +0,0 @@ -import {warning} from '@actions/core' - -export const retry = async ( - fn: Function, - args: unknown[], - times: number -): Promise => { - for (let i = 0; i < times; i++) { - try { - return await fn(...args) - } catch (error) { - if (i === times - 1) { - throw error - } - warning(`Function failed on try ${i + 1}, retrying...`) - continue - } - } -}