Merge pull request #13780 from overleaf/em-fetch-utils-project-history

Use fetch-utils in project-history

GitOrigin-RevId: 96afc5f3961210baa7ad597645b725fe2d8d3529

package-lock.json

@@ -40605,6 +40605,7 @@
     "services/project-history": {
       "name": "@overleaf/project-history",
       "dependencies": {
+        "@overleaf/fetch-utils": "*",
         "@overleaf/logger": "*",
         "@overleaf/metrics": "*",
         "@overleaf/o-error": "*",
@@ -40626,7 +40627,6 @@
         "lodash": "^4.17.20",
         "mongo-uri": "^0.1.2",
         "mongodb": "^4.11.0",
-        "node-fetch": "^2.6.0",
         "overleaf-editor-core": "*",
         "redis": "~0.10.1",
         "request": "^2.88.2",
@@ -49797,6 +49797,7 @@
     "@overleaf/project-history": {
       "version": "file:services/project-history",
       "requires": {
+        "@overleaf/fetch-utils": "*",
         "@overleaf/logger": "*",
         "@overleaf/metrics": "*",
         "@overleaf/o-error": "*",
@@ -49823,7 +49824,6 @@
         "mongodb": "^4.11.0",
         "multer": "^1.4.2",
         "nock": "^12.0.3",
-        "node-fetch": "^2.6.0",
         "overleaf-editor-core": "*",
         "redis": "~0.10.1",
         "request": "^2.88.2",

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

@@ -7,11 +7,15 @@ import BPromise from 'bluebird'
 import { URL } from 'url'
 import OError from '@overleaf/o-error'
 import Settings from '@overleaf/settings'
+import {
+  fetchStream,
+  fetchNothing,
+  RequestFailedError,
+} from '@overleaf/fetch-utils'
 import * as Versions from './Versions.js'
 import * as Errors from './Errors.js'
 import * as LocalFileWriter from './LocalFileWriter.js'
 import * as HashManager from './HashManager.js'
-import fetch from 'node-fetch'
 
 const HTTP_REQUEST_TIMEOUT = 300 * 1000 // 5 minutes
 
@@ -174,17 +178,9 @@ export function getProjectBlobStream(historyId, blobHash, callback) {
     { historyId, blobHash },
     'getting blob stream from history service'
   )
-  fetch(url, getHistoryFetchOptions())
-    .then(res => {
-      if (!res.ok) {
-        const err = new OError(
-          `history store a non-success status code: ${res.status}`
-        )
-        err.statusCode = res.status
-        logger.warn({ err, url }, 'cannot get project blob')
-        return callback(err)
-      }
-      callback(null, res.body)
+  fetchStream(url, getHistoryFetchOptions())
+    .then(stream => {
+      callback(null, stream)
     })
     .catch(err => callback(OError.tag(err)))
 }
@@ -258,20 +254,22 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
     }
     const fileId = urlMatch[2]
     const filestoreURL = `${Settings.apis.filestore.url}/project/${projectId}/file/${fileId}`
-    fetch(filestoreURL, { signal: AbortSignal.timeout(HTTP_REQUEST_TIMEOUT) })
-      .then(response => {
-        const statusCode = response.status
-        if (statusCode >= 200 && statusCode < 300) {
-          LocalFileWriter.bufferOnDisk(
-            response.body,
-            filestoreURL,
-            `project-${projectId}-file-${fileId}`,
-            (fsPath, cb) => {
-              _createBlob(historyId, fsPath, cb)
-            },
-            callback
-          )
-        } else if (statusCode === 404) {
+    fetchStream(filestoreURL, {
+      signal: AbortSignal.timeout(HTTP_REQUEST_TIMEOUT),
+    })
+      .then(stream => {
+        LocalFileWriter.bufferOnDisk(
+          stream,
+          filestoreURL,
+          `project-${projectId}-file-${fileId}`,
+          (fsPath, cb) => {
+            _createBlob(historyId, fsPath, cb)
+          },
+          callback
+        )
+      })
+      .catch(err => {
+        if (err instanceof RequestFailedError && err.response.status === 404) {
           logger.warn(
             { projectId, historyId, filestoreURL },
             'File contents not found in filestore. Storing in history as an empty file'
@@ -288,16 +286,9 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
           )
           emptyStream.push(null) // send an EOF signal
         } else {
-          const error = new OError(
-            `bad response from filestore: ${statusCode}`,
-            { filestoreURL, statusCode }
-          )
-          callback(error)
+          callback(OError.tag(err, 'error from filestore', { filestoreURL }))
         }
       })
-      .catch(err =>
-        callback(OError.tag(err, 'error from filestore', { filestoreURL }))
-      )
   } else {
     const error = new OError('invalid update for blob creation')
     callback(error)
@@ -318,19 +309,17 @@ function _createBlob(historyId, fsPath, _callback) {
       'sending blob to history service'
     )
     const url = `${Settings.overleaf.history.host}/projects/${historyId}/blobs/${hash}`
-    fetch(url, { method: 'PUT', body: outStream, ...getHistoryFetchOptions() })
+    fetchNothing(url, {
+      method: 'PUT',
+      body: outStream,
+      ...getHistoryFetchOptions(),
+    })
       .then(res => {
-        if (!res.ok) {
-          const err = new OError(
-            `history store a non-success status code: ${res.status}`
-          )
-          err.statusCode = res.status
-          logger.warn({ err, url }, 'cannot create project blob')
-          return callback(err)
-        }
         callback(null, hash)
       })
-      .catch(err => callback(OError.tag(err)))
+      .catch(err => {
+        callback(OError.tag(err))
+      })
   })
 }
 
@@ -413,12 +402,9 @@ function _requestOptions(options) {
 function getHistoryFetchOptions() {
   return {
     signal: AbortSignal.timeout(HTTP_REQUEST_TIMEOUT),
-    headers: {
-      Authorization:
-        'Basic ' +
-        Buffer.from(
-          `${Settings.overleaf.history.user}:${Settings.overleaf.history.pass}`
-        ).toString('base64'),
+    basicAuth: {
+      user: Settings.overleaf.history.user,
+      password: Settings.overleaf.history.pass,
     },
   }
 }

services/project-history/package.json

@@ -17,6 +17,7 @@
     "lint:fix": "eslint --fix ."
   },
   "dependencies": {
+    "@overleaf/fetch-utils": "*",
     "@overleaf/logger": "*",
     "@overleaf/metrics": "*",
     "@overleaf/o-error": "*",
@@ -38,7 +39,6 @@
     "lodash": "^4.17.20",
     "mongo-uri": "^0.1.2",
     "mongodb": "^4.11.0",
-    "node-fetch": "^2.6.0",
     "overleaf-editor-core": "*",
     "redis": "~0.10.1",
     "request": "^2.88.2",

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

@@ -2,6 +2,7 @@ import sinon from 'sinon'
 import { expect } from 'chai'
 import { strict as esmock } from 'esmock'
 import EventEmitter from 'events'
+import { RequestFailedError } from '@overleaf/fetch-utils'
 import * as Errors from '../../../../app/js/Errors.js'
 
 const MODULE_PATH = '../../../../app/js/HistoryStoreManager.js'
@@ -36,21 +37,29 @@ describe('HistoryStoreManager', function () {
     })
 
     this.callback = sinon.stub()
+
     this.LocalFileWriter = {
       bufferOnDisk: sinon.stub(),
     }
+
     this.WebApiManager = {
       getHistoryId: sinon.stub(),
     }
     this.WebApiManager.getHistoryId
       .withArgs(this.projectId)
       .yields(null, this.historyId)
+
+    this.FetchUtils = {
+      fetchStream: sinon.stub(),
+      fetchNothing: sinon.stub().resolves(),
+      RequestFailedError,
+    }
+
     this.request = sinon.stub()
     this.request.get = sinon.stub()
-    this.fetch = sinon.stub().resolves()
 
     this.HistoryStoreManager = await esmock(MODULE_PATH, {
-      'node-fetch': this.fetch,
+      '@overleaf/fetch-utils': this.FetchUtils,
       request: this.request,
       '@overleaf/settings': this.settings,
       '../../../../app/js/LocalFileWriter.js': this.LocalFileWriter,
@@ -366,10 +375,7 @@ describe('HistoryStoreManager', function () {
       this.fileStream = {}
       this.hash = 'random-hash'
       this.LocalFileWriter.bufferOnDisk.callsArgWith(4, null, this.hash)
-      this.fetch.resolves({
-        status: 200,
-        body: this.fileStream,
-      })
+      this.FetchUtils.fetchStream.resolves(this.fileStream)
     })
 
     describe('for a file update with any filestore location', function () {
@@ -394,7 +400,7 @@ describe('HistoryStoreManager', function () {
       })
 
       it('should request the file from the filestore in settings', function () {
-        expect(this.fetch).to.have.been.calledWithMatch(
+        expect(this.FetchUtils.fetchStream).to.have.been.calledWithMatch(
           `${this.settings.apis.filestore.url}/project/${this.projectId}/file/${this.file_id}`
         )
       })
@@ -467,11 +473,7 @@ describe('HistoryStoreManager', function () {
         this.historyResponse = new EventEmitter()
         this.blobHash = 'test hash'
 
-        this.fetch.resolves({
-          status: 200,
-          ok: true,
-          body: this.historyResponse,
-        })
+        this.FetchUtils.fetchStream.resolves(this.historyResponse)
         this.HistoryStoreManager.getProjectBlobStream(
           this.historyId,
           this.blobHash,
@@ -486,7 +488,7 @@ describe('HistoryStoreManager', function () {
       })
 
       it('should get the blob from the overleaf history service', function () {
-        expect(this.fetch).to.have.been.calledWithMatch(
+        expect(this.FetchUtils.fetchStream).to.have.been.calledWithMatch(
           `${this.settings.overleaf.history.host}/projects/${this.historyId}/blobs/${this.blobHash}`
         )
       })