From ad508b188202c111693927ec6bfeac2c19fad0f7 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Tue, 31 Mar 2026 10:06:55 +0200 Subject: [PATCH] [project-history] migrate simple request usages to fetch-utils (#32539) * [project-history] remove unused getQueueCounts in tests * [project-history] migrate simple request usages to fetch-utils GitOrigin-RevId: 0e299a9d2ea968b87d7f0f2fc1626393ca4e4fdc --- .../project-history/app/js/HealthChecker.js | 104 ++++++------------ .../project-history/app/js/HttpController.js | 7 +- .../scripts/clear_filestore_404.js | 21 ++-- .../clear_project_version_out_of_order.js | 21 ++-- .../acceptance/js/NumericProjectIdTests.js | 33 +++--- .../js/helpers/ProjectHistoryClient.js | 11 -- 6 files changed, 79 insertions(+), 118 deletions(-) diff --git a/services/project-history/app/js/HealthChecker.js b/services/project-history/app/js/HealthChecker.js index 0783e66221..849e126912 100644 --- a/services/project-history/app/js/HealthChecker.js +++ b/services/project-history/app/js/HealthChecker.js @@ -1,82 +1,50 @@ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import { ObjectId } from './mongodb.js' -import request from 'request' -import async from 'async' import settings from '@overleaf/settings' import logger from '@overleaf/logger' import OError from '@overleaf/o-error' import * as LockManager from './LockManager.js' +import { fetchNothing } from '@overleaf/fetch-utils' +import { callbackify } from '@overleaf/promise-utils' const { port } = settings.internal.history -export function check(callback) { +const checkCb = callbackify(check) +export { checkCb as check } + +async function check() { const projectId = new ObjectId(settings.history.healthCheck.project_id) const url = `http://127.0.0.1:${port}/project/${projectId}` logger.debug({ projectId }, 'running health check') - const jobs = [ - cb => - request.get( - { url: `http://127.0.0.1:${port}/check_lock`, timeout: 3000 }, - function (err, res, body) { - if (err != null) { - OError.tag(err, 'error checking lock for health check', { - project_id: projectId, - }) - return cb(err) - } else if ((res != null ? res.statusCode : undefined) !== 200) { - return cb( - new OError('status code not 200', { statusCode: res.statusCode }) - ) - } else { - return cb() - } - } - ), - cb => - request.post( - { url: `${url}/flush`, timeout: 10000 }, - function (err, res, body) { - if (err != null) { - OError.tag(err, 'error flushing for health check', { - project_id: projectId, - }) - return cb(err) - } else if ((res != null ? res.statusCode : undefined) !== 204) { - return cb( - new OError('status code not 204', { statusCode: res.statusCode }) - ) - } else { - return cb() - } - } - ), - cb => - request.get( - { url: `${url}/updates`, timeout: 10000 }, - function (err, res, body) { - if (err != null) { - OError.tag(err, 'error getting updates for health check', { - project_id: projectId, - }) - return cb(err) - } else if ((res != null ? res.statusCode : undefined) !== 200) { - return cb( - new OError('status code not 200', { statusCode: res.statusCode }) - ) - } else { - return cb() - } - } - ), - ] - return async.series(jobs, callback) + try { + await fetchNothing(`http://127.0.0.1:${port}/check_lock`, { + signal: AbortSignal.timeout(3_000), + }) + } catch (err) { + throw OError.tag(err, 'error checking lock for health check', { + project_id: projectId, + }) + } + + try { + await fetchNothing(`${url}/flush`, { + method: 'POST', + signal: AbortSignal.timeout(10_000), + }) + } catch (err) { + throw OError.tag(err, 'error flushing for health check', { + project_id: projectId, + }) + } + + try { + await fetchNothing(`${url}/updates`, { + signal: AbortSignal.timeout(10_000), + }) + } catch (err) { + throw OError.tag(err, 'error getting updates for health check', { + project_id: projectId, + }) + } } export function checkLock(callback) { diff --git a/services/project-history/app/js/HttpController.js b/services/project-history/app/js/HttpController.js index d05ac97e3a..1ff9194118 100644 --- a/services/project-history/app/js/HttpController.js +++ b/services/project-history/app/js/HttpController.js @@ -1,6 +1,5 @@ import logger from '@overleaf/logger' import OError from '@overleaf/o-error' -import request from 'request' import * as UpdatesProcessor from './UpdatesProcessor.js' import * as SummarizedUpdatesManager from './SummarizedUpdatesManager.js' import * as DiffManager from './DiffManager.js' @@ -16,7 +15,7 @@ import * as HistoryApiManager from './HistoryApiManager.js' import * as RetryManager from './RetryManager.js' import * as FlushManager from './FlushManager.js' import { pipeline } from 'node:stream' -import { RequestFailedError } from '@overleaf/fetch-utils' +import { fetchNothing, RequestFailedError } from '@overleaf/fetch-utils' import { z, zz, parseReq } from '@overleaf/validation-tools' const ONE_DAY_IN_SECONDS = 24 * 60 * 60 @@ -800,7 +799,9 @@ export function retryFailures(req, res, next) { const found = key.match(/^X-CALLBACK-(.*)/i) callbackHeaders[found[1]] = req.headers[key] } - request({ url: callbackUrl, headers: callbackHeaders }) + fetchNothing(callbackUrl, { headers: callbackHeaders }).catch(err => { + logger.warn({ err }, 'failed to ping callback url') + }) } } else { if (error != null) { diff --git a/services/project-history/scripts/clear_filestore_404.js b/services/project-history/scripts/clear_filestore_404.js index 3c9ca98cc7..ddb4bdc1eb 100755 --- a/services/project-history/scripts/clear_filestore_404.js +++ b/services/project-history/scripts/clear_filestore_404.js @@ -10,10 +10,10 @@ import async from 'async' import logger from '@overleaf/logger' -import request from 'request' import Settings from '@overleaf/settings' import redis from '@overleaf/redis-wrapper' import { db, ObjectId } from '../app/js/mongodb.js' +import { fetchStringWithResponse } from '@overleaf/fetch-utils' logger.logger.level('fatal') @@ -100,15 +100,16 @@ function checkAndClear(project, callback) { const url = Settings.apis.documentupdater.url + '/project/' + projectId if (force) { console.log('3. making request to clear docupdater', url) - request.delete(url, (err, response, body) => { - console.log( - ' - result of request', - err, - response && response.statusCode, - body - ) - cb(err) - }) + fetchStringWithResponse(url, { method: 'DELETE' }).then( + ({ response, body }) => { + console.log(' - result of request: success', response.status, body) + cb() + }, + err => { + console.log(' - result of request: error', err) + cb(err) + } + ) } else { console.log('3. dry run, would request DELETE on url', url) cb() diff --git a/services/project-history/scripts/clear_project_version_out_of_order.js b/services/project-history/scripts/clear_project_version_out_of_order.js index 54883a8c1f..13bb25db6d 100755 --- a/services/project-history/scripts/clear_project_version_out_of_order.js +++ b/services/project-history/scripts/clear_project_version_out_of_order.js @@ -10,10 +10,10 @@ import async from 'async' import logger from '@overleaf/logger' -import request from 'request' import Settings from '@overleaf/settings' import redis from '@overleaf/redis-wrapper' import { db, ObjectId } from '../app/js/mongodb.js' +import { fetchStringWithResponse } from '@overleaf/fetch-utils' logger.logger.level('fatal') @@ -139,15 +139,16 @@ function checkAndClear(project, callback) { const url = Settings.apis.documentupdater.url + '/project/' + projectId if (force) { console.log('3. making request to clear docupdater', url) - request.delete(url, (err, response, body) => { - console.log( - ' - result of request', - err, - response && response.statusCode, - body - ) - cb(err) - }) + fetchStringWithResponse(url, { method: 'DELETE' }).then( + ({ response, body }) => { + console.log(' - result of request: success', response.status, body) + cb() + }, + err => { + console.log(' - result of request: error', err) + cb(err) + } + ) } else { console.log('3. dry run, would request DELETE on url', url) cb() diff --git a/services/project-history/test/acceptance/js/NumericProjectIdTests.js b/services/project-history/test/acceptance/js/NumericProjectIdTests.js index 1575a358e4..de0af69dd8 100644 --- a/services/project-history/test/acceptance/js/NumericProjectIdTests.js +++ b/services/project-history/test/acceptance/js/NumericProjectIdTests.js @@ -1,8 +1,8 @@ import { expect } from 'chai' import nock from 'nock' -import request from 'request' import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js' +import { fetchStringWithResponse } from '@overleaf/fetch-utils' const MockHistoryStore = () => nock('http://127.0.0.1:3100') const MockWeb = () => nock('http://127.0.0.1:3000') @@ -57,12 +57,13 @@ describe('NumericProjectId', function () { nock.cleanAll() }) - function makeRequest(options) { - return new Promise((resolve, reject) => { - request(options, (error, response, body) => { - if (error) return reject(error) - resolve({ response, body }) - }) + async function makeRequest(options) { + const { method, url, qs = {}, json } = options + const u = new URL(url) + u.search = new URLSearchParams(qs).toString() + return await fetchStringWithResponse(u, { + method, + json, }) } @@ -71,7 +72,7 @@ describe('NumericProjectId', function () { method: 'POST', url: `http://127.0.0.1:3054/project/${this.numericProjectId}/flush`, }) - expect(response.statusCode).to.equal(204) + expect(response.status).to.equal(204) }) it('should accept numeric project_id for dump', async function () { @@ -79,7 +80,7 @@ describe('NumericProjectId', function () { method: 'GET', url: `http://127.0.0.1:3054/project/${this.numericProjectId}/dump`, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric project_id for filetree diff', async function () { @@ -88,7 +89,7 @@ describe('NumericProjectId', function () { url: `http://127.0.0.1:3054/project/${this.numericProjectId}/filetree/diff`, qs: { from: 7, to: 8 }, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric project_id for updates', async function () { @@ -97,7 +98,7 @@ describe('NumericProjectId', function () { url: `http://127.0.0.1:3054/project/${this.numericProjectId}/updates`, qs: { min_count: 1 }, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric project_id for version', async function () { @@ -105,7 +106,7 @@ describe('NumericProjectId', function () { method: 'GET', url: `http://127.0.0.1:3054/project/${this.numericProjectId}/version`, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric project_id for snapshot', async function () { @@ -113,7 +114,7 @@ describe('NumericProjectId', function () { method: 'GET', url: `http://127.0.0.1:3054/project/${this.numericProjectId}/snapshot`, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric project_id for getLabels', async function () { @@ -121,7 +122,7 @@ describe('NumericProjectId', function () { method: 'GET', url: `http://127.0.0.1:3054/project/${this.numericProjectId}/labels`, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric project_id for createLabel', async function () { @@ -134,7 +135,7 @@ describe('NumericProjectId', function () { user_id: '507f1f77bcf86cd799439011', }, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) }) it('should accept numeric history_id for getProjectBlob', async function () { @@ -149,7 +150,7 @@ describe('NumericProjectId', function () { method: 'GET', url: `http://127.0.0.1:3054/project/${this.historyId}/blob/${blobHash}`, }) - expect(response.statusCode).to.equal(200) + expect(response.status).to.equal(200) expect(body).to.equal(blobContent) }) }) diff --git a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js index 9ca5af4f91..6b9961f185 100644 --- a/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js +++ b/services/project-history/test/acceptance/js/helpers/ProjectHistoryClient.js @@ -1,5 +1,4 @@ import { expect } from 'chai' -import request from 'request' import Settings from '@overleaf/settings' import RedisWrapper from '@overleaf/redis-wrapper' import { db } from '../../../../app/js/mongodb.js' @@ -154,16 +153,6 @@ export function getQueueLength(projectId, callback) { rclient.llen(Keys.projectHistoryOps({ project_id: projectId }), callback) } -export function getQueueCounts(callback) { - return request.get( - { - url: 'http://127.0.0.1:3054/status/queue', - json: true, - }, - callback - ) -} - export async function resyncHistory(projectId) { const response = await fetchNothing( `http://127.0.0.1:3054/project/${projectId}/resync`,