[project-history] Removed `request` dependency (#32686)

* [project-history] Removed `request` dependency

GitOrigin-RevId: 086bbbf2efeea6026127653a1f68ca6bf0476de6

services/project-history/app/js/HistoryStoreManager.js

@@ -1,6 +1,5 @@
 import { promisify } from 'node:util'
 import fs from 'node:fs'
-import request from 'request'
 import stream from 'node:stream'
 import logger from '@overleaf/logger'
 import _ from 'lodash'
@@ -9,6 +8,7 @@ import OError from '@overleaf/o-error'
 import Settings from '@overleaf/settings'
 import {
   fetchStream,
+  fetchString,
   fetchNothing,
   RequestFailedError,
 } from '@overleaf/fetch-utils'
@@ -574,33 +574,6 @@ export function getBlobStore(projectId) {
   return new BlobStore(projectId)
 }
 
-function _requestOptions(options) {
-  const requestOptions = {
-    method: options.method || 'GET',
-    url: `${Settings.overleaf.history.host}/${options.path}`,
-    timeout: HTTP_REQUEST_TIMEOUT,
-    auth: {
-      user: Settings.overleaf.history.user,
-      pass: Settings.overleaf.history.pass,
-      sendImmediately: true,
-    },
-  }
-
-  if (options.json != null) {
-    requestOptions.json = options.json
-  }
-
-  if (options.body != null) {
-    requestOptions.body = options.body
-  }
-
-  if (options.qs != null) {
-    requestOptions.qs = options.qs
-  }
-
-  return requestOptions
-}
-
 /**
  * @return {RequestInit}
  */
@@ -614,25 +587,54 @@ function getHistoryFetchOptions() {
   }
 }
 
-function _requestHistoryService(options, callback) {
-  const requestOptions = _requestOptions(options)
-  request(requestOptions, (error, res, body) => {
-    if (error) {
-      return callback(OError.tag(error))
+function _buildUrl(options) {
+  const url = new URL(`${Settings.overleaf.history.host}/${options.path}`)
+  if (options.qs) {
+    for (const [key, value] of Object.entries(options.qs)) {
+      url.searchParams.set(key, String(value))
     }
+  }
+  return url.href
+}
 
-    if (res.statusCode >= 200 && res.statusCode < 300) {
-      callback(null, body)
-    } else {
-      const { method, url, qs } = requestOptions
-      error = new OError(
-        // Keep the status code in the message. It is used by the ErrorRecorder.
-        `history store a non-success status code: ${res.statusCode}`,
-        { method, url, qs, statusCode: res.statusCode }
-      )
-      callback(error)
+function _requestHistoryService(options, callback) {
+  const url = _buildUrl(options)
+  const method = options.method || 'GET'
+  const fetchOptions = {
+    method,
+    ...getHistoryFetchOptions(),
+  }
+
+  if (options.json != null && options.json !== true) {
+    fetchOptions.json = options.json
+  }
+
+  if (options.body != null) {
+    fetchOptions.body = options.body
+  }
+
+  const useJson = options.json != null
+
+  fetchString(url, fetchOptions).then(
+    body => {
+      if (useJson && body) {
+        callback(null, JSON.parse(body))
+      } else {
+        callback(null, body)
+      }
+    },
+    err => {
+      if (err instanceof RequestFailedError) {
+        const error = new OError(
+          // Keep the status code in the message. It is used by the ErrorRecorder.
+          `history store a non-success status code: ${err.response.status}`,
+          { method, url, qs: options.qs, statusCode: err.response.status }
+        )
+        return callback(error)
+      }
+      callback(OError.tag(err))
     }
-  })
+  )
 }
 
 export const cloneProject = callbackify(_cloneProject)

services/project-history/package.json

@@ -36,8 +36,7 @@
     "minimist": "^1.2.8",
     "mongodb-legacy": "6.1.3",
     "overleaf-editor-core": "workspace:*",
-    "p-queue": "^8.1.0",
-    "request": "2.88.2"
+    "p-queue": "^8.1.0"
   },
   "devDependencies": {
     "@overleaf/migrations": "workspace:*",

services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js

@@ -27,17 +27,6 @@ describe('HistoryStoreManager', function () {
         },
       },
     }
-    this.latestChunkRequestArgs = sinon.match({
-      method: 'GET',
-      url: `${this.settings.overleaf.history.host}/projects/${this.historyId}/latest/history`,
-      json: true,
-      auth: {
-        user: this.settings.overleaf.history.user,
-        pass: this.settings.overleaf.history.pass,
-        sendImmediately: true,
-      },
-    })
-
     this.callback = sinon.stub()
 
     this.LocalFileWriter = {
@@ -53,12 +42,11 @@ describe('HistoryStoreManager', function () {
 
     this.FetchUtils = {
       fetchStream: sinon.stub(),
+      fetchString: sinon.stub(),
       fetchNothing: sinon.stub().resolves(),
       RequestFailedError,
     }
 
-    this.request = sinon.stub()
-
     this.logger = {
       debug: sinon.stub(),
       warn: sinon.stub(),
@@ -66,7 +54,6 @@ describe('HistoryStoreManager', function () {
 
     this.HistoryStoreManager = await esmock(MODULE_PATH, {
       '@overleaf/fetch-utils': this.FetchUtils,
-      request: this.request,
       '@overleaf/settings': this.settings,
       '../../../../app/js/LocalFileWriter.js': this.LocalFileWriter,
       '../../../../app/js/WebApiManager.js': this.WebApiManager,
@@ -77,7 +64,7 @@ describe('HistoryStoreManager', function () {
 
   describe('getMostRecentChunk', function () {
     describe('successfully', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.chunk = {
           chunk: {
             startVersion: 0,
@@ -89,13 +76,27 @@ describe('HistoryStoreManager', function () {
             },
           },
         }
-        this.request
-          .withArgs(this.latestChunkRequestArgs)
-          .yields(null, { statusCode: 200 }, this.chunk)
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.chunk))
         this.HistoryStoreManager.getMostRecentChunk(
           this.projectId,
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
+        )
+      })
+
+      it('should request latest history from the overleaf history service', function () {
+        expect(this.FetchUtils.fetchString).to.have.been.calledWithMatch(
+          `${this.settings.overleaf.history.host}/projects/${this.historyId}/latest/history`,
+          {
+            method: 'GET',
+            basicAuth: {
+              user: this.settings.overleaf.history.user,
+              password: this.settings.overleaf.history.pass,
+            },
+          }
         )
       })
 
@@ -107,7 +108,7 @@ describe('HistoryStoreManager', function () {
 
   describe('getMostRecentVersion', function () {
     describe('successfully', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.chunk = {
           chunk: {
             startVersion: 5,
@@ -123,13 +124,14 @@ describe('HistoryStoreManager', function () {
           },
         }
 
-        this.request
-          .withArgs(this.latestChunkRequestArgs)
-          .yields(null, { statusCode: 200 }, this.chunk)
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.chunk))
         this.HistoryStoreManager.getMostRecentVersion(
           this.projectId,
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
@@ -141,10 +143,23 @@ describe('HistoryStoreManager', function () {
           { v2Authors: ['5678'], timestamp: '2017-10-17T10:44:40.227Z' }
         )
       })
+
+      it('should request latest history from the overleaf history service', function () {
+        expect(this.FetchUtils.fetchString).to.have.been.calledWithMatch(
+          `${this.settings.overleaf.history.host}/projects/${this.historyId}/latest/history`,
+          {
+            method: 'GET',
+            basicAuth: {
+              user: this.settings.overleaf.history.user,
+              password: this.settings.overleaf.history.pass,
+            },
+          }
+        )
+      })
     })
 
     describe('out of order doc ops', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.chunk = {
           chunk: {
             startVersion: 5,
@@ -172,13 +187,14 @@ describe('HistoryStoreManager', function () {
           },
         }
 
-        this.request
-          .withArgs(this.latestChunkRequestArgs)
-          .yields(null, { statusCode: 200 }, this.chunk)
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.chunk))
         this.HistoryStoreManager.getMostRecentVersion(
           this.projectId,
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
@@ -204,7 +220,7 @@ describe('HistoryStoreManager', function () {
     })
 
     describe('out of order project structure versions', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.chunk = {
           chunk: {
             startVersion: 5,
@@ -222,13 +238,14 @@ describe('HistoryStoreManager', function () {
           },
         }
 
-        this.request
-          .withArgs(this.latestChunkRequestArgs)
-          .yields(null, { statusCode: 200 }, this.chunk)
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.chunk))
         this.HistoryStoreManager.getMostRecentVersion(
           this.projectId,
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
@@ -256,7 +273,7 @@ describe('HistoryStoreManager', function () {
     })
 
     describe('out of order project structure and doc versions', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.chunk = {
           chunk: {
             startVersion: 5,
@@ -313,13 +330,14 @@ describe('HistoryStoreManager', function () {
           },
         }
 
-        this.request
-          .withArgs(this.latestChunkRequestArgs)
-          .yields(null, { statusCode: 200 }, this.chunk)
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.chunk))
         this.HistoryStoreManager.getMostRecentVersion(
           this.projectId,
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
@@ -350,20 +368,21 @@ describe('HistoryStoreManager', function () {
     })
 
     describe('with an unexpected response', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.badChunk = {
           chunk: {
             foo: 123, // valid chunk should have startVersion property
             bar: 456,
           },
         }
-        this.request
-          .withArgs(this.latestChunkRequestArgs)
-          .yields(null, { statusCode: 200 }, this.badChunk)
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.badChunk))
         this.HistoryStoreManager.getMostRecentVersion(
           this.projectId,
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
@@ -613,28 +632,25 @@ describe('HistoryStoreManager', function () {
 
   describe('getProjectBlob', function () {
     describe('successfully', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.blobContent = 'test content'
-        this.blobHash = 'test hash'
+        this.blobHash = 'testhash'
 
-        this.request.yields(null, { statusCode: 200 }, this.blobContent)
+        this.FetchUtils.fetchString.resolves(this.blobContent)
         this.HistoryStoreManager.getProjectBlob(
           this.historyId,
           this.blobHash,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
       it('should get the blob from the overleaf history service', function () {
-        expect(this.request).to.have.been.calledWithMatch({
-          method: 'GET',
-          url: `${this.settings.overleaf.history.host}/projects/${this.historyId}/blobs/${this.blobHash}`,
-          auth: {
-            user: this.settings.overleaf.history.user,
-            pass: this.settings.overleaf.history.pass,
-            sendImmediately: true,
-          },
-        })
+        expect(this.FetchUtils.fetchString).to.have.been.calledWithMatch(
+          `${this.settings.overleaf.history.host}/projects/${this.historyId}/blobs/${this.blobHash}`
+        )
       })
 
       it('should call the callback with the blob', function () {
@@ -677,32 +693,27 @@ describe('HistoryStoreManager', function () {
 
   describe('initializeProject', function () {
     describe('successfully', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
         this.response_body = { projectId: this.historyId }
-        this.request.callsArgWith(
-          1,
-          null,
-          { statusCode: 200 },
-          this.response_body
-        )
+        this.FetchUtils.fetchString.resolves(JSON.stringify(this.response_body))
 
         this.HistoryStoreManager.initializeProject(
           this.historyId,
-          this.callback
+          (...args) => {
+            this.callback(...args)
+            done()
+          }
         )
       })
 
       it('should send the change to the history store', function () {
-        expect(this.request).to.have.been.calledWithMatch({
-          method: 'POST',
-          url: `${this.settings.overleaf.history.host}/projects`,
-          auth: {
-            user: this.settings.overleaf.history.user,
-            pass: this.settings.overleaf.history.pass,
-            sendImmediately: true,
-          },
-          json: { projectId: this.historyId },
-        })
+        expect(this.FetchUtils.fetchString).to.have.been.calledWithMatch(
+          `${this.settings.overleaf.history.host}/projects`,
+          {
+            method: 'POST',
+            json: { projectId: this.historyId },
+          }
+        )
       })
 
       it('should call the callback with the new overleaf id', function () {
@@ -713,15 +724,15 @@ describe('HistoryStoreManager', function () {
 
   describe('deleteProject', function () {
     beforeEach(function (done) {
-      this.request.yields(null, { statusCode: 204 }, '')
+      this.FetchUtils.fetchString.resolves('')
       this.HistoryStoreManager.deleteProject(this.historyId, done)
     })
 
     it('should ask the history store to delete the project', function () {
-      expect(this.request).to.have.been.calledWithMatch({
-        method: 'DELETE',
-        url: `${this.settings.overleaf.history.host}/projects/${this.historyId}`,
-      })
+      expect(this.FetchUtils.fetchString).to.have.been.calledWithMatch(
+        `${this.settings.overleaf.history.host}/projects/${this.historyId}`,
+        { method: 'DELETE' }
+      )
     })
   })
 })

services/project-history/test/unit/js/HttpController/HttpControllerTests.js

@@ -53,10 +53,8 @@ describe('HttpController', function () {
     }
     this.RetryManager = {}
     this.FlushManager = {}
-    this.request = {}
     this.pipeline = sinon.stub()
     this.HttpController = await esmock(MODULE_PATH, {
-      request: this.request,
       stream: { pipeline: this.pipeline },
       '../../../../app/js/UpdatesProcessor.js': this.UpdatesProcessor,
       '../../../../app/js/SummarizedUpdatesManager.js':

services/project-history/test/unit/js/RetryManager/RetryManagerTests.js

@@ -89,7 +89,6 @@ describe('RetryManager', function () {
         },
       },
     }
-    this.request = {}
     this.RetryManager = await esmock(MODULE_PATH, {
       '../../../../app/js/WebApiManager.js': this.WebApiManager,
       '../../../../app/js/RedisManager.js': this.RedisManager,
@@ -97,7 +96,6 @@ describe('RetryManager', function () {
       '../../../../app/js/SyncManager.js': this.SyncManager,
       '../../../../app/js/UpdatesProcessor.js': this.UpdatesProcessor,
       '@overleaf/settings': this.settings,
-      request: this.request,
     })
   })
 

yarn.lock

@@ -6886,7 +6886,6 @@ __metadata:
     nock: "npm:^13.5.3"
     overleaf-editor-core: "workspace:*"
     p-queue: "npm:^8.1.0"
-    request: "npm:2.88.2"
     sinon: "npm:~9.0.1"
     sinon-chai: "npm:^3.7.0"
     timekeeper: "npm:2.2.0"