Merge pull request #184 from overleaf/jpa-custom-errors

[misc] migrate to custom OErrors

services/real-time/app/js/AuthorizationManager.js

@@ -1,6 +1,8 @@
 /* eslint-disable
     camelcase,
 */
+const { NotAuthorizedError } = require('./Errors')
+
 let AuthorizationManager
 module.exports = AuthorizationManager = {
   assertClientCanViewProject(client, callback) {
@@ -23,7 +25,7 @@ module.exports = AuthorizationManager = {
     if (allowedLevels.includes(client.ol_context.privilege_level)) {
       callback(null)
     } else {
-      callback(new Error('not authorized'))
+      callback(new NotAuthorizedError())
     }
   },
 
@@ -49,7 +51,7 @@ module.exports = AuthorizationManager = {
     if (client.ol_context[`doc:${doc_id}`] === 'allowed') {
       callback(null)
     } else {
-      callback(new Error('not authorized'))
+      callback(new NotAuthorizedError())
     }
   },
 

services/real-time/app/js/DocumentUpdaterManager.js

@@ -6,6 +6,12 @@ const _ = require('underscore')
 const logger = require('logger-sharelatex')
 const settings = require('settings-sharelatex')
 const metrics = require('metrics-sharelatex')
+const {
+  ClientRequestedMissingOpsError,
+  DocumentUpdaterRequestFailedError,
+  NullBytesInOpError,
+  UpdateTooLargeError
+} = require('./Errors')
 
 const rclient = require('redis-sharelatex').createClient(
   settings.redis.documentupdater
@@ -42,23 +48,11 @@ const DocumentUpdaterManager = {
         body = body || {}
         callback(null, body.lines, body.version, body.ranges, body.ops)
       } else if ([404, 422].includes(res.statusCode)) {
-        err = new Error('doc updater could not load requested ops')
-        err.statusCode = res.statusCode
-        logger.warn(
-          { err, project_id, doc_id, url, fromVersion },
-          'doc updater could not load requested ops'
-        )
-        callback(err)
+        callback(new ClientRequestedMissingOpsError(res.statusCode))
       } else {
-        err = new Error(
-          `doc updater returned a non-success status code: ${res.statusCode}`
+        callback(
+          new DocumentUpdaterRequestFailedError('getDocument', res.statusCode)
         )
-        err.statusCode = res.statusCode
-        logger.error(
-          { err, project_id, doc_id, url },
-          `doc updater returned a non-success status code: ${res.statusCode}`
-        )
-        callback(err)
       }
     })
   },
@@ -88,15 +82,12 @@ const DocumentUpdaterManager = {
         logger.log({ project_id }, 'deleted project from document updater')
         callback(null)
       } else {
-        err = new Error(
-          `document updater returned a failure status code: ${res.statusCode}`
+        callback(
+          new DocumentUpdaterRequestFailedError(
+            'flushProjectToMongoAndDelete',
+            res.statusCode
+          )
         )
-        err.statusCode = res.statusCode
-        logger.error(
-          { err, project_id },
-          `document updater returned failure status code: ${res.statusCode}`
-        )
-        callback(err)
       }
     })
   },
@@ -115,19 +106,12 @@ const DocumentUpdaterManager = {
     const jsonChange = JSON.stringify(change)
     if (jsonChange.indexOf('\u0000') !== -1) {
       // memory corruption check
-      const error = new Error('null bytes found in op')
-      logger.error(
-        { err: error, project_id, doc_id, jsonChange },
-        error.message
-      )
-      return callback(error)
+      return callback(new NullBytesInOpError(jsonChange))
     }
 
     const updateSize = jsonChange.length
     if (updateSize > settings.maxUpdateSize) {
-      const error = new Error('update is too large')
-      error.updateSize = updateSize
-      return callback(error)
+      return callback(new UpdateTooLargeError(updateSize))
     }
 
     // record metric for each update added to queue

services/real-time/app/js/Errors.js

@@ -1,9 +1,103 @@
-class CodedError extends Error {
+const OError = require('@overleaf/o-error')
+
+class ClientRequestedMissingOpsError extends OError {
+  constructor(statusCode) {
+    super('doc updater could not load requested ops', {
+      statusCode
+    })
+  }
+}
+
+class CodedError extends OError {
   constructor(message, code) {
-    super(message)
-    this.name = this.constructor.name
-    this.code = code
+    super(message, { code })
+  }
+}
+
+class CorruptedJoinProjectResponseError extends OError {
+  constructor() {
+    super('no data returned from joinProject request')
+  }
+}
+
+class DataTooLargeToParseError extends OError {
+  constructor(data) {
+    super('data too large to parse', {
+      head: data.slice(0, 1024),
+      length: data.length
+    })
+  }
+}
+
+class DocumentUpdaterRequestFailedError extends OError {
+  constructor(action, statusCode) {
+    super('doc updater returned a non-success status code', {
+      action,
+      statusCode
+    })
+  }
+}
+
+class JoinLeaveEpochMismatchError extends OError {
+  constructor() {
+    super('joinLeaveEpoch mismatch')
+  }
+}
+
+class MissingSessionError extends OError {
+  constructor() {
+    super('could not look up session by key')
   }
 }
 
-module.exports = { CodedError }
+class NotAuthorizedError extends OError {
+  constructor() {
+    super('not authorized')
+  }
+}
+
+class NotJoinedError extends OError {
+  constructor() {
+    super('no project_id found on client')
+  }
+}
+
+class NullBytesInOpError extends OError {
+  constructor(jsonChange) {
+    super('null bytes found in op', { jsonChange })
+  }
+}
+
+class UnexpectedArgumentsError extends OError {
+  constructor() {
+    super('unexpected arguments')
+  }
+}
+
+class UpdateTooLargeError extends OError {
+  constructor(updateSize) {
+    super('update is too large', { updateSize })
+  }
+}
+
+class WebApiRequestFailedError extends OError {
+  constructor(statusCode) {
+    super('non-success status code from web', { statusCode })
+  }
+}
+
+module.exports = {
+  CodedError,
+  CorruptedJoinProjectResponseError,
+  ClientRequestedMissingOpsError,
+  DataTooLargeToParseError,
+  DocumentUpdaterRequestFailedError,
+  JoinLeaveEpochMismatchError,
+  MissingSessionError,
+  NotAuthorizedError,
+  NotJoinedError,
+  NullBytesInOpError,
+  UnexpectedArgumentsError,
+  UpdateTooLargeError,
+  WebApiRequestFailedError
+}

services/real-time/app/js/Router.js

@@ -9,6 +9,7 @@ const HttpController = require('./HttpController')
 const HttpApiController = require('./HttpApiController')
 const bodyParser = require('body-parser')
 const base64id = require('base64id')
+const { UnexpectedArgumentsError } = require('./Errors')
 
 const basicAuth = require('basic-auth-connect')
 const httpAuth = basicAuth(function (user, pass) {
@@ -33,8 +34,8 @@ module.exports = Router = {
     attrs.client_id = client.id
     attrs.err = error
     if (error.name === 'CodedError') {
-      logger.warn(attrs, error.message, { code: error.code })
-      const serializedError = { message: error.message, code: error.code }
+      logger.warn(attrs, error.message)
+      const serializedError = { message: error.message, code: error.info.code }
       callback(serializedError)
     } else if (error.message === 'unexpected arguments') {
       // the payload might be very large, put it on level info
@@ -64,7 +65,7 @@ module.exports = Router = {
   },
 
   _handleInvalidArguments(client, method, args) {
-    const error = new Error('unexpected arguments')
+    const error = new UnexpectedArgumentsError()
     let callback = args[args.length - 1]
     if (typeof callback !== 'function') {
       callback = function () {}

services/real-time/app/js/SafeJsonParse.js

@@ -1,14 +1,10 @@
 const Settings = require('settings-sharelatex')
-const logger = require('logger-sharelatex')
+const { DataTooLargeToParseError } = require('./Errors')
 
 module.exports = {
   parse(data, callback) {
     if (data.length > Settings.maxUpdateSize) {
-      logger.error(
-        { head: data.slice(0, 1024), length: data.length },
-        'data too large to parse'
-      )
-      return callback(new Error('data too large to parse'))
+      return callback(new DataTooLargeToParseError(data))
     }
     let parsed
     try {

services/real-time/app/js/SessionSockets.js

@@ -1,7 +1,8 @@
 const { EventEmitter } = require('events')
+const { MissingSessionError } = require('./Errors')
 
 module.exports = function (io, sessionStore, cookieParser, cookieName) {
-  const missingSessionError = new Error('could not look up session by key')
+  const missingSessionError = new MissingSessionError()
 
   const sessionSockets = new EventEmitter()
   function next(error, socket, session) {

services/real-time/app/js/WebApiManager.js

@@ -4,7 +4,11 @@
 const request = require('request')
 const settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
-const { CodedError } = require('./Errors')
+const {
+  CodedError,
+  CorruptedJoinProjectResponseError,
+  WebApiRequestFailedError
+} = require('./Errors')
 
 module.exports = {
   joinProject(project_id, user, callback) {
@@ -32,15 +36,9 @@ module.exports = {
         if (error) {
           return callback(error)
         }
-        let err
         if (response.statusCode >= 200 && response.statusCode < 300) {
           if (!(data && data.project)) {
-            err = new Error('no data returned from joinProject request')
-            logger.error(
-              { err, project_id, user_id },
-              'error accessing web api'
-            )
-            return callback(err)
+            return callback(new CorruptedJoinProjectResponseError())
           }
           callback(
             null,
@@ -57,11 +55,7 @@ module.exports = {
             )
           )
         } else {
-          err = new Error(
-            `non-success status code from web: ${response.statusCode}`
-          )
-          logger.error({ err, project_id, user_id }, 'error accessing web api')
-          callback(err)
+          callback(new WebApiRequestFailedError(response.statusCode))
         }
       }
     )

services/real-time/app/js/WebsocketController.js

@@ -9,6 +9,11 @@ const DocumentUpdaterManager = require('./DocumentUpdaterManager')
 const ConnectedUsersManager = require('./ConnectedUsersManager')
 const WebsocketLoadBalancer = require('./WebsocketLoadBalancer')
 const RoomManager = require('./RoomManager')
+const {
+  JoinLeaveEpochMismatchError,
+  NotAuthorizedError,
+  NotJoinedError
+} = require('./Errors')
 
 let WebsocketController
 module.exports = WebsocketController = {
@@ -48,12 +53,7 @@ module.exports = WebsocketController = {
       }
 
       if (!privilegeLevel) {
-        const err = new Error('not authorized')
-        logger.warn(
-          { err, project_id, user_id, client_id: client.id },
-          'user is not authorized to join project'
-        )
-        return callback(err)
+        return callback(new NotAuthorizedError())
       }
 
       client.ol_context = {}
@@ -162,7 +162,7 @@ module.exports = WebsocketController = {
     metrics.inc('editor.join-doc')
     const { project_id, user_id, is_restricted_user } = client.ol_context
     if (!project_id) {
-      return callback(new Error('no project_id found on client'))
+      return callback(new NotJoinedError())
     }
     logger.log(
       { user_id, project_id, doc_id, fromVersion, client_id: client.id },
@@ -184,7 +184,7 @@ module.exports = WebsocketController = {
       }
       if (joinLeaveEpoch !== client.joinLeaveEpoch) {
         // another joinDoc or leaveDoc rpc overtook us
-        return callback(new Error('joinLeaveEpoch mismatch'))
+        return callback(new JoinLeaveEpochMismatchError())
       }
       // ensure the per-doc applied-ops channel is subscribed before sending the
       // doc to the client, so that no events are missed.
@@ -428,7 +428,7 @@ module.exports = WebsocketController = {
       return callback(null, [])
     }
     if (!project_id) {
-      return callback(new Error('no project_id found on client'))
+      return callback(new NotJoinedError())
     }
     logger.log(
       { user_id, project_id, client_id: client.id },
@@ -463,7 +463,7 @@ module.exports = WebsocketController = {
     // client may have disconnected, but we can submit their update to doc-updater anyways.
     const { user_id, project_id } = client.ol_context
     if (!project_id) {
-      return callback(new Error('no project_id found on client'))
+      return callback(new NotJoinedError())
     }
 
     WebsocketController._assertClientCanApplyUpdate(
@@ -509,7 +509,7 @@ module.exports = WebsocketController = {
           function (error) {
             if ((error && error.message) === 'update is too large') {
               metrics.inc('update_too_large')
-              const { updateSize } = error
+              const { updateSize } = error.info
               logger.warn(
                 { user_id, project_id, doc_id, updateSize },
                 'update is too large'

services/real-time/package.json

@@ -19,6 +19,7 @@
     "format:fix": "node_modules/.bin/prettier-eslint $PWD'/**/*.js' --write"
   },
   "dependencies": {
+    "@overleaf/o-error": "^3.0.0",
     "async": "^0.9.0",
     "base64id": "0.1.0",
     "basic-auth-connect": "^1.0.0",

services/real-time/test/unit/js/DocumentUpdaterManagerTests.js

@@ -136,14 +136,16 @@ describe('DocumentUpdaterManager', function () {
 
         return it('should return the callback with an error', function () {
           this.callback.called.should.equal(true)
-          const err = this.callback.getCall(0).args[0]
-          err.should.have.property('statusCode', statusCode)
-          err.should.have.property(
-            'message',
-            'doc updater could not load requested ops'
-          )
+          this.callback
+            .calledWith(
+              sinon.match({
+                message: 'doc updater could not load requested ops',
+                info: { statusCode }
+              })
+            )
+            .should.equal(true)
           this.logger.error.called.should.equal(false)
-          return this.logger.warn.called.should.equal(true)
+          this.logger.warn.called.should.equal(false)
         })
       })
     )
@@ -163,13 +165,18 @@ describe('DocumentUpdaterManager', function () {
 
       return it('should return the callback with an error', function () {
         this.callback.called.should.equal(true)
-        const err = this.callback.getCall(0).args[0]
-        err.should.have.property('statusCode', 500)
-        err.should.have.property(
-          'message',
-          'doc updater returned a non-success status code: 500'
-        )
-        return this.logger.error.called.should.equal(true)
+        this.callback
+          .calledWith(
+            sinon.match({
+              message: 'doc updater returned a non-success status code',
+              info: {
+                action: 'getDocument',
+                statusCode: 500
+              }
+            })
+          )
+          .should.equal(true)
+        this.logger.error.called.should.equal(false)
       })
     })
   })
@@ -234,12 +241,17 @@ describe('DocumentUpdaterManager', function () {
 
       return it('should return the callback with an error', function () {
         this.callback.called.should.equal(true)
-        const err = this.callback.getCall(0).args[0]
-        err.should.have.property('statusCode', 500)
-        return err.should.have.property(
-          'message',
-          'document updater returned a failure status code: 500'
-        )
+        this.callback
+          .calledWith(
+            sinon.match({
+              message: 'doc updater returned a non-success status code',
+              info: {
+                action: 'flushProjectToMongoAndDelete',
+                statusCode: 500
+              }
+            })
+          )
+          .should.equal(true)
       })
     })
   })
@@ -346,7 +358,7 @@ describe('DocumentUpdaterManager', function () {
       })
 
       it('should add the size to the error', function () {
-        return this.callback.args[0][0].updateSize.should.equal(7782422)
+        return this.callback.args[0][0].info.updateSize.should.equal(7782422)
       })
 
       return it('should not push the change onto the pending-updates-list queue', function () {

services/real-time/test/unit/js/SafeJsonParseTest.js

@@ -50,7 +50,7 @@ describe('SafeJsonParse', function () {
       const data = `{\"foo\": \"${big_blob}\"}`
       this.Settings.maxUpdateSize = 2 * 1024
       return this.SafeJsonParse.parse(data, (error, parsed) => {
-        this.logger.error.called.should.equal(true)
+        this.logger.error.called.should.equal(false)
         expect(error).to.exist
         return done()
       })

services/real-time/test/unit/js/WebApiManagerTests.js

@@ -106,7 +106,10 @@ describe('WebApiManager', function () {
       return it('should call the callback with an error', function () {
         return this.callback
           .calledWith(
-            sinon.match({ message: 'non-success status code from web: 500' })
+            sinon.match({
+              message: 'non-success status code from web',
+              info: { statusCode: 500 }
+            })
           )
           .should.equal(true)
       })
@@ -152,7 +155,9 @@ describe('WebApiManager', function () {
           .calledWith(
             sinon.match({
               message: 'rate-limit hit when joining project',
-              code: 'TooManyRequests'
+              info: {
+                code: 'TooManyRequests'
+              }
             })
           )
           .should.equal(true)

services/real-time/test/unit/js/WebsocketControllerTests.js

@@ -19,6 +19,7 @@ const { expect } = chai
 const modulePath = '../../../app/js/WebsocketController.js'
 const SandboxedModule = require('sandboxed-module')
 const tk = require('timekeeper')
+const { UpdateTooLargeError } = require('../../../app/js/Errors')
 
 describe('WebsocketController', function () {
   beforeEach(function () {
@@ -1507,8 +1508,7 @@ describe('WebsocketController', function () {
         this.client.emit = sinon.stub()
         this.client.ol_context.user_id = this.user_id
         this.client.ol_context.project_id = this.project_id
-        const error = new Error('update is too large')
-        error.updateSize = 7372835
+        const error = new UpdateTooLargeError(7372835)
         this.DocumentUpdaterManager.queueChange = sinon
           .stub()
           .callsArgWith(3, error)