diff --git a/services/real-time/app/js/AuthorizationManager.js b/services/real-time/app/js/AuthorizationManager.js index a545594479..0c37ac1423 100644 --- a/services/real-time/app/js/AuthorizationManager.js +++ b/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()) } }, diff --git a/services/real-time/app/js/DocumentUpdaterManager.js b/services/real-time/app/js/DocumentUpdaterManager.js index 3fd09d7b1d..67ecd82c29 100644 --- a/services/real-time/app/js/DocumentUpdaterManager.js +++ b/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 diff --git a/services/real-time/app/js/Errors.js b/services/real-time/app/js/Errors.js index bdda1b4c21..562c9888ab 100644 --- a/services/real-time/app/js/Errors.js +++ b/services/real-time/app/js/Errors.js @@ -1,9 +1,103 @@ -class CodedError extends Error { - constructor(message, code) { - super(message) - this.name = this.constructor.name - this.code = code +const OError = require('@overleaf/o-error') + +class ClientRequestedMissingOpsError extends OError { + constructor(statusCode) { + super('doc updater could not load requested ops', { + statusCode + }) } } -module.exports = { CodedError } +class CodedError extends OError { + constructor(message, 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') + } +} + +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 +} diff --git a/services/real-time/app/js/Router.js b/services/real-time/app/js/Router.js index ae10d12e2b..5e0ae4bf76 100644 --- a/services/real-time/app/js/Router.js +++ b/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 () {} diff --git a/services/real-time/app/js/SafeJsonParse.js b/services/real-time/app/js/SafeJsonParse.js index 982fd9d424..a8a3afae4d 100644 --- a/services/real-time/app/js/SafeJsonParse.js +++ b/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 { diff --git a/services/real-time/app/js/SessionSockets.js b/services/real-time/app/js/SessionSockets.js index 4ade959829..84f87bf872 100644 --- a/services/real-time/app/js/SessionSockets.js +++ b/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) { diff --git a/services/real-time/app/js/WebApiManager.js b/services/real-time/app/js/WebApiManager.js index 62020a98ab..a39d9dd832 100644 --- a/services/real-time/app/js/WebApiManager.js +++ b/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)) } } ) diff --git a/services/real-time/app/js/WebsocketController.js b/services/real-time/app/js/WebsocketController.js index 8867320dbe..a443a49f95 100644 --- a/services/real-time/app/js/WebsocketController.js +++ b/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' diff --git a/services/real-time/package.json b/services/real-time/package.json index 4a7acfc1cd..0fab05972f 100644 --- a/services/real-time/package.json +++ b/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", diff --git a/services/real-time/test/unit/js/DocumentUpdaterManagerTests.js b/services/real-time/test/unit/js/DocumentUpdaterManagerTests.js index dc42b52140..828f516364 100644 --- a/services/real-time/test/unit/js/DocumentUpdaterManagerTests.js +++ b/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 () { diff --git a/services/real-time/test/unit/js/SafeJsonParseTest.js b/services/real-time/test/unit/js/SafeJsonParseTest.js index 4fb558a6b0..51a60dea23 100644 --- a/services/real-time/test/unit/js/SafeJsonParseTest.js +++ b/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() }) diff --git a/services/real-time/test/unit/js/WebApiManagerTests.js b/services/real-time/test/unit/js/WebApiManagerTests.js index 2d792b563b..4435bc14f9 100644 --- a/services/real-time/test/unit/js/WebApiManagerTests.js +++ b/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) diff --git a/services/real-time/test/unit/js/WebsocketControllerTests.js b/services/real-time/test/unit/js/WebsocketControllerTests.js index 515d407cc2..7c42dd6256 100644 --- a/services/real-time/test/unit/js/WebsocketControllerTests.js +++ b/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)