From 045e1ee33defc75ffcc6f7133b55c85cb4faf2a5 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Fri, 24 Mar 2023 10:46:14 +0100 Subject: [PATCH] Disabled got retries in testing environment - by default, got retries failed requests, which is causing issues in tests because we've disabled the network with `nock` - this is causing huge idle time because got pauses before retrying - this change disables the retries if we're running tests, so things are more stable --- .../core/core/server/lib/request-external.js | 12 ++-- .../core/server/services/webhooks/trigger.js | 2 +- ghost/core/package.json | 2 +- ghost/members-api/lib/services/geolocation.js | 11 +++- ghost/oembed-service/lib/oembed-service.js | 10 +++- .../lib/SlackNotifications.js | 4 ++ .../test/SlackNotifications.test.js | 3 +- yarn.lock | 56 ++++++++++++++++++- 8 files changed, 90 insertions(+), 10 deletions(-) diff --git a/ghost/core/core/server/lib/request-external.js b/ghost/core/core/server/lib/request-external.js index a311d2c3e1..2ce34e2228 100644 --- a/ghost/core/core/server/lib/request-external.js +++ b/ghost/core/core/server/lib/request-external.js @@ -48,15 +48,19 @@ async function errorIfInvalidUrl(options) { // same as our normal request lib but if any request in a redirect chain resolves // to a private IP address it will be blocked before the request is made. -const externalRequest = got.extend({ +const gotOpts = { headers: { 'user-agent': 'Ghost(https://github.com/TryGhost/Ghost)' }, timeout: 10000, // default is no timeout hooks: { - beforeRequest: [errorIfInvalidUrl,errorIfHostnameResolvesToPrivateIp], + beforeRequest: [errorIfInvalidUrl, errorIfHostnameResolvesToPrivateIp], beforeRedirect: [errorIfHostnameResolvesToPrivateIp] } -}); +}; -module.exports = externalRequest; +if (process.env.NODE_ENV?.startsWith('test')) { + gotOpts.retry = 0; +} + +module.exports = got.extend(gotOpts); diff --git a/ghost/core/core/server/services/webhooks/trigger.js b/ghost/core/core/server/services/webhooks/trigger.js index db80ca6a42..a667404134 100644 --- a/ghost/core/core/server/services/webhooks/trigger.js +++ b/ghost/core/core/server/services/webhooks/trigger.js @@ -102,7 +102,7 @@ class WebhookTrigger { body: reqPayload, headers, timeout: 2 * 1000, - retry: 5 + retry: process.env.NODE_ENV?.startsWith('test') ? 0 : 5 }; logging.info(`Triggering webhook for "${webhook.get('event')}" with url "${url}"`); diff --git a/ghost/core/package.json b/ghost/core/package.json index d917e063ae..7bbfdf707e 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -134,7 +134,7 @@ "@tryghost/posts-service": "0.0.0", "@tryghost/pretty-cli": "1.2.34", "@tryghost/promise": "0.3.2", - "@tryghost/request": "0.1.36", + "@tryghost/request": "0.1.37", "@tryghost/security": "0.0.0", "@tryghost/session-service": "0.0.0", "@tryghost/settings-path-manager": "0.0.0", diff --git a/ghost/members-api/lib/services/geolocation.js b/ghost/members-api/lib/services/geolocation.js index 72f7e907c7..45c8aa1944 100644 --- a/ghost/members-api/lib/services/geolocation.js +++ b/ghost/members-api/lib/services/geolocation.js @@ -7,8 +7,17 @@ module.exports = class GeolocationService { if (!ipAddress || (!IPV4_REGEX.test(ipAddress) && !IPV6_REGEX.test(ipAddress))) { return; } + + const gotOpts = { + timeout: 500 + }; + + if (process.env.NODE_ENV?.startsWith('test')) { + gotOpts.retry = 0; + } + const geojsUrl = `https://get.geojs.io/v1/ip/geo/${encodeURIComponent(ipAddress)}.json`; - const response = await got(geojsUrl, {timeout: 500}).json(); + const response = await got(geojsUrl, gotOpts).json(); return response; } }; diff --git a/ghost/oembed-service/lib/oembed-service.js b/ghost/oembed-service/lib/oembed-service.js index ae819a42d8..026a641869 100644 --- a/ghost/oembed-service/lib/oembed-service.js +++ b/ghost/oembed-service/lib/oembed-service.js @@ -187,6 +187,12 @@ class OEmbed { * @returns {Promise} */ async fetchBookmarkData(url, html) { + const gotOpts = {}; + + if (process.env.NODE_ENV?.startsWith('test')) { + gotOpts.retry = 0; + } + const metascraper = require('metascraper')([ require('metascraper-url')(), require('metascraper-title')(), @@ -194,7 +200,9 @@ class OEmbed { require('metascraper-author')(), require('metascraper-publisher')(), require('metascraper-image')(), - require('metascraper-logo-favicon')(), + require('metascraper-logo-favicon')({ + gotOpts + }), require('metascraper-logo')() ]); diff --git a/ghost/slack-notifications/lib/SlackNotifications.js b/ghost/slack-notifications/lib/SlackNotifications.js index 18335b9224..3796448c1c 100644 --- a/ghost/slack-notifications/lib/SlackNotifications.js +++ b/ghost/slack-notifications/lib/SlackNotifications.js @@ -172,6 +172,10 @@ class SlackNotifications { } }; + if (process.env.NODE_ENV?.startsWith('test')) { + requestOptions.retry = 0; + } + return await got.post(url, requestOptions); } diff --git a/ghost/slack-notifications/test/SlackNotifications.test.js b/ghost/slack-notifications/test/SlackNotifications.test.js index 381d7fa776..23ba27ac02 100644 --- a/ghost/slack-notifications/test/SlackNotifications.test.js +++ b/ghost/slack-notifications/test/SlackNotifications.test.js @@ -289,7 +289,8 @@ describe('SlackNotifications', function () { 'https://slack-webhook.com', { body: '{"data":"test"}', - headers: {'user-agent': 'Ghost/5.0.0 (https://github.com/TryGhost/Ghost)'} + headers: {'user-agent': 'Ghost/5.0.0 (https://github.com/TryGhost/Ghost)'}, + retry: 0 } ]; diff --git a/yarn.lock b/yarn.lock index 4777ed8122..6b83ba341b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5108,6 +5108,15 @@ lodash "^4.17.21" uuid "^9.0.0" +"@tryghost/errors@^1.2.22": + version "1.2.22" + resolved "https://registry.yarnpkg.com/@tryghost/errors/-/errors-1.2.22.tgz#951aee3632e8e819f90c04b86e2c443f7ef3855a" + integrity sha512-/DsrjIUvXEP2G4/vBUGPifp7D5EBqlE2N955ZGuM1iDLy6wqyA+IKy41huMuTZsZRMSUKeRL37L1OR6axMV3zw== + dependencies: + "@stdlib/utils" "^0.0.12" + lodash "^4.17.21" + uuid "^9.0.0" + "@tryghost/express-test@0.13.1": version "0.13.1" resolved "https://registry.yarnpkg.com/@tryghost/express-test/-/express-test-0.13.1.tgz#731773f9d8e9bf0e5222f50e1f829712c4248c67" @@ -5396,7 +5405,18 @@ resolved "https://registry.yarnpkg.com/@tryghost/promise/-/promise-0.3.2.tgz#a3e333e45dca3b5d2f8f72bf31d2ab50b8c4ed6e" integrity sha512-ktN80GWQ9WNfbZ7+SRYDJg6MZwttbRfsjvVurx8ng7EpD+9VOe1XP70h7Pt13TP9zD5cfSztwxzI3jEnZmCKKQ== -"@tryghost/request@0.1.36", "@tryghost/request@^0.1.36": +"@tryghost/request@0.1.37": + version "0.1.37" + resolved "https://registry.yarnpkg.com/@tryghost/request/-/request-0.1.37.tgz#c1fa04cc51b36aa25ada0db94a8e0f2666dcc12a" + integrity sha512-EtNir45HtTbFapb7vbsk/lxSh8jwwn8F2EbdHLuIQPBVQV+8GQR4tlP6GGWUgLBw06emb+Up6mtgHcSUTNEH4A== + dependencies: + "@tryghost/errors" "^1.2.22" + "@tryghost/validator" "^0.2.2" + "@tryghost/version" "^0.1.21" + got "9.6.0" + lodash "^4.17.21" + +"@tryghost/request@^0.1.36": version "0.1.36" resolved "https://registry.yarnpkg.com/@tryghost/request/-/request-0.1.36.tgz#4848adea95d1378913dfea938fb8d2628ae53ffc" integrity sha512-sH6SR2lyhXdxi5GdjMn1Eo98yIO2htid+5jbA0l3KwDQ5kCcIjayI9PUOUADG5nbjuLMtRqkqt3CafC5VRJyMg== @@ -5415,6 +5435,14 @@ caller "^1.0.1" find-root "^1.1.0" +"@tryghost/root-utils@^0.3.21": + version "0.3.21" + resolved "https://registry.yarnpkg.com/@tryghost/root-utils/-/root-utils-0.3.21.tgz#11691e10f3b83f5a3b0ff1ce290ca4d85cb3709c" + integrity sha512-JnNKipYhmlMKgD07HO+1doCiqY/TQxi+7QJE6mA69r6aZ8NgGQkJP6QDlg5D/g9K7xYzP0E6dFD7sBXVemWLlQ== + dependencies: + caller "^1.0.1" + find-root "^1.1.0" + "@tryghost/server@0.1.30": version "0.1.30" resolved "https://registry.yarnpkg.com/@tryghost/server/-/server-0.1.30.tgz#3f5e3a869648fcd333a7bb9b14b7ed7483e1b606" @@ -5447,6 +5475,13 @@ dependencies: lodash.template "^4.5.0" +"@tryghost/tpl@^0.1.23": + version "0.1.23" + resolved "https://registry.yarnpkg.com/@tryghost/tpl/-/tpl-0.1.23.tgz#fc9af14d2947c89c8547e7880700e0e4aa5b6917" + integrity sha512-UhM4p/7+LgCXm5zHFifQrWtemdqzKPXph/i6I/nJ2z4V5x3buzGzX86BmrejUu/A7tT3XYSWi5A12tLeqqc3DQ== + dependencies: + lodash.template "^4.5.0" + "@tryghost/url-utils@4.4.0", "@tryghost/url-utils@^4.0.0": version "4.4.0" resolved "https://registry.yarnpkg.com/@tryghost/url-utils/-/url-utils-4.4.0.tgz#3f2e0c50ddec49f9c90f6d611eddce51185e6cad" @@ -5470,6 +5505,17 @@ moment-timezone "^0.5.23" validator "7.2.0" +"@tryghost/validator@^0.2.2": + version "0.2.2" + resolved "https://registry.yarnpkg.com/@tryghost/validator/-/validator-0.2.2.tgz#99d120b092cb8dd3945f4adca44b13d670480a03" + integrity sha512-VXQjF6GAorWk4aC2HnoMGKRp4LzsJVcGXycjn4y2Tc0o4qiv+42E1lBL+WwhNpX1fv9j5iWR3dMD9BuQ3QpXdw== + dependencies: + "@tryghost/errors" "^1.2.22" + "@tryghost/tpl" "^0.1.23" + lodash "^4.17.21" + moment-timezone "^0.5.23" + validator "7.2.0" + "@tryghost/version@0.1.20", "@tryghost/version@^0.1.20": version "0.1.20" resolved "https://registry.yarnpkg.com/@tryghost/version/-/version-0.1.20.tgz#5b508833d7f5c5185eb78e533a6f11e2ccd32825" @@ -5478,6 +5524,14 @@ "@tryghost/root-utils" "^0.3.20" semver "^7.3.5" +"@tryghost/version@^0.1.21": + version "0.1.21" + resolved "https://registry.yarnpkg.com/@tryghost/version/-/version-0.1.21.tgz#d2a9fb959a1173075a45cdaf03c1020f47bb4932" + integrity sha512-vd6E6N6RkFlCy/FavFBd8stwUKF74Rhm8AumpO1kR2IRnS5S2HqgT4znXJ/jBT0FlfGUSQtO8peeFHhDgOd/zQ== + dependencies: + "@tryghost/root-utils" "^0.3.21" + semver "^7.3.5" + "@tryghost/webhook-mock-receiver@0.2.4": version "0.2.4" resolved "https://registry.yarnpkg.com/@tryghost/webhook-mock-receiver/-/webhook-mock-receiver-0.2.4.tgz#8e3306b2ad91c08ea03f397d72e498e87c2423a0"