From dccd9bf80a03d814be0595f9191dbfaf2fcd9d44 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Mon, 7 Mar 2022 09:13:00 -0500 Subject: [PATCH] Merge pull request #6918 from overleaf/em-v1-timeouts Reducing timeout for v1 calls for endpoints GitOrigin-RevId: 0f28569a1401e2fad7434df2a68a230ceb0f4aca --- .../src/Features/Exports/ExportsHandler.js | 3 ++ .../Features/Institutions/InstitutionsAPI.js | 2 +- .../Notifications/NotificationsBuilder.js | 2 +- .../Subscription/V1SubscriptionManager.js | 2 +- .../Features/Templates/TemplatesManager.js | 4 +- services/web/app/src/Features/V1/V1Api.js | 4 +- services/web/app/src/models/Institution.js | 38 ++++++++++--------- services/web/app/src/models/Publisher.js | 1 + .../unit/src/Exports/ExportsHandlerTests.js | 6 +++ 9 files changed, 38 insertions(+), 24 deletions(-) diff --git a/services/web/app/src/Features/Exports/ExportsHandler.js b/services/web/app/src/Features/Exports/ExportsHandler.js index 6d1321ce04..df7e8db4b1 100644 --- a/services/web/app/src/Features/Exports/ExportsHandler.js +++ b/services/web/app/src/Features/Exports/ExportsHandler.js @@ -178,6 +178,7 @@ module.exports = ExportsHandler = { url: `${settings.apis.v1.url}/api/v1/sharelatex/exports`, auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass }, json: export_data, + timeout: settings.apis.v1.timeout, }, function (err, res, body) { if (err != null) { @@ -236,6 +237,7 @@ module.exports = ExportsHandler = { { url: `${settings.apis.v1.url}/api/v1/sharelatex/exports/${export_id}`, auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass }, + timeout: settings.apis.v1.timeout, }, function (err, res, body) { if (err != null) { @@ -264,6 +266,7 @@ module.exports = ExportsHandler = { { url: `${settings.apis.v1.url}/api/v1/sharelatex/exports/${export_id}/${type}_url`, auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass }, + timeout: settings.apis.v1.timeout, }, function (err, res, body) { if (err != null) { diff --git a/services/web/app/src/Features/Institutions/InstitutionsAPI.js b/services/web/app/src/Features/Institutions/InstitutionsAPI.js index 98368fb96b..d84f29fa1b 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsAPI.js +++ b/services/web/app/src/Features/Institutions/InstitutionsAPI.js @@ -217,7 +217,7 @@ function makeAffiliationRequest(options, callback) { body: options.body, auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass }, json: true, - timeout: 20 * 1000, + timeout: settings.apis.v1.timeout, } if (options.method === 'GET') { requestOptions.maxAttempts = 3 diff --git a/services/web/app/src/Features/Notifications/NotificationsBuilder.js b/services/web/app/src/Features/Notifications/NotificationsBuilder.js index f6c6000b71..b3d4d68de2 100644 --- a/services/web/app/src/Features/Notifications/NotificationsBuilder.js +++ b/services/web/app/src/Features/Notifications/NotificationsBuilder.js @@ -151,7 +151,7 @@ function ipMatcherAffiliation(userId) { auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass }, body: { ip }, json: true, - timeout: 20 * 1000, + timeout: settings.apis.v1.timeout, }, function (error, response, body) { if (error != null) { diff --git a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js index 57831a9aa9..8636df141e 100644 --- a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js +++ b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js @@ -173,7 +173,7 @@ module.exports = V1SubscriptionManager = { sendImmediately: true, }, json: true, - timeout: 15 * 1000, + timeout: settings.apis.v1.timeout, } if (options.method === 'GET') { requestOptions.maxAttempts = 3 diff --git a/services/web/app/src/Features/Templates/TemplatesManager.js b/services/web/app/src/Features/Templates/TemplatesManager.js index fdb13251c8..e2a4a772d1 100644 --- a/services/web/app/src/Features/Templates/TemplatesManager.js +++ b/services/web/app/src/Features/Templates/TemplatesManager.js @@ -47,7 +47,7 @@ const TemplatesManager = { user: settings.apis.v1.user, pass: settings.apis.v1.pass, }, - timeout: 60 * 1000, + timeout: settings.apis.v1.timeout, }) zipReq.on('error', function (err) { logger.warn({ err }, 'error getting zip from template API') @@ -180,7 +180,7 @@ const TemplatesManager = { resolveWithFullResponse: true, simple: false, json: true, - timeout: 60 * 1000, + timeout: settings.apis.v1.timeout, }) if (statusCode === 404) { diff --git a/services/web/app/src/Features/V1/V1Api.js b/services/web/app/src/Features/V1/V1Api.js index df86d22f3d..6c101d996a 100644 --- a/services/web/app/src/Features/V1/V1Api.js +++ b/services/web/app/src/Features/V1/V1Api.js @@ -23,7 +23,7 @@ const DEFAULT_V1_PARAMS = { pass: settings.apis.v1.pass, }, json: true, - timeout: 30 * 1000, + timeout: settings.apis.v1.timeout, } const v1Request = request.defaults(DEFAULT_V1_PARAMS) @@ -31,7 +31,7 @@ const v1Request = request.defaults(DEFAULT_V1_PARAMS) const DEFAULT_V1_OAUTH_PARAMS = { baseUrl: settings.apis.v1.url, json: true, - timeout: 30 * 1000, + timeout: settings.apis.v1.timeout, } const v1OauthRequest = request.defaults(DEFAULT_V1_OAUTH_PARAMS) diff --git a/services/web/app/src/models/Institution.js b/services/web/app/src/models/Institution.js index 79aa0cf27a..bf3f8877bb 100644 --- a/services/web/app/src/models/Institution.js +++ b/services/web/app/src/models/Institution.js @@ -17,24 +17,28 @@ const InstitutionSchema = new Schema({ // fetch institution's data from v1 API. Errors are ignored InstitutionSchema.method('fetchV1Data', function (callback) { const url = `${settings.apis.v1.url}/universities/list/${this.v1Id}` - request.get(url, (error, response, body) => { - let parsedBody - try { - parsedBody = JSON.parse(body) - } catch (error1) { - // log error and carry on without v1 data - error = error1 - logger.err( - { model: 'Institution', v1Id: this.v1Id, error }, - '[fetchV1DataError]' - ) + request.get( + { url, timeout: settings.apis.v1.timeout }, + (error, response, body) => { + let parsedBody + try { + parsedBody = JSON.parse(body) + } catch (error1) { + // log error and carry on without v1 data + error = error1 + logger.err( + { model: 'Institution', v1Id: this.v1Id, error }, + '[fetchV1DataError]' + ) + } + this.name = parsedBody != null ? parsedBody.name : undefined + this.countryCode = + parsedBody != null ? parsedBody.country_code : undefined + this.departments = parsedBody != null ? parsedBody.departments : undefined + this.portalSlug = parsedBody != null ? parsedBody.portal_slug : undefined + callback(null, this) } - this.name = parsedBody != null ? parsedBody.name : undefined - this.countryCode = parsedBody != null ? parsedBody.country_code : undefined - this.departments = parsedBody != null ? parsedBody.departments : undefined - this.portalSlug = parsedBody != null ? parsedBody.portal_slug : undefined - callback(null, this) - }) + ) }) exports.Institution = mongoose.model('Institution', InstitutionSchema) diff --git a/services/web/app/src/models/Publisher.js b/services/web/app/src/models/Publisher.js index 84002a955a..8c8a0a6aaa 100644 --- a/services/web/app/src/models/Publisher.js +++ b/services/web/app/src/models/Publisher.js @@ -22,6 +22,7 @@ PublisherSchema.method('fetchV1Data', function (callback) { pass: settings.apis.v1.pass, sendImmediately: true, }, + timeout: settings.apis.v1.timeout, }, (error, response, body) => { let parsedBody diff --git a/services/web/test/unit/src/Exports/ExportsHandlerTests.js b/services/web/test/unit/src/Exports/ExportsHandlerTests.js index b02b121d3e..6e81a21a04 100644 --- a/services/web/test/unit/src/Exports/ExportsHandlerTests.js +++ b/services/web/test/unit/src/Exports/ExportsHandlerTests.js @@ -507,6 +507,7 @@ describe('ExportsHandler', function () { url: 'http://localhost:5000', user: 'overleaf', pass: 'pass', + timeout: 15000, }, } this.export_data = { iAmAnExport: true } @@ -537,6 +538,7 @@ describe('ExportsHandler', function () { pass: this.settings.apis.v1.pass, }, json: this.export_data, + timeout: 15000, }) }) @@ -598,6 +600,7 @@ describe('ExportsHandler', function () { url: 'http://localhost:5000', user: 'overleaf', pass: 'pass', + timeout: 15000, }, } this.export_id = 897 @@ -630,6 +633,7 @@ describe('ExportsHandler', function () { user: this.settings.apis.v1.user, pass: this.settings.apis.v1.pass, }, + timeout: 15000, }) }) @@ -648,6 +652,7 @@ describe('ExportsHandler', function () { url: 'http://localhost:5000', user: 'overleaf', pass: 'pass', + timeout: 15000, }, } this.export_id = 897 @@ -683,6 +688,7 @@ describe('ExportsHandler', function () { user: this.settings.apis.v1.user, pass: this.settings.apis.v1.pass, }, + timeout: 15000, }) })