diff --git a/package-lock.json b/package-lock.json index 61ded33185..786b36930c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -41450,6 +41450,7 @@ "@opentelemetry/sdk-trace-web": "^1.2.0", "@opentelemetry/semantic-conventions": "^1.2.0", "@overleaf/access-token-encryptor": "*", + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", @@ -50536,6 +50537,7 @@ "@opentelemetry/sdk-trace-web": "^1.2.0", "@opentelemetry/semantic-conventions": "^1.2.0", "@overleaf/access-token-encryptor": "*", + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", diff --git a/services/web/app/src/Features/Authentication/HaveIBeenPwned.js b/services/web/app/src/Features/Authentication/HaveIBeenPwned.js index 90d5604512..c400fc7b37 100644 --- a/services/web/app/src/Features/Authentication/HaveIBeenPwned.js +++ b/services/web/app/src/Features/Authentication/HaveIBeenPwned.js @@ -6,7 +6,7 @@ */ const { callbackify } = require('util') -const fetch = require('node-fetch') +const { fetchString } = require('@overleaf/fetch-utils') const crypto = require('crypto') const Settings = require('@overleaf/settings') const Metrics = require('@overleaf/metrics') @@ -38,7 +38,7 @@ async function getScoresForPrefix(prefix) { throw INVALID_PREFIX } try { - const response = await fetch( + return await fetchString( `${Settings.apis.haveIBeenPwned.url}/range/${prefix}`, { headers: { @@ -49,11 +49,6 @@ async function getScoresForPrefix(prefix) { signal: AbortSignal.timeout(Settings.apis.haveIBeenPwned.timeout), } ) - if (!response.ok) { - throw API_ERROR - } - const body = await response.text() - return body } catch (_errorWithPotentialReferenceToPrefix) { // NOTE: Do not leak request details by passing the original error up. throw API_ERROR diff --git a/services/web/app/src/Features/Captcha/CaptchaMiddleware.js b/services/web/app/src/Features/Captcha/CaptchaMiddleware.js index e9657a7d42..abbe6cabb9 100644 --- a/services/web/app/src/Features/Captcha/CaptchaMiddleware.js +++ b/services/web/app/src/Features/Captcha/CaptchaMiddleware.js @@ -1,4 +1,4 @@ -const fetch = require('node-fetch') +const { fetchJson } = require('@overleaf/fetch-utils') const logger = require('@overleaf/logger') const Settings = require('@overleaf/settings') const Metrics = require('@overleaf/metrics') @@ -60,24 +60,22 @@ function validateCaptcha(action) { return respondInvalidCaptcha(req, res) } - const response = await fetch(Settings.recaptcha.endpoint, { - method: 'POST', - body: new URLSearchParams([ - ['secret', Settings.recaptcha.secretKey], - ['response', reCaptchaResponse], - ]), - headers: { - Accept: 'application/json', - }, - }) - const body = await response.json() - if (!response.ok) { + let body + try { + body = await fetchJson(Settings.recaptcha.endpoint, { + method: 'POST', + body: new URLSearchParams([ + ['secret', Settings.recaptcha.secretKey], + ['response', reCaptchaResponse], + ]), + }) + } catch (err) { Metrics.inc('captcha', 1, { path: action, status: 'error' }) - throw new OError('failed recaptcha siteverify request', { - statusCode: response.status, - body, + throw OError.tag(err, 'failed recaptcha siteverify request', { + body: err.body, }) } + if (!body.success) { logger.warn( { statusCode: 200, body }, diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index 252dd7f566..11d4facb7e 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/services/web/app/src/Features/Compile/ClsiManager.js @@ -1,6 +1,11 @@ const { callbackify } = require('util') const { callbackifyMultiResult } = require('../../util/promises') -const fetch = require('node-fetch') +const { + fetchString, + fetchStringWithResponse, + fetchStream, + RequestFailedError, +} = require('@overleaf/fetch-utils') const Settings = require('@overleaf/settings') const ProjectGetter = require('../Project/ProjectGetter') const ProjectEntityHandler = require('../Project/ProjectEntityHandler') @@ -206,9 +211,9 @@ async function _makeRequestWithClsiServerId( url.searchParams.set('compileBackendClass', compileBackendClass) url.searchParams.set('clsiserverid', clsiserverid) - let response + let body try { - response = await fetch(url, opts) + body = await fetchString(url, opts) } catch (err) { throw OError.tag(err, 'error making request to CLSI', { userId, @@ -216,13 +221,14 @@ async function _makeRequestWithClsiServerId( }) } - let body + let json try { - body = await response.json() + json = JSON.parse(body) } catch (err) { // some responses are empty. Ignore JSON parsing errors. } - return { response, body } + + return { body: json } } else { return await _makeRequest( projectId, @@ -265,9 +271,9 @@ async function _makeRequest( const timer = new Metrics.Timer('compile.currentBackend') - let currentBackendResponse + let response, body try { - currentBackendResponse = await fetch(url, opts) + ;({ body, response } = await fetchStringWithResponse(url, opts)) } catch (err) { throw OError.tag(err, 'error making request to CLSI', { projectId, @@ -275,20 +281,19 @@ async function _makeRequest( }) } - Metrics.inc( - `compile.currentBackend.response.${currentBackendResponse.status}` - ) + Metrics.inc(`compile.currentBackend.response.${response.status}`) - let currentBackendBody + let json try { - currentBackendBody = await currentBackendResponse.json() + json = JSON.parse(body) } catch (err) { // some responses are empty. Ignore JSON parsing errors } + timer.done() let newClsiServerId if (CLSI_COOKIES_ENABLED) { - newClsiServerId = _getClsiServerIdFromResponse(currentBackendResponse) + newClsiServerId = _getClsiServerIdFromResponse(response) await ClsiCookieManager.promises.setServerId( projectId, userId, @@ -317,7 +322,7 @@ async function _makeRequest( const { response: newBackendResponse } = result Metrics.inc(`compile.newBackend.response.${newBackendResponse.status}`) const newBackendCompileTime = new Date() - newBackendStartTime - const currentStatusCode = currentBackendResponse.status + const currentStatusCode = response.status const newStatusCode = newBackendResponse.status const statusCodeSame = newStatusCode === currentStatusCode const timeDifference = newBackendCompileTime - currentCompileTime @@ -337,8 +342,7 @@ async function _makeRequest( }) return { - response: currentBackendResponse, - body: currentBackendBody, + body: json, clsiServerId: newClsiServerId || clsiServerId, } } @@ -380,9 +384,9 @@ async function _makeNewBackendRequest( const timer = new Metrics.Timer('compile.newBackend') - let response + let response, body try { - response = await fetch(url, opts) + ;({ body, response } = await fetchStringWithResponse(url, opts)) } catch (err) { throw OError.tag(err, 'error making request to new CLSI', { userId, @@ -390,9 +394,9 @@ async function _makeNewBackendRequest( }) } - let body + let json try { - body = await response.json() + json = JSON.parse(body) } catch (err) { // Some responses are empty. Ignore JSON parsing errors } @@ -408,7 +412,7 @@ async function _makeNewBackendRequest( clsiServerId ) } - return { response, body } + return { response, body: json } } function _getCompilerUrl( @@ -445,50 +449,54 @@ async function _postToClsi( 'compile' ) const opts = { - body: JSON.stringify(req), + json: req, method: 'POST', } - let response, body, clsiServerId try { - ;({ response, body, clsiServerId } = await _makeRequest( + const { body, clsiServerId } = await _makeRequest( projectId, userId, compileGroup, compileBackendClass, url, opts - )) - } catch (err) { - throw new OError( - 'failed to make request to CLSI', - { - projectId, - userId, - compileOptions: req.compile.options, - rootResourcePath: req.compile.rootResourcePath, - }, - err ) - } - if (response.ok) { return { response: body, clsiServerId } - } else if (response.status === 413) { - return { response: { compile: { status: 'project-too-large' } } } - } else if (response.status === 409) { - return { response: { compile: { status: 'conflict' } } } - } else if (response.status === 423) { - return { response: { compile: { status: 'compile-in-progress' } } } - } else if (response.status === 503) { - return { response: { compile: { status: 'unavailable' } } } - } else { - throw new OError(`CLSI returned non-success code: ${response.status}`, { - projectId, - userId, - compileOptions: req.compile.options, - rootResourcePath: req.compile.rootResourcePath, - clsiResponse: body, - statusCode: response.status, - }) + } catch (err) { + if (err instanceof RequestFailedError) { + if (err.response.status === 413) { + return { response: { compile: { status: 'project-too-large' } } } + } else if (err.response.status === 409) { + return { response: { compile: { status: 'conflict' } } } + } else if (err.response.status === 423) { + return { response: { compile: { status: 'compile-in-progress' } } } + } else if (err.response.status === 503) { + return { response: { compile: { status: 'unavailable' } } } + } else { + throw new OError( + `CLSI returned non-success code: ${err.response.status}`, + { + projectId, + userId, + compileOptions: req.compile.options, + rootResourcePath: req.compile.rootResourcePath, + clsiResponse: err.body, + statusCode: err.response.status, + } + ) + } + } else { + throw new OError( + 'failed to make request to CLSI', + { + projectId, + userId, + compileOptions: req.compile.options, + rootResourcePath: req.compile.rootResourcePath, + }, + err + ) + } } } @@ -593,24 +601,22 @@ async function getOutputFileStream( url.searchParams.set('compileBackendClass', compileBackendClass) url.searchParams.set('compileGroup', compileGroup) url.searchParams.set('clsiserverid', clsiServerId) - const response = await fetch(url, { - method: 'GET', - signal: AbortSignal.timeout(OUTPUT_FILE_TIMEOUT_MS), - }) - if (!response.ok) { - // Drain the response body - await response.arrayBuffer() + try { + const stream = await fetchStream(url, { + signal: AbortSignal.timeout(OUTPUT_FILE_TIMEOUT_MS), + }) + return stream + } catch (err) { throw new Errors.OutputFileFetchFailedError( 'failed to fetch output file from CLSI', { projectId, userId, url, - status: response.status, + status: err.response?.status, } ) } - return response.body } function _buildRequestFromDocupdater( diff --git a/services/web/app/src/Features/Contacts/ContactManager.js b/services/web/app/src/Features/Contacts/ContactManager.js index 788bcf9434..be5bdce1fb 100644 --- a/services/web/app/src/Features/Contacts/ContactManager.js +++ b/services/web/app/src/Features/Contacts/ContactManager.js @@ -1,6 +1,6 @@ const { callbackify } = require('util') const OError = require('@overleaf/o-error') -const fetch = require('node-fetch') +const { fetchJson } = require('@overleaf/fetch-utils') const settings = require('@overleaf/settings') async function getContactIds(userId, options) { @@ -12,18 +12,11 @@ async function getContactIds(userId, options) { url.searchParams.set(key, val) } - const response = await fetch(url.toString(), { - method: 'GET', - headers: { Accept: 'application/json' }, - }) - - const body = await response.json() - - if (!response.ok) { - throw new OError( - `contacts api responded with non-success code: ${response.statusCode}`, - { user_id: userId } - ) + let body + try { + body = await fetchJson(url) + } catch (err) { + throw OError.tag(err, 'failed request to contacts API', { userId }) } return body?.contact_ids || [] @@ -31,24 +24,18 @@ async function getContactIds(userId, options) { async function addContact(userId, contactId) { const url = new URL(`${settings.apis.contacts.url}/user/${userId}/contacts`) - const response = await fetch(url.toString(), { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Accept: 'application/json', - }, - body: JSON.stringify({ contact_id: contactId }), - }) - const body = await response.json() - if (!response.ok) { - throw new OError( - `contacts api responded with non-success code: ${response.statusCode}`, - { - user_id: userId, - contact_id: contactId, - } - ) + let body + try { + body = await fetchJson(url, { + method: 'POST', + json: { contact_id: contactId }, + }) + } catch (err) { + throw OError.tag(err, 'failed request to contacts API', { + userId, + contactId, + }) } return body?.contact_ids || [] diff --git a/services/web/app/src/Features/History/HistoryManager.js b/services/web/app/src/Features/History/HistoryManager.js index 451a74444c..e1f20f1a8d 100644 --- a/services/web/app/src/Features/History/HistoryManager.js +++ b/services/web/app/src/Features/History/HistoryManager.js @@ -1,24 +1,14 @@ const { callbackify } = require('util') -const fetch = require('node-fetch') +const { fetchJson, fetchNothing } = require('@overleaf/fetch-utils') const settings = require('@overleaf/settings') const OError = require('@overleaf/o-error') const UserGetter = require('../User/UserGetter') async function initializeProject(projectId) { - const response = await fetch(`${settings.apis.project_history.url}/project`, { + const body = await fetchJson(`${settings.apis.project_history.url}/project`, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - Accept: 'application/json', - }, - body: JSON.stringify({ historyId: projectId.toString() }), + json: { historyId: projectId.toString() }, }) - if (!response.ok) { - throw new OError('failed to initialize project history', { - statusCode: response.status, - }) - } - const body = await response.json() const historyId = body && body.project && body.project.id if (!historyId) { throw new OError('project-history did not provide an id', { body }) @@ -27,27 +17,27 @@ async function initializeProject(projectId) { } async function flushProject(projectId) { - const response = await fetch( - `${settings.apis.project_history.url}/project/${projectId}/flush`, - { method: 'POST' } - ) - if (!response.ok) { - throw new OError('failed to flush project to project history', { + try { + await fetchNothing( + `${settings.apis.project_history.url}/project/${projectId}/flush`, + { method: 'POST' } + ) + } catch (err) { + throw OError.tag(err, 'failed to flush project to project history', { projectId, - statusCode: response.status, }) } } async function deleteProjectHistory(projectId) { - const response = await fetch( - `${settings.apis.project_history.url}/project/${projectId}`, - { method: 'DELETE' } - ) - if (!response.ok) { - throw new OError('failed to delete project history', { + try { + await fetchNothing( + `${settings.apis.project_history.url}/project/${projectId}`, + { method: 'DELETE' } + ) + } catch (err) { + throw OError.tag(err, 'failed to delete project history', { projectId, - statusCode: response.status, }) } } @@ -60,21 +50,18 @@ async function resyncProject(projectId, options = {}) { if (options.origin) { body.origin = options.origin } - const response = await fetch( - `${settings.apis.project_history.url}/project/${projectId}/resync`, - { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify(body), - signal: AbortSignal.timeout(6 * 60 * 1000), - } - ) - if (!response.ok) { - throw new OError('failed to resync project history', { + try { + await fetchNothing( + `${settings.apis.project_history.url}/project/${projectId}/resync`, + { + method: 'POST', + json: body, + signal: AbortSignal.timeout(6 * 60 * 1000), + } + ) + } catch (err) { + throw OError.tag(err, 'failed to resync project history', { projectId, - statusCode: response.status, }) } } @@ -89,37 +76,34 @@ async function deleteProject(projectId, historyId) { } async function _deleteProjectInProjectHistory(projectId) { - const response = await fetch( - `${settings.apis.project_history.url}/project/${projectId}`, - { method: 'DELETE' } - ) - if (!response.ok) { - throw new OError('failed to clear project history in project-history', { - projectId, - statusCode: response.status, - }) + try { + await fetchNothing( + `${settings.apis.project_history.url}/project/${projectId}`, + { method: 'DELETE' } + ) + } catch (err) { + throw OError.tag( + err, + 'failed to clear project history in project-history', + { projectId } + ) } } async function _deleteProjectInFullProjectHistory(historyId) { - const response = await fetch( - `${settings.apis.v1_history.url}/projects/${historyId}`, - { - method: 'DELETE', - headers: { - Authorization: - 'Basic ' + - Buffer.from( - `${settings.apis.v1_history.user}:${settings.apis.v1_history.pass}` - ).toString('base64'), - }, - } - ) - if (!response.ok) { - throw new OError('failed to clear project history', { - historyId, - statusCode: response.status, - }) + try { + await fetchNothing( + `${settings.apis.v1_history.url}/projects/${historyId}`, + { + method: 'DELETE', + basicAuth: { + user: settings.apis.v1_history.user, + password: settings.apis.v1_history.pass, + }, + } + ) + } catch (err) { + throw OError.tag(err, 'failed to clear project history', { historyId }) } } diff --git a/services/web/app/src/Features/Institutions/InstitutionsManager.js b/services/web/app/src/Features/Institutions/InstitutionsManager.js index 57c472fe05..6b4be74127 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsManager.js +++ b/services/web/app/src/Features/Institutions/InstitutionsManager.js @@ -3,7 +3,7 @@ const { callbackify, promisify } = require('util') const { ObjectId } = require('mongodb') const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') -const fetch = require('node-fetch') +const { fetchJson } = require('@overleaf/fetch-utils') const { getInstitutionAffiliations, getConfirmedInstitutionAffiliations, @@ -343,10 +343,9 @@ const notifyUser = ( async function fetchV1Data(institution) { const url = `${Settings.apis.v1.url}/universities/list/${institution.v1Id}` try { - const response = await fetch(url, { + const data = await fetchJson(url, { signal: AbortSignal.timeout(Settings.apis.v1.timeout), }) - const data = await response.json() institution.name = data?.name institution.countryCode = data?.country_code diff --git a/services/web/app/src/Features/Publishers/PublishersGetter.js b/services/web/app/src/Features/Publishers/PublishersGetter.js index 56b4482c84..769c8ed144 100644 --- a/services/web/app/src/Features/Publishers/PublishersGetter.js +++ b/services/web/app/src/Features/Publishers/PublishersGetter.js @@ -1,6 +1,6 @@ const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') -const fetch = require('node-fetch') +const { fetchJson } = require('@overleaf/fetch-utils') const { callbackify } = require('../../util/promises') const UserMembershipsHandler = require('../UserMembership/UserMembershipsHandler') const UserMembershipEntityConfigs = require('../UserMembership/UserMembershipEntityConfigs') @@ -14,17 +14,14 @@ async function getManagedPublishers(userId) { async function fetchV1Data(publisher) { const url = `${Settings.apis.v1.url}/api/v2/brands/${publisher.slug}` - const authorization = `Basic ${Buffer.from( - Settings.apis.v1.user + ':' + Settings.apis.v1.pass - ).toString('base64')}` try { - const response = await fetch(url, { - headers: { - Authorization: authorization, + const data = await fetchJson(url, { + basicAuth: { + user: Settings.apis.v1.user, + password: Settings.apis.v1.pass, }, signal: AbortSignal.timeout(Settings.apis.v1.timeout), }) - const data = await response.json() publisher.name = data?.name publisher.partner = data?.partner diff --git a/services/web/app/src/Features/Templates/TemplatesManager.js b/services/web/app/src/Features/Templates/TemplatesManager.js index f3b506a28b..72d8305075 100644 --- a/services/web/app/src/Features/Templates/TemplatesManager.js +++ b/services/web/app/src/Features/Templates/TemplatesManager.js @@ -1,13 +1,3 @@ -/* eslint-disable - max-len, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const { Project } = require('../../models/Project') const OError = require('@overleaf/o-error') const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler') @@ -19,8 +9,8 @@ const async = require('async') const fs = require('fs') const util = require('util') const logger = require('@overleaf/logger') +const { fetchJson, RequestFailedError } = require('@overleaf/fetch-utils') const request = require('request') -const fetch = require('node-fetch') const settings = require('@overleaf/settings') const crypto = require('crypto') const Errors = require('../Errors/Errors') @@ -52,7 +42,7 @@ const TemplatesManager = { return callback(err) }) FileWriter.ensureDumpFolderExists(function (err) { - if (err != null) { + if (err) { return callback(err) } @@ -77,7 +67,7 @@ const TemplatesManager = { dumpPath, attributes, function (err, project) { - if (err != null) { + if (err) { OError.tag(err, 'problem building project from zip', { zipReq, }) @@ -96,11 +86,11 @@ const TemplatesManager = { ), ], function (err) { - if (err != null) { + if (err) { return callback(err) } fs.unlink(dumpPath, function (err) { - if (err != null) { + if (err) { return logger.err({ err }, 'error unlinking template zip') } }) @@ -113,7 +103,7 @@ const TemplatesManager = { update, {}, function (err) { - if (err != null) { + if (err) { return callback(err) } callback(null, project) @@ -166,32 +156,22 @@ const TemplatesManager = { `/api/v2/templates/${templateId}`, settings.apis.v1.url ) - const response = await fetch(url, { - headers: { - Authorization: - 'Basic ' + - Buffer.from( - `${settings.apis.v1.user}:${settings.apis.v1.pass}` - ).toString('base64'), - Accept: 'application/json', - }, - signal: AbortSignal.timeout(settings.apis.v1.timeout), - }) - if (response.status === 404) { - throw new Errors.NotFoundError() + try { + return await fetchJson(url, { + basicAuth: { + user: settings.apis.v1.user, + password: settings.apis.v1.pass, + }, + signal: AbortSignal.timeout(settings.apis.v1.timeout), + }) + } catch (err) { + if (err instanceof RequestFailedError && err.response.status === 404) { + throw new Errors.NotFoundError() + } else { + throw err + } } - - if (response.status !== 200) { - logger.warn( - { templateId }, - "[TemplateMetrics] Couldn't fetch template data from v1" - ) - throw new Error("Couldn't fetch template data from v1") - } - - const body = await response.json() - return body }, }, } diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsQueueManager.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsQueueManager.js index 4b79c97e01..160198e1a8 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsQueueManager.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsQueueManager.js @@ -1,21 +1,13 @@ const Settings = require('@overleaf/settings') const OError = require('@overleaf/o-error') -const fetch = require('node-fetch') +const { fetchJson } = require('@overleaf/fetch-utils') async function getQueues(userId) { - const response = await fetch( - `${Settings.apis.tpdsworker.url}/queues/${userId}`, - { - headers: { - Accept: 'application/json', - }, - } - ) - if (!response.ok) { - throw new OError('failed to query TPDS queues for user', { userId }) + try { + return await fetchJson(`${Settings.apis.tpdsworker.url}/queues/${userId}`) + } catch (err) { + throw OError.tag(err, 'failed to query TPDS queues for user', { userId }) } - const body = await response.json() - return body } module.exports = { diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js index 61477a501e..d7e18bfe0c 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateSender.js @@ -4,7 +4,7 @@ const { callbackify } = require('util') const logger = require('@overleaf/logger') const metrics = require('@overleaf/metrics') const Path = require('path') -const fetch = require('node-fetch') +const { fetchNothing } = require('@overleaf/fetch-utils') const settings = require('@overleaf/settings') const CollaboratorsGetter = @@ -184,17 +184,10 @@ async function deleteProject(params) { metrics.inc('tpds.delete-project') // send the request directly to project archiver, bypassing third-party-datastore try { - const response = await fetch( + await fetchNothing( `${settings.apis.project_archiver.url}/project/${projectId}`, { method: 'DELETE' } ) - if (!response.ok) { - logger.error( - { statusCode: response.status, projectId }, - 'error deleting project in third party datastore (project_archiver)' - ) - return false - } return true } catch (err) { logger.error( @@ -213,19 +206,11 @@ async function enqueue(group, method, job) { } try { const url = new URL('/enqueue/web_to_tpds_http_requests', tpdsWorkerUrl) - const response = await fetch(url, { + await fetchNothing(url, { method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ group, job, method }), + json: { group, job, method }, signal: AbortSignal.timeout(5 * 1000), }) - if (!response.ok) { - // log error and continue - logger.error( - { statusCode: response.status, group, job, method }, - 'error enqueueing tpdsworker job' - ) - } } catch (err) { // log error and continue logger.error({ err, group, job, method }, 'error enqueueing tpdsworker job') diff --git a/services/web/package.json b/services/web/package.json index 27feb2f156..ffa79386ab 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -100,6 +100,7 @@ "@opentelemetry/sdk-trace-web": "^1.2.0", "@opentelemetry/semantic-conventions": "^1.2.0", "@overleaf/access-token-encryptor": "*", + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", diff --git a/services/web/scripts/delete_orphaned_project_archives.js b/services/web/scripts/delete_orphaned_project_archives.js index c1db9591e6..67be55cf02 100644 --- a/services/web/scripts/delete_orphaned_project_archives.js +++ b/services/web/scripts/delete_orphaned_project_archives.js @@ -1,12 +1,11 @@ const Settings = require('@overleaf/settings') -const OError = require('@overleaf/o-error') +const { fetchJson } = require('@overleaf/fetch-utils') const { waitForDb } = require('../app/src/infrastructure/mongodb') const { promiseMapWithLimit } = require('../app/src/util/promises') const { getHardDeletedProjectIds } = require('./delete_orphaned_data_helper') const TpdsUpdateSender = require('../app/src/Features/ThirdPartyDataStore/TpdsUpdateSender') const { promisify } = require('util') const { ObjectId } = require('mongodb') -const fetch = require('node-fetch') const sleep = promisify(setTimeout) const START_OFFSET = process.env.START_OFFSET @@ -34,15 +33,7 @@ async function main() { const url = new URL(`${Settings.apis.project_archiver.url}/project/list`) url.searchParams.append('pageToken', pageToken) url.searchParams.append('startOffset', startOffset) - const response = await fetch(url, { - headers: { Accept: 'application/json' }, - }) - if (!response.ok) { - throw new OError('Failed to get list of projects from project archiver', { - status: response.status, - }) - } - const { nextPageToken, entries } = await response.json() + const { nextPageToken, entries } = await fetchJson(url) pageToken = nextPageToken startOffset = undefined @@ -60,6 +51,7 @@ async function main() { ) } } + async function processBatch(entries) { const projectIdToPrefix = new Map() for (const { prefix, projectId } of entries) { diff --git a/services/web/scripts/learn/checkSanitize/scrape.js b/services/web/scripts/learn/checkSanitize/scrape.js index 621eb132d2..7d14d4504f 100644 --- a/services/web/scripts/learn/checkSanitize/scrape.js +++ b/services/web/scripts/learn/checkSanitize/scrape.js @@ -1,7 +1,11 @@ const Path = require('path') const fs = require('fs') -const fetch = require('node-fetch') +const { + fetchString, + fetchJson, + RequestFailedError, +} = require('@overleaf/fetch-utils') const CACHE_IN = Path.join( Path.dirname(Path.dirname(Path.dirname(__dirname))), @@ -17,11 +21,16 @@ async function scrape(baseUrl, page) { format: 'json', redirects: true, }).toString() - const response = await fetch(uri) - if (response.status !== 200) { - console.error(response.status, page, response) + + try { + return await fetchString(uri) + } catch (err) { + if (err instanceof RequestFailedError) { + console.error(err.response.status, page, err.response) + } else { + console.error(err) + } } - return await response.text() } const crypto = require('crypto') @@ -70,11 +79,18 @@ async function getAllPagesFrom(baseUrl, continueFrom) { gaplimit: 100, ...continueFrom, }).toString() - const response = await fetch(uri) - if (response.status !== 200) { - console.error(response.status, continueFrom, response) + + let blob + try { + blob = await fetchJson(uri) + } catch (err) { + if (err instanceof RequestFailedError) { + console.error(err.response.status, continueFrom, err.response) + } else { + console.error(err) + throw err + } } - const blob = await response.json() const nextContinueFrom = blob && blob.continue const pagesRaw = (blob && blob.query && blob.query.pages) || {} const pages = Object.values(pagesRaw).map(page => page.title) diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index 15a625dc14..6620ac1af9 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js @@ -1,9 +1,9 @@ const { setTimeout } = require('timers/promises') -const _ = require('lodash') const sinon = require('sinon') const { expect } = require('chai') const SandboxedModule = require('sandboxed-module') const tk = require('timekeeper') +const { RequestFailedError } = require('@overleaf/fetch-utils') const FILESTORE_URL = 'http://filestore.example.com' const CLSI_HOST = 'clsi.example.com' @@ -47,7 +47,6 @@ describe('ClsiManager', function () { this.response = { ok: true, status: 200, - json: sinon.stub().resolves(this.responseBody), headers: { raw: sinon.stub().returns({ 'set-cookie': [`${this.clsiCookieKey}=${this.newClsiServerId}`], @@ -55,7 +54,19 @@ describe('ClsiManager', function () { }, } - this.fetch = sinon.stub().resolves(this.response) + this.FetchUtils = { + fetchString: sinon + .stub() + .callsFake(() => Promise.resolve(JSON.stringify(this.responseBody))), + fetchStringWithResponse: sinon.stub().callsFake(() => + Promise.resolve({ + body: JSON.stringify(this.responseBody), + response: this.response, + }) + ), + fetchStream: sinon.stub(), + RequestFailedError, + } this.ClsiCookieManager = { promises: { clearServerId: sinon.stub().resolves(), @@ -131,7 +142,7 @@ describe('ClsiManager', function () { this.DocumentUpdaterHandler, './ClsiCookieManager': () => this.ClsiCookieManager, './ClsiStateManager': this.ClsiStateManager, - 'node-fetch': this.fetch, + '@overleaf/fetch-utils': this.FetchUtils, './ClsiFormatChecker': this.ClsiFormatChecker, '@overleaf/metrics': this.Metrics, }, @@ -179,7 +190,7 @@ describe('ClsiManager', function () { }) it('should send the request to the CLSI', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( sinon.match( url => url.host === CLSI_HOST && @@ -190,28 +201,25 @@ describe('ClsiManager', function () { ), { method: 'POST', - body: sinon.match(body => { - body = JSON.parse(body) - const options = body.compile.options - return ( - options.compiler === this.project.compiler && - options.imageName === this.project.imageName && - options.timeout === this.timeout && - options.draft === false && - options.compileGroup === 'standard' && - options.metricsMethod === 'standard' && - options.stopOnFirstError === false && - options.syncType === undefined && - body.compile.rootResourcePath === 'main.tex' && - _.isEqual( - body.compile.resources, - _makeResources(this.project, this.docs, this.files) - ) - ) + json: sinon.match({ + compile: { + options: { + compiler: this.project.compiler, + imageName: this.project.imageName, + timeout: this.timeout, + draft: false, + compileGroup: 'standard', + metricsMethod: 'standard', + stopOnFirstError: false, + syncType: undefined, + }, + rootResourcePath: 'main.tex', + resources: _makeResources(this.project, this.docs, this.files), + }, }), headers: { - 'Content-Type': 'application/json', Accept: 'application/json', + 'Content-Type': 'application/json', Cookie: `${this.clsiCookieKey}=${this.clsiServerId}`, }, } @@ -255,13 +263,6 @@ describe('ClsiManager', function () { ) }) - it('should process a request with a cookie jar', function () { - expect(this.fetch).to.have.been.calledWith( - sinon.match.any, - sinon.match(opts => opts.jar === this.jar && opts.qs == null) - ) - }) - it('should persist the cookie from the response', function () { expect( this.ClsiCookieManager.promises.setServerId @@ -320,12 +321,12 @@ describe('ClsiManager', function () { expect(this.result.status).to.equal('success') expect(this.result.clsiServerId).to.equal(this.newClsiServerId) expect(this.result.validationError).to.be.undefined - expect(this.result.stats).to.equal(this.stats) - expect(this.result.timings).to.equal(this.timings) + expect(this.result.stats).to.deep.equal(this.stats) + expect(this.result.timings).to.deep.equal(this.timings) const outputPdf = this.result.outputFiles.find( f => f.path === 'output.pdf' ) - expect(outputPdf.ranges).to.equal(this.ranges) + expect(outputPdf.ranges).to.deep.equal(this.ranges) expect(outputPdf.startXRefTable).to.equal(this.startXRefTable) expect(outputPdf.contentId).to.equal(this.contentId) expect(outputPdf.size).to.equal(this.size) @@ -385,7 +386,7 @@ describe('ClsiManager', function () { }) it('should build up the CLSI request', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( sinon.match( url => url.hostname === CLSI_HOST && @@ -396,33 +397,33 @@ describe('ClsiManager', function () { ), { method: 'POST', - body: sinon.match(body => { - const json = JSON.parse(body) - const options = json.compile.options - return ( - options.compiler === this.project.compiler && - options.timeout === 100 && - options.imageName === this.project.imageName && - options.draft === false && - options.syncType === 'incremental' && - options.syncState === '01234567890abcdef' && - options.compileGroup === 'priority' && - options.enablePdfCaching === true && - options.pdfCachingMinChunkSize === 1337 && - options.metricsMethod === 'priority' && - options.stopOnFirstError === false && - json.compile.rootResourcePath === 'main.tex' && - _.isEqual(json.compile.resources, [ + json: sinon.match({ + compile: { + options: { + compiler: this.project.compiler, + timeout: 100, + imageName: this.project.imageName, + draft: false, + syncType: 'incremental', + syncState: '01234567890abcdef', + compileGroup: 'priority', + enablePdfCaching: true, + pdfCachingMinChunkSize: 1337, + metricsMethod: 'priority', + stopOnFirstError: false, + }, + rootResourcePath: 'main.tex', + resources: [ { path: 'main.tex', content: this.docs['/main.tex'].lines.join('\n'), }, - ]) - ) + ], + }, }), headers: { - 'Content-Type': 'application/json', Accept: 'application/json', + 'Content-Type': 'application/json', Cookie: `${this.clsiCookieKey}=${this.clsiServerId}`, }, } @@ -452,14 +453,10 @@ describe('ClsiManager', function () { }) it('should still change the root path', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => - JSON.parse(body).compile.rootResourcePath === - 'chapters/chapter1.tex' - ), + json: { compile: { rootResourcePath: 'chapters/chapter1.tex' } }, }) ) }) @@ -475,14 +472,10 @@ describe('ClsiManager', function () { }) it('should change root path', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => - JSON.parse(body).compile.rootResourcePath === - 'chapters/chapter1.tex' - ), + json: { compile: { rootResourcePath: 'chapters/chapter1.tex' } }, }) ) }) @@ -498,12 +491,10 @@ describe('ClsiManager', function () { }) it('should fallback to default root doc', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => JSON.parse(body).compile.rootResourcePath === 'main.tex' - ), + json: { compile: { rootResourcePath: 'main.tex' } }, }) ) }) @@ -520,12 +511,10 @@ describe('ClsiManager', function () { }) it('should set the compiler to pdflatex', function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => JSON.parse(body).compile.options.compiler === 'pdflatex' - ), + json: { compile: { options: { compiler: 'pdflatex' } } }, }) ) }) @@ -542,12 +531,10 @@ describe('ClsiManager', function () { }) it('should set to main.tex', function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => JSON.parse(body).compile.rootResourcePath === 'main.tex' - ), + json: { compile: { rootResourcePath: 'main.tex' } }, }) ) }) @@ -600,12 +587,10 @@ describe('ClsiManager', function () { }) it('should set io to the only file', function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => JSON.parse(body).compile.rootResourcePath === 'other.tex' - ), + json: { compile: { rootResourcePath: 'other.tex' } }, }) ) }) @@ -624,12 +609,10 @@ describe('ClsiManager', function () { }) it('should add the draft option into the request', function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith( sinon.match.any, sinon.match({ - body: sinon.match( - body => JSON.parse(body).compile.options.draft === true - ), + json: { compile: { options: { draft: true } } }, }) ) }) @@ -653,20 +636,19 @@ describe('ClsiManager', function () { describe('with a sync conflict', function () { beforeEach(async function () { const conflictResponseBody = { compile: { status: 'conflict' } } - const conflictResponse = { - ...this.response, - json: sinon.stub().resolves(conflictResponseBody), - } - this.fetch + this.FetchUtils.fetchStringWithResponse .withArgs( sinon.match.any, sinon.match({ - body: sinon.match( - body => JSON.parse(body).compile.options.syncType !== 'full' + json: sinon.match( + json => json.compile.options.syncType !== 'full' ), }) ) - .resolves(conflictResponse) + .resolves({ + body: JSON.stringify(conflictResponseBody), + response: this.response, + }) this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, @@ -675,18 +657,20 @@ describe('ClsiManager', function () { }) it('should send two requests to CLSI', function () { - this.fetch.should.have.been.calledTwice + this.FetchUtils.fetchStringWithResponse.should.have.been.calledTwice }) it('should call the CLSI first without syncType:full', function () { - const compileOptions = JSON.parse(this.fetch.getCall(0).args[1].body) - .compile.options + const compileOptions = + this.FetchUtils.fetchStringWithResponse.getCall(0).args[1].json + .compile.options expect(compileOptions.syncType).to.be.undefined }) it('should call the CLSI a second time with syncType:full', function () { - const compileOptions = JSON.parse(this.fetch.getCall(1).args[1].body) - .compile.options + const compileOptions = + this.FetchUtils.fetchStringWithResponse.getCall(1).args[1].json + .compile.options expect(compileOptions.syncType).to.equal('full') }) @@ -697,8 +681,9 @@ describe('ClsiManager', function () { describe('with an unavailable response', function () { beforeEach(async function () { - this.response.json.onCall(0).resolves({ - compile: { status: 'unavailable' }, + this.FetchUtils.fetchStringWithResponse.onCall(0).resolves({ + body: JSON.stringify({ compile: { status: 'unavailable' } }), + response: this.response, }) this.result = await this.ClsiManager.promises.sendRequest( this.project._id, @@ -708,18 +693,20 @@ describe('ClsiManager', function () { }) it('should send two requests to CLSI', function () { - this.fetch.should.have.been.calledTwice + this.FetchUtils.fetchStringWithResponse.should.have.been.calledTwice }) it('should call the CLSI first without syncType:full', function () { - const compileOptions = JSON.parse(this.fetch.getCall(0).args[1].body) - .compile.options + const compileOptions = + this.FetchUtils.fetchStringWithResponse.getCall(0).args[1].json + .compile.options expect(compileOptions.syncType).to.be.undefined }) it('should call the CLSI a second time with syncType:full', function () { - const compileOptions = JSON.parse(this.fetch.getCall(1).args[1].body) - .compile.options + const compileOptions = + this.FetchUtils.fetchStringWithResponse.getCall(1).args[1].json + .compile.options expect(compileOptions.syncType).to.equal('full') }) @@ -768,8 +755,8 @@ describe('ClsiManager', function () { }) it('makes a request to the new backend', function () { - expect(this.fetch).to.have.been.calledTwice - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledTwice + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith( sinon.match( url => url.host === CLSI_HOST && @@ -779,7 +766,7 @@ describe('ClsiManager', function () { url.searchParams.get('compileGroup') === 'standard' ) ) - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith( sinon.match( url => url.toString() === @@ -826,7 +813,7 @@ describe('ClsiManager', function () { }) it('should send the request to the CLSI', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( sinon.match( url => url.host === CLSI_HOST && @@ -836,10 +823,10 @@ describe('ClsiManager', function () { ), { method: 'POST', - body: JSON.stringify(this.clsiRequest), + json: this.clsiRequest, headers: { - 'Content-Type': 'application/json', Accept: 'application/json', + 'Content-Type': 'application/json', Cookie: `${this.clsiCookieKey}=${this.clsiServerId}`, }, } @@ -901,7 +888,7 @@ describe('ClsiManager', function () { }) it('should call the delete method in the standard CLSI', function () { - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchString.should.have.been.calledWith( sinon.match( url => url.host === CLSI_HOST && @@ -927,13 +914,6 @@ describe('ClsiManager', function () { .should.equal(true) }) - it('should not add a cookie jar', function () { - expect(this.fetch).to.have.been.calledWith( - sinon.match.any, - sinon.match(opts => opts.jar == null) - ) - }) - it('should not persist a cookie on response', function () { expect(this.ClsiCookieManager.promises.setServerId).not.to.have.been .called @@ -954,13 +934,12 @@ describe('ClsiManager', function () { }) it('should call wordCount with root file', function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchString).to.have.been.calledWith( sinon.match( url => url.toString() === `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard&file=main.tex&image=mock-image-name&clsiserverid=node-1` - ), - { method: 'GET' } + ) ) }) @@ -982,7 +961,7 @@ describe('ClsiManager', function () { }) it('should call wordCount with param file', function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchString).to.have.been.calledWith( sinon.match( url => url.host === CLSI_HOST && @@ -993,10 +972,7 @@ describe('ClsiManager', function () { url.searchParams.get('clsiserverid') === 'node-2' && url.searchParams.get('file') === 'other.tex' && url.searchParams.get('image') === 'mock-image-name' - ), - { - method: 'GET', - } + ) ) }) diff --git a/services/web/test/unit/src/Contact/ContactManagerTests.js b/services/web/test/unit/src/Contact/ContactManagerTests.js index 31dddabf53..75048dcb1f 100644 --- a/services/web/test/unit/src/Contact/ContactManagerTests.js +++ b/services/web/test/unit/src/Contact/ContactManagerTests.js @@ -8,11 +8,12 @@ describe('ContactManager', function () { this.user_id = 'user-id-123' this.contact_id = 'contact-id-123' this.contact_ids = ['mock', 'contact_ids'] - this.qs = new URLSearchParams({ limit: 42 }) - this.fetch = sinon.stub() + this.FetchUtils = { + fetchJson: sinon.stub(), + } this.ContactManager = SandboxedModule.require(modulePath, { requires: { - 'node-fetch': this.fetch, + '@overleaf/fetch-utils': this.FetchUtils, '@overleaf/settings': (this.settings = { apis: { contacts: { @@ -27,11 +28,7 @@ describe('ContactManager', function () { describe('getContacts', function () { describe('with a successful response code', function () { beforeEach(async function () { - this.response = { - ok: true, - json: sinon.stub().resolves({ contact_ids: this.contact_ids }), - } - this.fetch.resolves(this.response) + this.FetchUtils.fetchJson.resolves({ contact_ids: this.contact_ids }) this.result = await this.ContactManager.promises.getContactIds( this.user_id, @@ -40,12 +37,12 @@ describe('ContactManager', function () { }) it('should get the contacts from the contacts api', function () { - this.fetch.should.have.been.calledWithMatch( - `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts?${this.qs}`, - sinon.match({ - method: 'GET', - headers: { Accept: 'application/json' }, - }) + this.FetchUtils.fetchJson.should.have.been.calledWithMatch( + sinon.match( + url => + url.toString() === + `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts?limit=42` + ) ) }) @@ -54,14 +51,14 @@ describe('ContactManager', function () { }) }) - describe('with a failed response code', function () { + describe('when an error occurs', function () { beforeEach(async function () { this.response = { ok: false, statusCode: 500, json: sinon.stub().resolves({ contact_ids: this.contact_ids }), } - this.fetch.resolves(this.response) + this.FetchUtils.fetchJson.rejects(new Error('request error')) }) it('should reject the promise', async function () { @@ -69,9 +66,7 @@ describe('ContactManager', function () { this.ContactManager.promises.getContactIds(this.user_id, { limit: 42, }) - ).to.be.rejectedWith( - 'contacts api responded with non-success code: 500' - ) + ).to.be.rejected }) }) }) @@ -79,11 +74,7 @@ describe('ContactManager', function () { describe('addContact', function () { describe('with a successful response code', function () { beforeEach(async function () { - this.response = { - ok: true, - json: sinon.stub().resolves({ contact_ids: this.contact_ids }), - } - this.fetch.resolves(this.response) + this.FetchUtils.fetchJson.resolves({ contact_ids: this.contact_ids }) this.result = await this.ContactManager.promises.addContact( this.user_id, @@ -92,17 +83,15 @@ describe('ContactManager', function () { }) it('should add the contacts for the user in the contacts api', function () { - this.fetch.should.have.been.calledWithMatch( - `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts`, + this.FetchUtils.fetchJson.should.have.been.calledWithMatch( + sinon.match( + url => + url.toString() === + `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts` + ), sinon.match({ method: 'POST', - headers: { - 'Content-Type': 'application/json', - Accept: 'application/json', - }, - body: JSON.stringify({ - contact_id: this.contact_id, - }), + json: { contact_id: this.contact_id }, }) ) }) diff --git a/services/web/test/unit/src/History/HistoryManagerTests.js b/services/web/test/unit/src/History/HistoryManagerTests.js index b64662956d..a27220d78f 100644 --- a/services/web/test/unit/src/History/HistoryManagerTests.js +++ b/services/web/test/unit/src/History/HistoryManagerTests.js @@ -12,11 +12,10 @@ describe('HistoryManager', function () { this.AuthenticationController = { getLoggedInUserId: sinon.stub().returns(this.user_id), } - this.response = { - ok: true, - json: sinon.stub(), + this.FetchUtils = { + fetchJson: sinon.stub(), + fetchNothing: sinon.stub().resolves(), } - this.fetch = sinon.stub().resolves(this.response) this.projectHistoryUrl = 'http://project_history.example.com' this.v1HistoryUrl = 'http://v1_history.example.com' this.v1HistoryUser = 'system' @@ -43,7 +42,7 @@ describe('HistoryManager', function () { this.HistoryManager = SandboxedModule.require(MODULE_PATH, { requires: { - 'node-fetch': this.fetch, + '@overleaf/fetch-utils': this.FetchUtils, '@overleaf/settings': this.settings, '../User/UserGetter': this.UserGetter, }, @@ -57,14 +56,14 @@ describe('HistoryManager', function () { describe('project history returns a successful response', function () { beforeEach(async function () { - this.response.json.resolves({ project: { id: this.historyId } }) + this.FetchUtils.fetchJson.resolves({ project: { id: this.historyId } }) this.result = await this.HistoryManager.promises.initializeProject( this.historyId ) }) it('should call the project history api', function () { - this.fetch.should.have.been.calledWithMatch( + this.FetchUtils.fetchJson.should.have.been.calledWithMatch( `${this.settings.apis.project_history.url}/project`, { method: 'POST' } ) @@ -77,7 +76,7 @@ describe('HistoryManager', function () { describe('project history returns a response without the project id', function () { it('should throw an error', async function () { - this.response.json.resolves({ project: {} }) + this.FetchUtils.fetchJson.resolves({ project: {} }) await expect( this.HistoryManager.promises.initializeProject(this.historyId) ).to.be.rejected @@ -86,7 +85,7 @@ describe('HistoryManager', function () { describe('project history errors', function () { it('should propagate the error', async function () { - this.fetch.rejects(new Error('problem connecting')) + this.FetchUtils.fetchJson.rejects(new Error('problem connecting')) await expect( this.HistoryManager.promises.initializeProject(this.historyId) ).to.be.rejected @@ -255,23 +254,20 @@ describe('HistoryManager', function () { }) it('should call the project-history service', async function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchNothing).to.have.been.calledWith( `${this.projectHistoryUrl}/project/${projectId}`, { method: 'DELETE' } ) }) it('should call the v1-history service', async function () { - expect(this.fetch).to.have.been.calledWith( + expect(this.FetchUtils.fetchNothing).to.have.been.calledWith( `${this.v1HistoryUrl}/projects/${historyId}`, { method: 'DELETE', - headers: { - Authorization: - 'Basic ' + - Buffer.from( - `${this.v1HistoryUser}:${this.v1HistoryPassword}` - ).toString('base64'), + basicAuth: { + user: this.v1HistoryUser, + password: this.v1HistoryPassword, }, } ) diff --git a/services/web/test/unit/src/Templates/TemplatesManagerTests.js b/services/web/test/unit/src/Templates/TemplatesManagerTests.js index 2823b3cfe1..0d8ae50b0d 100644 --- a/services/web/test/unit/src/Templates/TemplatesManagerTests.js +++ b/services/web/test/unit/src/Templates/TemplatesManagerTests.js @@ -13,6 +13,7 @@ const SandboxedModule = require('sandboxed-module') const assert = require('assert') const sinon = require('sinon') +const { RequestFailedError } = require('@overleaf/fetch-utils') const modulePath = '../../../../app/src/Features/Templates/TemplatesManager' @@ -61,9 +62,13 @@ describe('TemplatesManager', function () { } this.Project = { updateOne: sinon.stub().callsArgWith(3, null) } this.FileWriter = { ensureDumpFolderExists: sinon.stub().callsArg(0) } + this.FetchUtils = { + fetchJson: sinon.stub(), + RequestFailedError, + } this.TemplatesManager = SandboxedModule.require(modulePath, { requires: { - 'node-fetch': sinon.stub(), + '@overleaf/fetch-utils': this.FetchUtils, '../Uploads/ProjectUploadManager': this.ProjectUploadManager, '../Project/ProjectOptionsHandler': this.ProjectOptionsHandler, '../Project/ProjectRootDocManager': this.ProjectRootDocManager, diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js index b4886d1c4b..c4e98e8144 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js @@ -26,6 +26,10 @@ describe('TpdsUpdateSender', function () { _id: '12390i', } this.memberIds = [userId, collaberatorRef, readOnlyRef] + this.enqueueUrl = new URL( + 'http://tpdsworker/enqueue/web_to_tpds_http_requests' + ) + this.CollaboratorsGetter = { promises: { getInvitedMemberIds: sinon.stub().resolves(this.memberIds), @@ -36,7 +40,9 @@ describe('TpdsUpdateSender', function () { ok: true, json: sinon.stub(), } - this.fetch = sinon.stub().resolves(this.response) + this.FetchUtils = { + fetchNothing: sinon.stub().resolves(), + } this.settings = { siteUrl, apis: { @@ -69,7 +75,7 @@ describe('TpdsUpdateSender', function () { requires: { mongodb: { ObjectId }, '@overleaf/settings': this.settings, - 'node-fetch': this.fetch, + '@overleaf/fetch-utils': this.FetchUtils, '../Collaborators/CollaboratorsGetter': this.CollaboratorsGetter, '../User/UserGetter.js': this.UserGetter, '@overleaf/metrics': { @@ -82,7 +88,7 @@ describe('TpdsUpdateSender', function () { describe('enqueue', function () { it('should not call request if there is no tpdsworker url', async function () { await this.TpdsUpdateSender.promises.enqueue(null, null, null) - this.fetch.should.not.have.been.called + this.FetchUtils.fetchNothing.should.not.have.been.called }) it('should post the message to the tpdsworker', async function () { @@ -91,15 +97,13 @@ describe('TpdsUpdateSender', function () { const method0 = 'somemethod0' const job0 = 'do something' await this.TpdsUpdateSender.promises.enqueue(group0, method0, job0) - this.fetch.should.have.been.calledWith( - new URL('http://tpdsworker/enqueue/web_to_tpds_http_requests'), - sinon.match({ method: 'POST' }) + this.FetchUtils.fetchNothing.should.have.been.calledWithMatch( + this.enqueueUrl, + { + method: 'POST', + json: { group: group0, job: job0, method: method0 }, + } ) - const opts = this.fetch.firstCall.args[1] - const body = JSON.parse(opts.body) - body.group.should.equal(group0) - body.job.should.equal(job0) - body.method.should.equal(method0) }) }) @@ -119,39 +123,56 @@ describe('TpdsUpdateSender', function () { projectName, }) - const { - group: group0, - job: job0, - method: method0, - } = JSON.parse(this.fetch.firstCall.args[1].body) - group0.should.equal(userId.toString()) - method0.should.equal('pipeStreamFrom') - job0.method.should.equal('post') - job0.streamOrigin.should.equal( - `${filestoreUrl}/project/${projectId}/file/${fileId}` + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'pipeStreamFrom', + job: { + method: 'post', + streamOrigin: `${filestoreUrl}/project/${projectId}/file/${fileId}`, + uri: `${thirdPartyDataStoreApiUrl}/user/${userId}/entity/${encodeURIComponent( + projectName + )}${encodeURIComponent(path)}`, + headers: { + sl_all_user_ids: JSON.stringify([userId]), + sl_project_owner_user_id: userId, + }, + }, + }, + } ) - const expectedUrl = `${thirdPartyDataStoreApiUrl}/user/${userId}/entity/${encodeURIComponent( - projectName - )}${encodeURIComponent(path)}` - job0.uri.should.equal(expectedUrl) - job0.headers.sl_all_user_ids.should.equal(JSON.stringify([userId])) - job0.headers.sl_project_owner_user_id.should.equal(userId.toString()) - const { group: group1, job: job1 } = JSON.parse( - this.fetch.secondCall.args[1].body + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: collaberatorRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([collaberatorRef]), + sl_project_owner_user_id: userId, + }, + }, + }, + } ) - group1.should.equal(collaberatorRef.toString()) - job1.headers.sl_all_user_ids.should.equal( - JSON.stringify([collaberatorRef]) - ) - job1.headers.sl_project_owner_user_id.should.equal(userId.toString()) - const { group: group2, job: job2 } = JSON.parse( - this.fetch.thirdCall.args[1].body + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: readOnlyRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([readOnlyRef]), + sl_project_owner_user_id: userId, + }, + }, + }, + } ) - group2.should.equal(readOnlyRef.toString()) - job2.headers.sl_all_user_ids.should.equal(JSON.stringify([readOnlyRef])) - job2.headers.sl_project_owner_user_id.should.equal(userId.toString()) }) it('post doc with stream origin of docstore', async function () { @@ -167,37 +188,53 @@ describe('TpdsUpdateSender', function () { projectName, }) - const { - group: group0, - job: job0, - method: method0, - } = JSON.parse(this.fetch.firstCall.args[1].body) - - group0.should.equal(userId.toString()) - method0.should.equal('pipeStreamFrom') - job0.method.should.equal('post') - const expectedUrl = `${thirdPartyDataStoreApiUrl}/user/${userId.toString()}/entity/${encodeURIComponent( - projectName - )}${encodeURIComponent(path)}` - job0.uri.should.equal(expectedUrl) - job0.streamOrigin.should.equal( - `${this.docstoreUrl}/project/${projectId}/doc/${docId}/raw` - ) - job0.headers.sl_all_user_ids.should.eql(JSON.stringify([userId])) - - const { group: group1, job: job1 } = JSON.parse( - this.fetch.secondCall.args[1].body - ) - group1.should.equal(collaberatorRef.toString()) - job1.headers.sl_all_user_ids.should.equal( - JSON.stringify([collaberatorRef]) + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'pipeStreamFrom', + job: { + method: 'post', + uri: `${thirdPartyDataStoreApiUrl}/user/${userId}/entity/${encodeURIComponent( + projectName + )}${encodeURIComponent(path)}`, + streamOrigin: `${this.docstoreUrl}/project/${projectId}/doc/${docId}/raw`, + headers: { + sl_all_user_ids: JSON.stringify([userId]), + }, + }, + }, + } ) - const { group: group2, job: job2 } = JSON.parse( - this.fetch.thirdCall.args[1].body + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: collaberatorRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([collaberatorRef]), + }, + }, + }, + } + ) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: readOnlyRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([readOnlyRef]), + }, + }, + }, + } ) - group2.should.equal(readOnlyRef.toString()) - job2.headers.sl_all_user_ids.should.equal(JSON.stringify([readOnlyRef])) }) it('deleting entity', async function () { @@ -211,35 +248,53 @@ describe('TpdsUpdateSender', function () { subtreeEntityIds, }) - const { - group: group0, - job: job0, - method: method0, - } = JSON.parse(this.fetch.firstCall.args[1].body) - - group0.should.equal(userId.toString()) - method0.should.equal('standardHttpRequest') - job0.method.should.equal('delete') - const expectedUrl = `${thirdPartyDataStoreApiUrl}/user/${userId}/entity/${encodeURIComponent( - projectName - )}${encodeURIComponent(path)}` - job0.headers.sl_all_user_ids.should.eql(JSON.stringify([userId])) - job0.uri.should.equal(expectedUrl) - expect(job0.json).to.deep.equal({ subtreeEntityIds }) - - const { group: group1, job: job1 } = JSON.parse( - this.fetch.secondCall.args[1].body - ) - group1.should.equal(collaberatorRef.toString()) - job1.headers.sl_all_user_ids.should.equal( - JSON.stringify([collaberatorRef]) + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'standardHttpRequest', + job: { + method: 'delete', + uri: `${thirdPartyDataStoreApiUrl}/user/${userId}/entity/${encodeURIComponent( + projectName + )}${encodeURIComponent(path)}`, + headers: { + sl_all_user_ids: JSON.stringify([userId]), + }, + json: { subtreeEntityIds }, + }, + }, + } ) - const { group: group2, job: job2 } = JSON.parse( - this.fetch.thirdCall.args[1].body + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: collaberatorRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([collaberatorRef]), + }, + }, + }, + } + ) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: readOnlyRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([readOnlyRef]), + }, + }, + }, + } ) - group2.should.equal(readOnlyRef.toString()) - job2.headers.sl_all_user_ids.should.equal(JSON.stringify([readOnlyRef])) }) it('moving entity', async function () { @@ -253,35 +308,54 @@ describe('TpdsUpdateSender', function () { projectName, }) - const { - group: group0, - job: job0, - method: method0, - } = JSON.parse(this.fetch.firstCall.args[1].body) - - group0.should.equal(userId.toString()) - method0.should.equal('standardHttpRequest') - job0.method.should.equal('put') - job0.uri.should.equal( - `${thirdPartyDataStoreApiUrl}/user/${userId}/entity` - ) - job0.json.startPath.should.equal(`/${projectName}/${startPath}`) - job0.json.endPath.should.equal(`/${projectName}/${endPath}`) - job0.headers.sl_all_user_ids.should.eql(JSON.stringify([userId])) - - const { group: group1, job: job1 } = JSON.parse( - this.fetch.secondCall.args[1].body - ) - group1.should.equal(collaberatorRef.toString()) - job1.headers.sl_all_user_ids.should.equal( - JSON.stringify([collaberatorRef]) + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'standardHttpRequest', + job: { + method: 'put', + uri: `${thirdPartyDataStoreApiUrl}/user/${userId}/entity`, + json: { + startPath: `/${projectName}/${startPath}`, + endPath: `/${projectName}/${endPath}`, + }, + headers: { + sl_all_user_ids: JSON.stringify([userId]), + }, + }, + }, + } ) - const { group: group2, job: job2 } = JSON.parse( - this.fetch.thirdCall.args[1].body + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: collaberatorRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([collaberatorRef]), + }, + }, + }, + } + ) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: readOnlyRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([readOnlyRef]), + }, + }, + }, + } ) - group2.should.equal(readOnlyRef.toString()) - job2.headers.sl_all_user_ids.should.equal(JSON.stringify([readOnlyRef])) }) it('should be able to rename a project using the move entity func', async function () { @@ -294,63 +368,87 @@ describe('TpdsUpdateSender', function () { newProjectName, }) - const { - group: group0, - job: job0, - method: method0, - } = JSON.parse(this.fetch.firstCall.args[1].body) - - group0.should.equal(userId.toString()) - method0.should.equal('standardHttpRequest') - job0.method.should.equal('put') - job0.uri.should.equal( - `${thirdPartyDataStoreApiUrl}/user/${userId}/entity` - ) - job0.json.startPath.should.equal(oldProjectName) - job0.json.endPath.should.equal(newProjectName) - job0.headers.sl_all_user_ids.should.eql(JSON.stringify([userId])) - - const { group: group1, job: job1 } = JSON.parse( - this.fetch.secondCall.args[1].body - ) - group1.should.equal(collaberatorRef.toString()) - job1.headers.sl_all_user_ids.should.equal( - JSON.stringify([collaberatorRef]) + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'standardHttpRequest', + job: { + method: 'put', + uri: `${thirdPartyDataStoreApiUrl}/user/${userId}/entity`, + json: { + startPath: oldProjectName, + endPath: newProjectName, + }, + headers: { + sl_all_user_ids: JSON.stringify([userId]), + }, + }, + }, + } ) - const { group: group2, job: job2 } = JSON.parse( - this.fetch.thirdCall.args[1].body + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: collaberatorRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([collaberatorRef]), + }, + }, + }, + } + ) + + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: readOnlyRef, + job: { + headers: { + sl_all_user_ids: JSON.stringify([readOnlyRef]), + }, + }, + }, + } ) - group2.should.equal(readOnlyRef.toString()) - job2.headers.sl_all_user_ids.should.equal(JSON.stringify([readOnlyRef])) }) it('pollDropboxForUser', async function () { - await this.TpdsUpdateSender.promises.pollDropboxForUser(userId.toString()) + await this.TpdsUpdateSender.promises.pollDropboxForUser(userId) - const { - group: group0, - job: job0, - method: method0, - } = JSON.parse(this.fetch.firstCall.args[1].body) - - group0.should.equal(userId.toString()) - method0.should.equal('standardHttpRequest') - - job0.method.should.equal('post') - job0.uri.should.equal(`${thirdPartyDataStoreApiUrl}/user/poll`) - job0.json.user_ids[0].should.equal(userId.toString()) + expect(this.FetchUtils.fetchNothing).to.have.been.calledWithMatch( + this.enqueueUrl, + { + json: { + group: userId, + method: 'standardHttpRequest', + job: { + method: 'post', + uri: `${thirdPartyDataStoreApiUrl}/user/poll`, + json: { + user_ids: [userId], + }, + }, + }, + } + ) }) }) + describe('deleteProject', function () { it('should not call request if there is no project archiver url', async function () { await this.TpdsUpdateSender.promises.deleteProject({ projectId }) - this.fetch.should.not.have.been.called + this.FetchUtils.fetchNothing.should.not.have.been.called }) it('should make a delete request to project archiver', async function () { this.settings.apis.project_archiver = { url: projectArchiverUrl } await this.TpdsUpdateSender.promises.deleteProject({ projectId }) - this.fetch.should.have.been.calledWith( + this.FetchUtils.fetchNothing.should.have.been.calledWith( `${projectArchiverUrl}/project/${projectId}`, { method: 'DELETE' } ) @@ -380,6 +478,6 @@ describe('TpdsUpdateSender', function () { path, projectName, }) - this.fetch.should.not.have.been.called + this.FetchUtils.fetchNothing.should.not.have.been.called }) })