Explorar o código

[monorepo] remove PII and variables from error messages (#31508)

* [monorepo] remove PII and variables from error messages

Exclusions:
- scripts
- tests
- fuzzing
- SplitTestManager (messages are sent to admin frontend)
- Group setup (we may want an error per unique tuple)
- sharejs (unused types; text type errors are shadowed already)
- history-v1 error messages that are used by the ErrorRecorder
- errors that flag issues with configuration/call signatures

I've used these search terms for finding unwanted error messages:
- new Error(`
- new Error\(\n\s+` (regex search)
- new OError(`
- new OError\(\n\s+` (regex search)

* [web] throw NotFoundError from ProjectLocator

* [github-sync] fix OError.tag call in script

Co-authored-by: Jessica Lawshe <jessica.lawshe@overleaf.com>

* [templates] revert changes to test client

---------

Co-authored-by: Jessica Lawshe <jessica.lawshe@overleaf.com>
GitOrigin-RevId: 736857a4fc5d9bfb0f8cb03e0f004eda87e5a220
Jakob Ackermann hai 5 meses
pai
achega
7c70b749d4
Modificáronse 32 ficheiros con 210 adicións e 163 borrados
  1. 3 0
      package-lock.json
  2. 4 3
      services/clsi/app.js
  3. 5 1
      services/docstore/app/js/HealthChecker.js
  4. 1 1
      services/document-updater/app/js/RedisManager.js
  5. 1 1
      services/history-v1/storage/lib/backupDeletion.mjs
  6. 1 1
      services/history-v1/storage/lib/chunk_store/redis.js
  7. 12 8
      services/project-history/app/js/DocumentUpdaterManager.js
  8. 9 3
      services/project-history/app/js/HealthChecker.js
  9. 1 0
      services/project-history/app/js/HistoryStoreManager.js
  10. 2 2
      services/project-history/test/unit/js/DocumentUpdaterManager/DocumentUpdaterManagerTests.js
  11. 8 11
      services/web/app/src/Features/Compile/ClsiManager.mjs
  12. 40 41
      services/web/app/src/Features/Docstore/DocstoreManager.mjs
  13. 10 10
      services/web/app/src/Features/Exports/ExportsHandler.mjs
  14. 2 1
      services/web/app/src/Features/LinkedFiles/ProjectFileAgent.mjs
  15. 2 1
      services/web/app/src/Features/Project/ProjectHelper.mjs
  16. 10 6
      services/web/app/src/Features/Project/ProjectLocator.mjs
  17. 4 3
      services/web/app/src/Features/Project/ProjectOptionsHandler.mjs
  18. 2 1
      services/web/app/src/Features/Referal/ReferalFeatures.mjs
  19. 3 4
      services/web/app/src/Features/Subscription/RecurlyWrapper.mjs
  20. 6 5
      services/web/app/src/Features/Subscription/V1SubscriptionManager.mjs
  21. 2 1
      services/web/app/src/Features/Templates/TemplatesManager.mjs
  22. 5 4
      services/web/app/src/Features/TokenGenerator/TokenGenerator.mjs
  23. 3 2
      services/web/app/src/Features/Uploads/FileSystemImportManager.mjs
  24. 6 6
      services/web/app/src/Features/User/SAMLIdentityManager.mjs
  25. 6 3
      services/web/app/src/Features/V1/V1Api.mjs
  26. 6 6
      services/web/app/src/Features/V1/V1Handler.mjs
  27. 3 1
      services/web/app/src/infrastructure/FileWriter.mjs
  28. 1 1
      services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.mjs
  29. 1 1
      services/web/test/smoke/src/support/Csrf.mjs
  30. 9 9
      services/web/test/unit/src/Docstore/DocstoreManager.test.mjs
  31. 18 17
      services/web/test/unit/src/Project/ProjectOptionsHandler.test.mjs
  32. 24 9
      services/web/test/unit/src/User/SAMLIdentityManager.test.mjs

+ 3 - 0
package-lock.json

@@ -589,6 +589,9 @@
       "name": "@overleaf/piece-table",
       "version": "1.0.0",
       "license": "Proprietary",
+      "dependencies": {
+        "@overleaf/o-error": "*"
+      },
       "devDependencies": {
         "chai": "^4.3.6",
         "mocha": "^11.1.0",

+ 4 - 3
services/clsi/app.js

@@ -25,6 +25,7 @@ import ForbidSymlinks from './app/js/StaticServerForbidSymlinks.js'
 
 import net from 'node:net'
 import os from 'node:os'
+import OError from '@overleaf/o-error'
 logger.initialize('clsi')
 logger.logger.serializers.clsiRequest = LoggerSerializers.clsiRequest
 
@@ -70,7 +71,7 @@ app.param('build_id', function (req, res, next, buildId) {
   if (buildId?.match(OutputCacheManager.BUILD_REGEX)) {
     next()
   } else {
-    next(new Error(`invalid build id ${buildId}`))
+    next(new OError('invalid build id', { buildId }))
   }
 })
 
@@ -78,7 +79,7 @@ app.param('contentId', function (req, res, next, contentId) {
   if (contentId?.match(OutputCacheManager.CONTENT_REGEX)) {
     next()
   } else {
-    next(new Error(`invalid content id ${contentId}`))
+    next(new OError('invalid content id', { contentId }))
   }
 })
 
@@ -86,7 +87,7 @@ app.param('hash', function (req, res, next, hash) {
   if (hash?.match(ContentCacheManager.HASH_REGEX)) {
     next()
   } else {
-    next(new Error(`invalid hash ${hash}`))
+    next(new OError('invalid hash', { hash }))
   }
 })
 

+ 5 - 1
services/docstore/app/js/HealthChecker.js

@@ -4,6 +4,7 @@ import crypto from 'node:crypto'
 import settings from '@overleaf/settings'
 import logger from '@overleaf/logger'
 import { fetchNothing, fetchJson } from '@overleaf/fetch-utils'
+import OError from '@overleaf/o-error'
 
 const { db, ObjectId } = mongodb
 
@@ -30,7 +31,10 @@ async function check() {
     await db.docs.deleteOne({ _id: docId, project_id: projectId })
   }
   if (!_.isEqual(body?.lines, lines)) {
-    throw new Error(`health check lines not equal ${body.lines} != ${lines}`)
+    throw new OError('health check lines not equal', {
+      got: body.lines,
+      want: lines,
+    })
   }
 }
 

+ 1 - 1
services/document-updater/app/js/RedisManager.js

@@ -373,7 +373,7 @@ const RedisManager = {
     const shareJSTextOT = Array.isArray(docLines)
     const currentVersion = await RedisManager.getDocVersion(docId)
     if (currentVersion + appliedOps.length !== newVersion) {
-      throw new OError(`Version mismatch. '${docId}' is corrupted.`, {
+      throw new OError('Version mismatch. doc is corrupted', {
         docId,
         currentVersion,
         newVersion,

+ 1 - 1
services/history-v1/storage/lib/backupDeletion.mjs

@@ -84,7 +84,7 @@ export async function healthCheck() {
 
   for (const historyId of HEALTH_CHECK_PROJECTS) {
     if (!(await projectHasLatestChunk(historyId))) {
-      throw new Error(`project has no history: ${historyId}`)
+      throw new OError('project has no history', { historyId })
     }
   }
 }

+ 1 - 1
services/history-v1/storage/lib/chunk_store/redis.js

@@ -212,7 +212,7 @@ async function queueChanges(
         baseVersion,
       })
     } else {
-      throw new Error(`unexpected result queuing changes: ${status}`)
+      throw new OError('unexpected result queuing changes', { status })
     }
   } catch (err) {
     if (err instanceof BaseVersionConflictError) {

+ 12 - 8
services/project-history/app/js/DocumentUpdaterManager.js

@@ -37,10 +37,12 @@ export function getDocument(projectId, docId, callback) {
       )
       return callback(null, body.lines.join('\n'), body.version)
     } else {
-      error = new OError(
-        `doc updater returned a non-success status code: ${res.statusCode}`,
-        { project_id: projectId, doc_id: docId, url }
-      )
+      error = new OError('doc updater returned a non-success status code', {
+        project_id: projectId,
+        doc_id: docId,
+        url,
+        statusCode: res.statusCode,
+      })
       return callback(error)
     }
   })
@@ -69,10 +71,12 @@ export function setDocument(projectId, docId, content, userId, callback) {
       if (res.statusCode >= 200 && res.statusCode < 300) {
         return callback(null)
       } else {
-        error = new OError(
-          `doc updater returned a non-success status code: ${res.statusCode}`,
-          { project_id: projectId, doc_id: docId, url }
-        )
+        error = new OError('doc updater returned a non-success status code', {
+          project_id: projectId,
+          doc_id: docId,
+          url,
+          statusCode: res.statusCode,
+        })
         return callback(error)
       }
     }

+ 9 - 3
services/project-history/app/js/HealthChecker.js

@@ -31,7 +31,9 @@ export function check(callback) {
             })
             return cb(err)
           } else if ((res != null ? res.statusCode : undefined) !== 200) {
-            return cb(new Error(`status code not 200, it's ${res.statusCode}`))
+            return cb(
+              new OError('status code not 200', { statusCode: res.statusCode })
+            )
           } else {
             return cb()
           }
@@ -47,7 +49,9 @@ export function check(callback) {
             })
             return cb(err)
           } else if ((res != null ? res.statusCode : undefined) !== 204) {
-            return cb(new Error(`status code not 204, it's ${res.statusCode}`))
+            return cb(
+              new OError('status code not 204', { statusCode: res.statusCode })
+            )
           } else {
             return cb()
           }
@@ -63,7 +67,9 @@ export function check(callback) {
             })
             return cb(err)
           } else if ((res != null ? res.statusCode : undefined) !== 200) {
-            return cb(new Error(`status code not 200, it's ${res.statusCode}`))
+            return cb(
+              new OError('status code not 200', { statusCode: res.statusCode })
+            )
           } else {
             return cb()
           }

+ 1 - 0
services/project-history/app/js/HistoryStoreManager.js

@@ -614,6 +614,7 @@ function _requestHistoryService(options, callback) {
     } 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 }
       )

+ 2 - 2
services/project-history/test/unit/js/DocumentUpdaterManager/DocumentUpdaterManagerTests.js

@@ -93,7 +93,7 @@ describe('DocumentUpdaterManager', function () {
           .calledWith(
             sinon.match.has(
               'message',
-              'doc updater returned a non-success status code: 500'
+              'doc updater returned a non-success status code'
             )
           )
           .should.equal(true)
@@ -174,7 +174,7 @@ describe('DocumentUpdaterManager', function () {
           .calledWith(
             sinon.match.has(
               'message',
-              'doc updater returned a non-success status code: 500'
+              'doc updater returned a non-success status code'
             )
           )
           .should.equal(true)

+ 8 - 11
services/web/app/src/Features/Compile/ClsiManager.mjs

@@ -609,17 +609,14 @@ async function _postToClsi(
       } 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,
-          }
-        )
+        throw new OError('CLSI returned non-success code', {
+          projectId,
+          userId,
+          compileOptions: req.compile.options,
+          rootResourcePath: req.compile.rootResourcePath,
+          clsiResponse: err.body,
+          statusCode: err.response.status,
+        })
       }
     } else {
       throw new OError(

+ 40 - 41
services/web/app/src/Features/Docstore/DocstoreManager.mjs

@@ -53,13 +53,11 @@ async function deleteDoc(projectId, docId, name, deletedAt) {
           },
         })
       }
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        {
-          projectId,
-          docId,
-        }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        docId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -75,10 +73,10 @@ async function getAllDocs(projectId) {
     return await fetchJson(url, { signal: AbortSignal.timeout(TIMEOUT) })
   } catch (error) {
     if (error instanceof RequestFailedError) {
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        { projectId }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -96,10 +94,10 @@ async function getAllDeletedDocs(projectId) {
     return await fetchJson(url, { signal: AbortSignal.timeout(TIMEOUT) })
   } catch (error) {
     if (error instanceof RequestFailedError) {
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        { projectId }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        status: error.response.status,
+      })
     }
     throw OError.tag(error, 'could not get deleted docs from docstore')
   }
@@ -131,10 +129,10 @@ async function getAllRanges(projectId) {
     return await fetchJson(url, { signal: AbortSignal.timeout(TIMEOUT) })
   } catch (error) {
     if (error instanceof RequestFailedError) {
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        { projectId }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -191,13 +189,11 @@ async function getDoc(projectId, docId, options = {}) {
           },
         })
       }
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        {
-          projectId,
-          docId,
-        }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        docId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -223,10 +219,11 @@ async function isDocDeleted(projectId, docId) {
           info: { projectId, docId },
         })
       }
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        { projectId, docId }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        docId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -258,10 +255,11 @@ async function updateDoc(projectId, docId, lines, version, ranges) {
     return { modified: result.modified, rev: result.rev }
   } catch (error) {
     if (error instanceof RequestFailedError) {
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        { projectId, docId }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        docId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -280,10 +278,10 @@ async function projectHasRanges(projectId) {
     return body.projectHasRanges
   } catch (error) {
     if (error instanceof RequestFailedError) {
-      throw new OError(
-        `docstore api responded with non-success code: ${error.response.status}`,
-        { projectId }
-      )
+      throw new OError('docstore api responded with non-success code', {
+        projectId,
+        status: error.response.status,
+      })
     }
     throw error
   }
@@ -332,9 +330,10 @@ async function _operateOnProject(projectId, method) {
     })
   } catch (err) {
     if (err instanceof RequestFailedError) {
-      const error = new Error(
-        `docstore api responded with non-success code: ${err.response.status}`
-      )
+      const error = new OError('docstore api responded with non-success code', {
+        projectId,
+        status: err.response.status,
+      })
       logger.warn(
         { err: error, projectId },
         `error calling ${method} project in docstore`

+ 10 - 10
services/web/app/src/Features/Exports/ExportsHandler.mjs

@@ -213,8 +213,8 @@ export default ExportsHandler = {
           return callback(null, body.version)
         } else {
           err = new OError(
-            `project history version returned a failure status code: ${res.statusCode}`,
-            { project_id: projectId }
+            'project history version returned a failure status code',
+            { project_id: projectId, statusCode: res.statusCode }
           )
           return callback(err)
         }
@@ -241,10 +241,10 @@ export default ExportsHandler = {
         } else if (res.statusCode >= 200 && res.statusCode < 300) {
           return callback(null, body)
         } else {
-          err = new OError(
-            `v1 export returned a failure status code: ${res.statusCode}`,
-            { export: exportId }
-          )
+          err = new OError('v1 export returned a failure status code', {
+            export: exportId,
+            statusCode: res.statusCode,
+          })
           return callback(err)
         }
       }
@@ -270,10 +270,10 @@ export default ExportsHandler = {
         } else if (res.statusCode >= 200 && res.statusCode < 300) {
           return callback(null, body)
         } else {
-          err = new OError(
-            `v1 export returned a failure status code: ${res.statusCode}`,
-            { export: exportId }
-          )
+          err = new OError('v1 export returned a failure status code', {
+            export: exportId,
+            statusCode: res.statusCode,
+          })
           return callback(err)
         }
       }

+ 2 - 1
services/web/app/src/Features/LinkedFiles/ProjectFileAgent.mjs

@@ -19,6 +19,7 @@ import LinkedFilesHandler from './LinkedFilesHandler.mjs'
 import LinkedFilesErrors from './LinkedFilesErrors.mjs'
 import { promisify } from '@overleaf/promise-utils'
 import HistoryManager from '../History/HistoryManager.mjs'
+import Errors from '../Errors/Errors.js'
 
 const {
   BadDataError,
@@ -197,7 +198,7 @@ export default ProjectFileAgent = {
               },
               function (err, entity, type) {
                 if (err != null) {
-                  if (/^not found.*/.test(err.message)) {
+                  if (err instanceof Errors.NotFoundError) {
                     err = new SourceFileNotFoundError()
                   }
                   return callback(err)

+ 2 - 1
services/web/app/src/Features/Project/ProjectHelper.mjs

@@ -2,6 +2,7 @@ import mongodb from 'mongodb-legacy'
 
 import _ from 'lodash'
 import Settings from '@overleaf/settings'
+import OError from '@overleaf/o-error'
 
 const { ObjectId } = mongodb
 
@@ -90,7 +91,7 @@ function ensureNameIsUnique(nameList, name, suffixes, maxLength) {
   if (uniqueName != null) {
     return uniqueName
   } else {
-    throw new Error(`Failed to generate a unique name for: ${name}`)
+    throw new OError('Failed to generate a unique name', { name })
   }
 }
 

+ 10 - 6
services/web/app/src/Features/Project/ProjectLocator.mjs

@@ -167,9 +167,11 @@ async function _findElementByPathWithProject(
       }
     }
     if (!found) {
-      throw new Error(
-        `not found project: ${project._id} search path: ${needlePath}, folder ${foldersList[level]} could not be found`
-      )
+      throw new Errors.NotFoundError('parent folder not found in project', {
+        projectId: project._id,
+        needlePath,
+        needleFolderName,
+      })
     }
   }
 
@@ -202,9 +204,11 @@ async function _findElementByPathWithProject(
     if (result != null) {
       return { element: result, type, folder }
     }
-    throw new Error(
-      `not found project: ${project._id} search path: ${needlePath}, entity ${entityName} could not be found`
-    )
+    throw new Errors.NotFoundError('element not found in project', {
+      projectId: project._id,
+      needlePath,
+      entityName,
+    })
   }
 
   if (project == null) {

+ 4 - 3
services/web/app/src/Features/Project/ProjectOptionsHandler.mjs

@@ -4,6 +4,7 @@ import { callbackify } from 'node:util'
 import { db, ObjectId } from '../../infrastructure/mongodb.mjs'
 import Errors from '../Errors/Errors.js'
 import mongodb from 'mongodb-legacy'
+import OError from '@overleaf/o-error'
 const safeCompilers = ['xelatex', 'pdflatex', 'latex', 'lualatex']
 
 const { ReturnDocument } = mongodb
@@ -16,7 +17,7 @@ const ProjectOptionsHandler = {
   normalizeCompiler(compiler) {
     compiler = compiler.toLowerCase()
     if (!safeCompilers.includes(compiler)) {
-      throw new Error(`invalid compiler: ${compiler}`)
+      throw new OError('invalid compiler', { compiler })
     }
     return compiler
   },
@@ -44,7 +45,7 @@ const ProjectOptionsHandler = {
       allowed => imageName === allowed.imageName
     )
     if (!isAllowed) {
-      throw new Error(`invalid imageName: ${imageName}`)
+      throw new OError('invalid imageName', { imageName })
     }
     return settings.imageRoot + '/' + imageName
   },
@@ -67,7 +68,7 @@ const ProjectOptionsHandler = {
       language => language.code === languageCode
     )
     if (languageCode && !language) {
-      throw new Error(`invalid languageCode: ${languageCode}`)
+      throw new OError('invalid languageCode', { languageCode })
     }
     const conditions = { _id: projectId }
     const update = { spellCheckLanguage: languageCode }

+ 2 - 1
services/web/app/src/Features/Referal/ReferalFeatures.mjs

@@ -2,6 +2,7 @@ import _ from 'lodash'
 import { callbackify } from 'node:util'
 import { User } from '../../models/User.mjs'
 import Settings from '@overleaf/settings'
+import OError from '@overleaf/o-error'
 
 const ReferalFeatures = {
   async getBonusFeatures(userId) {
@@ -9,7 +10,7 @@ const ReferalFeatures = {
     const user = await User.findOne(query, { refered_user_count: 1 }).exec()
 
     if (user == null) {
-      throw new Error(`user not found ${userId} for assignBonus`)
+      throw new OError('user not found for assignBonus', { userId })
     }
 
     if (user.refered_user_count != null && user.refered_user_count > 0) {

+ 3 - 4
services/web/app/src/Features/Subscription/RecurlyWrapper.mjs

@@ -446,10 +446,9 @@ const promises = {
           },
           'error returned from recurly'
         )
-        throw new OError(
-          `Recurly API returned with status code: ${error.response.status}`,
-          { statusCode: error.response.status }
-        )
+        throw new OError('Recurly API returned with status code', {
+          statusCode: error.response.status,
+        })
       } else {
         throw error
       }

+ 6 - 5
services/web/app/src/Features/Subscription/V1SubscriptionManager.mjs

@@ -3,6 +3,7 @@ import request from 'requestretry'
 import settings from '@overleaf/settings'
 import { V1ConnectionError, NotFoundError } from '../Errors/Errors.js'
 import { promisify } from '@overleaf/promise-utils'
+import OError from '@overleaf/o-error'
 
 const V1SubscriptionManager = {
   cancelV1Subscription(userId, callback) {
@@ -104,11 +105,11 @@ const V1SubscriptionManager = {
             return callback(new NotFoundError(`v1 user not found: ${userId}`))
           } else {
             return callback(
-              new Error(
-                `non-success code from v1: ${response.statusCode} ${
-                  options.method
-                } ${options.url(v1Id)}`
-              )
+              new OError('non-success code from v1', {
+                url: options.url(v1Id),
+                method: options.method,
+                statusCode: response.statusCode,
+              })
             )
           }
         }

+ 2 - 1
services/web/app/src/Features/Templates/TemplatesManager.mjs

@@ -16,6 +16,7 @@ import Errors from '../Errors/Errors.js'
 import { pipeline } from 'node:stream/promises'
 import ClsiCacheManager from '../Compile/ClsiCacheManager.mjs'
 import Path from 'node:path'
+import OError from '@overleaf/o-error'
 
 const { promises: ProjectRootDocManager } = ProjectRootDocManagerModule
 const { promises: ProjectOptionsHandler } = ProjectOptionsHandlerModule
@@ -64,7 +65,7 @@ const TemplatesManager = {
           { uri: zipUrl, statusCode: zipReq.response.status },
           'non-success code getting zip from template API'
         )
-        throw new Error(`get zip failed: ${zipReq.response.status}`)
+        throw new OError('get zip failed', { status: zipReq.response.status })
       }
       const { fileEntries, docEntries, project } =
         await ProjectUploadManager.promises.createProjectFromZipArchiveWithName(

+ 5 - 4
services/web/app/src/Features/TokenGenerator/TokenGenerator.mjs

@@ -2,6 +2,7 @@ import crypto from 'node:crypto'
 import V1Api from '../V1/V1Api.mjs'
 import Features from '../../infrastructure/Features.mjs'
 import { callbackify } from 'node:util'
+import OError from '@overleaf/o-error'
 
 // (From Overleaf `random_token.rb`)
 //   Letters (not numbers! see generate_token) used in tokens. They're all
@@ -59,13 +60,13 @@ async function generateUniqueReadOnlyToken() {
     })
 
     if (response.statusCode !== 200) {
-      throw new Error(
-        `non-200 response from v1 read-token-exists api: ${response.statusCode}`
-      )
+      throw new OError('non-200 response from v1 read-token-exists api', {
+        statusCode: response.statusCode,
+      })
     }
 
     if (body.exists === true) {
-      throw new Error(`token already exists in v1: ${token}`)
+      throw new OError('token already exists in v1', { token })
     }
 
     return token

+ 3 - 2
services/web/app/src/Features/Uploads/FileSystemImportManager.mjs

@@ -6,6 +6,7 @@ import Errors from '../Errors/Errors.js'
 import FileTypeManager from './FileTypeManager.mjs'
 import SafePath from '../Project/SafePath.mjs'
 import logger from '@overleaf/logger'
+import OError from '@overleaf/o-error'
 
 export default {
   addEntity: callbackify(addEntity),
@@ -183,7 +184,7 @@ async function _isSafeOnFileSystem(path) {
 async function importFile(fsPath, projectPath) {
   const stat = await fs.promises.lstat(fsPath)
   if (!stat.isFile()) {
-    throw new Error(`can't import ${fsPath}: not a regular file`)
+    throw new OError("can't import file: not a regular file", { fsPath })
   }
   _validateProjectPath(projectPath)
   const filename = Path.basename(projectPath)
@@ -206,7 +207,7 @@ async function importFile(fsPath, projectPath) {
 async function importDir(dirPath) {
   const stat = await fs.promises.lstat(dirPath)
   if (!stat.isDirectory()) {
-    throw new Error(`can't import ${dirPath}: not a directory`)
+    throw new OError("can't import dir: not a directory", { dirPath })
   }
   const entries = []
   for await (const filePath of _walkDir(dirPath)) {

+ 6 - 6
services/web/app/src/Features/User/SAMLIdentityManager.mjs

@@ -207,9 +207,11 @@ function _sendUnlinkedEmail(primaryEmail, providerName, institutionEmail) {
 
 async function getUser(providerId, externalUserId, userIdAttribute) {
   if (!providerId || !externalUserId || !userIdAttribute) {
-    throw new Error(
-      `invalid arguments: providerId: ${providerId}, externalUserId: ${externalUserId}, userIdAttribute: ${userIdAttribute}`
-    )
+    throw new OError('invalid arguments', {
+      providerId,
+      externalUserId,
+      userIdAttribute,
+    })
   }
   const user = await User.findOne({
     samlIdentifiers: {
@@ -252,9 +254,7 @@ async function linkAccounts(userId, samlData, auditLog) {
   } = samlData
 
   if (!externalUserId || !institutionEmail || !providerId || !userIdAttribute) {
-    throw new Error(
-      `missing data when linking institution SSO: ${JSON.stringify(samlData)}`
-    )
+    throw new OError('missing data when linking institution SSO', { samlData })
   }
 
   await _addIdentifier(

+ 6 - 3
services/web/app/src/Features/V1/V1Api.mjs

@@ -12,6 +12,7 @@ import request from 'request'
 import settings from '@overleaf/settings'
 import Errors from '../Errors/Errors.js'
 import { promisifyMultiResult } from '@overleaf/promise-utils'
+import OError from '@overleaf/o-error'
 
 // TODO: check what happens when these settings aren't defined
 const DEFAULT_V1_PARAMS = {
@@ -89,9 +90,11 @@ const V1Api = {
       error.statusCode = response.statusCode
       return callback(error)
     } else {
-      error = new Error(
-        `overleaf v1 returned non-success code: ${response?.statusCode} ${options.method} ${options.uri}`
-      )
+      error = new OError('overleaf v1 returned non-success code', {
+        status: response?.statusCode,
+        method: options.method,
+        url: options.uri,
+      })
       error.statusCode = response?.statusCode
       return callback(error)
     }

+ 6 - 6
services/web/app/src/Features/V1/V1Handler.mjs

@@ -45,9 +45,9 @@ export default V1Handler = {
           )
           return callback(null, isValid, userProfile)
         } else {
-          err = new Error(
-            `Unexpected status from v1 login api: ${response.statusCode}`
-          )
+          err = new OError('Unexpected status from v1 login api', {
+            status: response.statusCode,
+          })
           return callback(err)
         }
       }
@@ -79,9 +79,9 @@ export default V1Handler = {
           )
           return callback(null, true)
         } else {
-          err = new Error(
-            `Unexpected status from v1 password reset api: ${response.statusCode}`
-          )
+          err = new OError('Unexpected status from v1 password reset api', {
+            status: response.statusCode,
+          })
           return callback(err, false)
         }
       }

+ 3 - 1
services/web/app/src/infrastructure/FileWriter.mjs

@@ -172,7 +172,9 @@ const FileWriter = {
       if (response.statusCode >= 200 && response.statusCode < 300) {
         FileWriter.writeStreamToDisk(identifier, stream, options, callback)
       } else {
-        const err = new Error(`bad response from url: ${response.statusCode}`)
+        const err = new OError('bad response from url', {
+          statusCode: response.statusCode,
+        })
         logger.warn({ err, identifier, url }, `[writeUrlToDisk] ${err.message}`)
         return callback(err)
       }

+ 1 - 1
services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.mjs

@@ -198,7 +198,7 @@ describe('CollectPayPalPastDueInvoice', function () {
             body: invoiceCollectXml,
           }
         }
-        throw new OError(`Recurly API returned with status code: 404`, {
+        throw new OError('Recurly API returned with status code: 404', {
           statusCode: 404,
         })
       }

+ 1 - 1
services/web/test/smoke/src/support/Csrf.mjs

@@ -17,7 +17,7 @@ export function getCsrfTokenForFactory({ request }) {
       assertHasStatusCode(response, 200)
       return _parseCsrf(response.body)
     } catch (err) {
-      throw new OError(`error fetching csrf token on ${endpoint}`, {}, err)
+      throw new OError('error fetching csrf token', { endpoint }, err)
     }
   }
 }

+ 9 - 9
services/web/test/unit/src/Docstore/DocstoreManager.test.mjs

@@ -93,7 +93,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -201,7 +201,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -269,7 +269,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -391,7 +391,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -464,7 +464,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -515,7 +515,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -549,7 +549,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -583,7 +583,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })
@@ -617,7 +617,7 @@ describe('DocstoreManager', function () {
         expect(error).to.be.instanceOf(Error)
         expect(error).to.have.property(
           'message',
-          'docstore api responded with non-success code: 500'
+          'docstore api responded with non-success code'
         )
       })
     })

+ 18 - 17
services/web/test/unit/src/Project/ProjectOptionsHandler.test.mjs

@@ -55,10 +55,9 @@ describe('ProjectOptionsHandler', function () {
     })
 
     it('should not perform and update on mongo if it is not a recognised compiler', async function (ctx) {
-      const fakeComplier = 'something'
-      expect(
+      await expect(
         ctx.handler.promises.setCompiler(projectId, 'something')
-      ).to.be.rejectedWith(`invalid compiler: ${fakeComplier}`)
+      ).to.be.rejectedWith('invalid compiler')
 
       ctx.projectModel.updateOne.called.should.equal(false)
     })
@@ -76,8 +75,8 @@ describe('ProjectOptionsHandler', function () {
       })
 
       it('should be rejected', async function (ctx) {
-        expect(ctx.handler.promises.setCompiler(projectId, 'xeLaTeX')).to.be
-          .rejected
+        await expect(ctx.handler.promises.setCompiler(projectId, 'xeLaTeX')).to
+          .be.rejected
       })
     })
   })
@@ -92,9 +91,9 @@ describe('ProjectOptionsHandler', function () {
 
     it('should not perform and update on mongo if it is not a reconised image name', async function (ctx) {
       const fakeImageName = 'something'
-      expect(
+      await expect(
         ctx.handler.promises.setImageName(projectId, fakeImageName)
-      ).to.be.rejectedWith(`invalid imageName: ${fakeImageName}`)
+      ).to.be.rejectedWith('invalid imageName')
 
       ctx.projectModel.updateOne.called.should.equal(false)
     })
@@ -112,8 +111,9 @@ describe('ProjectOptionsHandler', function () {
       })
 
       it('should be rejected', async function (ctx) {
-        expect(ctx.handler.promises.setImageName(projectId, 'texlive-1234.5'))
-          .to.be.rejected
+        await expect(
+          ctx.handler.promises.setImageName(projectId, 'texlive-1234.5')
+        ).to.be.rejected
       })
     })
   })
@@ -128,9 +128,9 @@ describe('ProjectOptionsHandler', function () {
 
     it('should not perform and update on mongo if it is not a reconised langauge', async function (ctx) {
       const fakeLanguageCode = 'not a lang'
-      expect(
+      await expect(
         ctx.handler.promises.setSpellCheckLanguage(projectId, fakeLanguageCode)
-      ).to.be.rejectedWith(`invalid languageCode: ${fakeLanguageCode}`)
+      ).to.be.rejectedWith('invalid languageCode')
       ctx.projectModel.updateOne.called.should.equal(false)
     })
 
@@ -145,8 +145,8 @@ describe('ProjectOptionsHandler', function () {
       })
 
       it('should be rejected', async function (ctx) {
-        expect(ctx.handler.promises.setSpellCheckLanguage(projectId)).to.be
-          .rejected
+        await expect(ctx.handler.promises.setSpellCheckLanguage(projectId)).to
+          .be.rejected
       })
     })
   })
@@ -175,8 +175,8 @@ describe('ProjectOptionsHandler', function () {
       })
 
       it('should be rejected', async function (ctx) {
-        expect(ctx.handler.promises.setBrandVariationId(projectId, '123')).to.be
-          .rejected
+        await expect(ctx.handler.promises.setBrandVariationId(projectId, '123'))
+          .to.be.rejected
       })
     })
   })
@@ -197,8 +197,9 @@ describe('ProjectOptionsHandler', function () {
       })
 
       it('should be rejected', async function (ctx) {
-        expect(ctx.handler.promises.setHistoryRangesSupport(projectId, true)).to
-          .be.rejected
+        await expect(
+          ctx.handler.promises.setHistoryRangesSupport(projectId, true)
+        ).to.be.rejected
       })
     })
   })

+ 24 - 9
services/web/test/unit/src/User/SAMLIdentityManager.test.mjs

@@ -152,9 +152,12 @@ describe('SAMLIdentityManager', function () {
         error = e
       } finally {
         expect(error).to.exist
-        expect(error.message).to.equal(
-          'invalid arguments: providerId: undefined, externalUserId: undefined, userIdAttribute: undefined'
-        )
+        expect(error.message).to.equal('invalid arguments')
+        expect(error.info).to.deep.equal({
+          providerId: undefined,
+          externalUserId: undefined,
+          userIdAttribute: undefined,
+        })
       }
     })
     it('should throw an error if missing provider ID', async function (ctx) {
@@ -165,9 +168,12 @@ describe('SAMLIdentityManager', function () {
         error = e
       } finally {
         expect(error).to.exist
-        expect(error.message).to.equal(
-          'invalid arguments: providerId: undefined, externalUserId: id123, userIdAttribute: someAttr'
-        )
+        expect(error.message).to.equal('invalid arguments')
+        expect(error.info).to.deep.equal({
+          providerId: undefined,
+          externalUserId: 'id123',
+          userIdAttribute: 'someAttr',
+        })
       }
     })
     it('should throw an error if missing external user ID', async function (ctx) {
@@ -178,6 +184,12 @@ describe('SAMLIdentityManager', function () {
         error = e
       } finally {
         expect(error).to.exist
+        expect(error.message).to.equal('invalid arguments')
+        expect(error.info).to.deep.equal({
+          providerId: '123',
+          externalUserId: null,
+          userIdAttribute: 'someAttr',
+        })
       }
     })
     it('should throw an error if missing attribute', async function (ctx) {
@@ -188,9 +200,12 @@ describe('SAMLIdentityManager', function () {
         error = e
       } finally {
         expect(error).to.exist
-        expect(error.message).to.equal(
-          'invalid arguments: providerId: 123, externalUserId: id123, userIdAttribute: undefined'
-        )
+        expect(error.message).to.equal('invalid arguments')
+        expect(error.info).to.deep.equal({
+          providerId: '123',
+          externalUserId: 'id123',
+          userIdAttribute: undefined,
+        })
       }
     })
   })