From 2e8f66ac6b3e10143767a0c1855d94c804df150e Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Tue, 11 Aug 2020 11:35:08 +0200 Subject: [PATCH] Merge pull request #3075 from overleaf/msm-codemod-oerror-v3-logging Replaced logger.warn statements with OError.tag() GitOrigin-RevId: 4d821ec48a0006abb1fcffe07dbb5511c88f5b9a --- .../BrandVariations/BrandVariationsHandler.js | 8 +- .../app/src/Features/Chat/ChatApiHandler.js | 8 +- .../CollaboratorsInviteHandler.js | 81 +++++++---- .../src/Features/Compile/ClsiCookieManager.js | 12 +- .../src/Features/Contacts/ContactManager.js | 23 ++- .../src/Features/Docstore/DocstoreManager.js | 83 ++++++----- .../DocumentUpdater/DocumentUpdaterHandler.js | 24 ++-- .../src/Features/Exports/ExportsHandler.js | 74 ++++------ .../Features/FileStore/FileStoreHandler.js | 49 ++++--- .../InactiveData/InactiveProjectManager.js | 12 +- .../Project/ProjectCreationHandler.js | 6 +- .../Project/ProjectEntityUpdateHandler.js | 45 +++--- .../app/src/Features/Project/ProjectGetter.js | 6 +- .../src/Features/Referal/ReferalAllocator.js | 10 +- .../Features/References/ReferencesHandler.js | 31 +++-- .../Features/Subscription/FeaturesUpdater.js | 14 +- .../Subscription/LimitationsManager.js | 5 +- .../Features/Subscription/RecurlyWrapper.js | 90 +++++++----- .../Subscription/SubscriptionUpdater.js | 10 +- .../Features/SudoMode/SudoModeMiddleware.js | 8 +- .../Features/Templates/TemplatesManager.js | 5 +- .../ThirdPartyDataStore/UpdateMerger.js | 14 +- .../src/Features/Uploads/ArchiveManager.js | 16 ++- .../User/ThirdPartyIdentityManager.js | 3 +- .../src/Features/User/UserSessionsManager.js | 74 +++++----- .../web/app/src/Features/User/UserUpdater.js | 37 ++--- services/web/app/src/Features/V1/V1Handler.js | 15 +- .../web/app/src/infrastructure/FileWriter.js | 11 +- .../unit/src/Contact/ContactManagerTests.js | 43 ------ .../src/FileStore/FileStoreHandlerTests.js | 2 +- .../InactiveProjectManagerTests.js | 7 +- .../test/unit/src/User/UserUpdaterTests.js | 19 +-- services/web/transform/o-error/transform.js | 131 ++++++++---------- 33 files changed, 487 insertions(+), 489 deletions(-) diff --git a/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js b/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js index f36f0e4e5a..580d13761a 100644 --- a/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js +++ b/services/web/app/src/Features/BrandVariations/BrandVariationsHandler.js @@ -13,6 +13,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let BrandVariationsHandler +const OError = require('@overleaf/o-error') const url = require('url') const settings = require('settings-sharelatex') const logger = require('logger-sharelatex') @@ -33,10 +34,9 @@ module.exports = BrandVariationsHandler = { }, function(error, response, brandVariationDetails) { if (error != null) { - logger.warn( - { brandVariationId, error }, - 'error getting brand variation details' - ) + OError.tag(error, 'error getting brand variation details', { + brandVariationId + }) return callback(error) } _formatBrandVariationDetails(brandVariationDetails) diff --git a/services/web/app/src/Features/Chat/ChatApiHandler.js b/services/web/app/src/Features/Chat/ChatApiHandler.js index 1c2a12fe5e..ebed71feba 100644 --- a/services/web/app/src/Features/Chat/ChatApiHandler.js +++ b/services/web/app/src/Features/Chat/ChatApiHandler.js @@ -12,9 +12,9 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let ChatApiHandler +const OError = require('@overleaf/o-error') const request = require('request') const settings = require('settings-sharelatex') -const logger = require('logger-sharelatex') module.exports = ChatApiHandler = { _apiRequest(opts, callback) { @@ -28,11 +28,11 @@ module.exports = ChatApiHandler = { if (response.statusCode >= 200 && response.statusCode < 300) { return callback(null, data) } else { - error = new Error( - `chat api returned non-success code: ${response.statusCode}` + error = new OError( + `chat api returned non-success code: ${response.statusCode}`, + opts ) error.statusCode = response.statusCode - logger.warn({ err: error, opts }, 'error sending request to chat api') return callback(error) } }) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js index 7a9934f5d5..3728d68c5d 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js @@ -12,6 +12,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const { ProjectInvite } = require('../../models/ProjectInvite') +const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const CollaboratorsEmailHandler = require('./CollaboratorsEmailHandler') const CollaboratorsHandler = require('./CollaboratorsHandler') @@ -32,7 +33,9 @@ const CollaboratorsInviteHandler = { logger.log({ projectId }, 'fetching invites for project') return ProjectInvite.find({ projectId }, function(err, invites) { if (err != null) { - logger.warn({ err, projectId }, 'error getting invites from mongo') + OError.tag(err, 'error getting invites from mongo', { + projectId + }) return callback(err) } logger.log( @@ -50,7 +53,9 @@ const CollaboratorsInviteHandler = { logger.log({ projectId }, 'counting invites for project') return ProjectInvite.count({ projectId }, function(err, count) { if (err != null) { - logger.warn({ err, projectId }, 'error getting invites from mongo') + OError.tag(err, 'error getting invites from mongo', { + projectId + }) return callback(err) } return callback(null, count) @@ -67,7 +72,10 @@ const CollaboratorsInviteHandler = { existingUser ) { if (err != null) { - logger.warn({ projectId, email }, 'error checking if user exists') + OError.tag(err, 'error checking if user exists', { + projectId, + email + }) return callback(err) } if (existingUser == null) { @@ -79,7 +87,10 @@ const CollaboratorsInviteHandler = { project ) { if (err != null) { - logger.warn({ projectId, email }, 'error getting project') + OError.tag(err, 'error getting project', { + projectId, + email + }) return callback(err) } if (project == null) { @@ -153,10 +164,11 @@ const CollaboratorsInviteHandler = { ) return Crypto.randomBytes(24, function(err, buffer) { if (err != null) { - logger.warn( - { err, projectId, sendingUserId: sendingUser._id, email }, - 'error generating random token' - ) + OError.tag(err, 'error generating random token', { + projectId, + sendingUserId: sendingUser._id, + email + }) return callback(err) } const token = buffer.toString('hex') @@ -169,10 +181,11 @@ const CollaboratorsInviteHandler = { }) return invite.save(function(err, invite) { if (err != null) { - logger.warn( - { err, projectId, sendingUserId: sendingUser._id, email }, - 'error saving token' - ) + OError.tag(err, 'error saving token', { + projectId, + sendingUserId: sendingUser._id, + email + }) return callback(err) } // Send email and notification in background @@ -201,7 +214,10 @@ const CollaboratorsInviteHandler = { logger.log({ projectId, inviteId }, 'removing invite') return ProjectInvite.remove({ projectId, _id: inviteId }, function(err) { if (err != null) { - logger.warn({ err, projectId, inviteId }, 'error removing invite') + OError.tag(err, 'error removing invite', { + projectId, + inviteId + }) return callback(err) } CollaboratorsInviteHandler._tryCancelInviteNotification( @@ -222,7 +238,10 @@ const CollaboratorsInviteHandler = { invite ) { if (err != null) { - logger.warn({ err, projectId, inviteId }, 'error finding invite') + OError.tag(err, 'error finding invite', { + projectId, + inviteId + }) return callback(err) } if (invite == null) { @@ -238,10 +257,10 @@ const CollaboratorsInviteHandler = { invite, function(err) { if (err != null) { - logger.warn( - { projectId, inviteId }, - 'error resending invite messages' - ) + OError.tag(err, 'error resending invite messages', { + projectId, + inviteId + }) return callback(err) } return callback(null) @@ -260,7 +279,9 @@ const CollaboratorsInviteHandler = { invite ) { if (err != null) { - logger.warn({ err, projectId }, 'error fetching invite') + OError.tag(err, 'error fetching invite', { + projectId + }) return callback(err) } if (invite == null) { @@ -281,7 +302,10 @@ const CollaboratorsInviteHandler = { tokenString, function(err, invite) { if (err != null) { - logger.warn({ err, projectId, tokenString }, 'error finding invite') + OError.tag(err, 'error finding invite', { + projectId, + tokenString + }) return callback(err) } if (!invite) { @@ -300,20 +324,21 @@ const CollaboratorsInviteHandler = { invite.privileges, function(err) { if (err != null) { - logger.warn( - { err, projectId, inviteId, userId: user._id }, - 'error adding user to project' - ) + OError.tag(err, 'error adding user to project', { + projectId, + inviteId, + userId: user._id + }) return callback(err) } // Remove invite logger.log({ projectId, inviteId }, 'removing invite') return ProjectInvite.remove({ _id: inviteId }, function(err) { if (err != null) { - logger.warn( - { err, projectId, inviteId }, - 'error removing invite' - ) + OError.tag(err, 'error removing invite', { + projectId, + inviteId + }) return callback(err) } CollaboratorsInviteHandler._tryCancelInviteNotification( diff --git a/services/web/app/src/Features/Compile/ClsiCookieManager.js b/services/web/app/src/Features/Compile/ClsiCookieManager.js index 8a07882fd7..b1201e8d11 100644 --- a/services/web/app/src/Features/Compile/ClsiCookieManager.js +++ b/services/web/app/src/Features/Compile/ClsiCookieManager.js @@ -12,6 +12,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let rclient_secondary +const OError = require('@overleaf/o-error') const Settings = require('settings-sharelatex') const request = require('request') const RedisWrapper = require('../../infrastructure/RedisWrapper') @@ -59,10 +60,9 @@ module.exports = function(backendGroup) { const url = `${Settings.apis.clsi.url}/project/${project_id}/status` return request.get(url, (err, res, body) => { if (err != null) { - logger.warn( - { err, project_id }, - 'error getting initial server id for project' - ) + OError.tag(err, 'error getting initial server id for project', { + project_id + }) return callback(err) } return this.setServerId(project_id, res, function(err, serverId) { @@ -139,7 +139,9 @@ module.exports = function(backendGroup) { } return this._getServerId(project_id, (err, serverId) => { if (err != null) { - logger.warn({ err, project_id }, 'error getting server id') + OError.tag(err, 'error getting server id', { + project_id + }) return callback(err) } const serverCookie = request.cookie( diff --git a/services/web/app/src/Features/Contacts/ContactManager.js b/services/web/app/src/Features/Contacts/ContactManager.js index aaeb3cab8e..807ecdf2d1 100644 --- a/services/web/app/src/Features/Contacts/ContactManager.js +++ b/services/web/app/src/Features/Contacts/ContactManager.js @@ -13,9 +13,9 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let ContactManager +const OError = require('@overleaf/o-error') const request = require('request') const settings = require('settings-sharelatex') -const logger = require('logger-sharelatex') module.exports = ContactManager = { getContactIds(user_id, options, callback) { @@ -43,12 +43,9 @@ module.exports = ContactManager = { (data != null ? data.contact_ids : undefined) || [] ) } else { - error = new Error( - `contacts api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, user_id }, - 'error getting contacts for user' + error = new OError( + `contacts api responded with non-success code: ${res.statusCode}`, + { user_id } ) return callback(error) } @@ -79,12 +76,12 @@ module.exports = ContactManager = { (data != null ? data.contact_ids : undefined) || [] ) } else { - error = new Error( - `contacts api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, user_id, contact_id }, - 'error adding contact for user' + error = new OError( + `contacts api responded with non-success code: ${res.statusCode}`, + { + user_id, + contact_id + } ) return callback(error) } diff --git a/services/web/app/src/Features/Docstore/DocstoreManager.js b/services/web/app/src/Features/Docstore/DocstoreManager.js index aac383ef5d..5424883c2d 100644 --- a/services/web/app/src/Features/Docstore/DocstoreManager.js +++ b/services/web/app/src/Features/Docstore/DocstoreManager.js @@ -13,6 +13,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const request = require('request').defaults({ jar: false }) +const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const settings = require('settings-sharelatex') const Errors = require('../Errors/Errors') @@ -39,19 +40,21 @@ const DocstoreManager = { if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null) } else if (res.statusCode === 404) { - error = new Errors.NotFoundError('tried to delete doc not in docstore') - logger.warn( - { err: error, project_id, doc_id }, - 'tried to delete doc not in docstore' - ) + error = new Errors.NotFoundError({ + message: 'tried to delete doc not in docstore', + info: { + project_id, + doc_id + } + }) return callback(error) // maybe suppress the error when delete doc which is not present? } else { - error = new Error( - `docstore api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, project_id, doc_id }, - 'error deleting doc in docstore' + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { + project_id, + doc_id + } ) return callback(error) } @@ -76,12 +79,9 @@ const DocstoreManager = { if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null, docs) } else { - error = new Error( - `docstore api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, project_id }, - 'error getting all docs from docstore' + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { project_id } ) return callback(error) } @@ -107,12 +107,9 @@ const DocstoreManager = { if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null, docs) } else { - error = new Error( - `docstore api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, project_id }, - 'error getting all doc ranges from docstore' + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { project_id } ) return callback(error) } @@ -154,19 +151,21 @@ const DocstoreManager = { ) return callback(null, doc.lines, doc.rev, doc.version, doc.ranges) } else if (res.statusCode === 404) { - error = new Errors.NotFoundError('doc not found in docstore') - logger.warn( - { err: error, project_id, doc_id }, - 'doc not found in docstore' - ) + error = new Errors.NotFoundError({ + message: 'doc not found in docstore', + info: { + project_id, + doc_id + } + }) return callback(error) } else { - error = new Error( - `docstore api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, project_id, doc_id }, - 'error getting doc from docstore' + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { + project_id, + doc_id + } ) return callback(error) } @@ -202,12 +201,9 @@ const DocstoreManager = { ) return callback(null, result.modified, result.rev) } else { - error = new Error( - `docstore api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, project_id, doc_id }, - 'error updating doc in docstore' + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { project_id, doc_id } ) return callback(error) } @@ -233,10 +229,9 @@ const DocstoreManager = { // use default timeout for archiving/unarchiving/destroying request.post(url, function(err, res, docs) { if (err != null) { - logger.warn( - { err, project_id }, - `error calling ${method} project in docstore` - ) + OError.tag(err, `error calling ${method} project in docstore`, { + project_id + }) return callback(err) } diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index 082856052a..92a7cc055c 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -1,4 +1,5 @@ const request = require('request').defaults() +const OError = require('@overleaf/o-error') const settings = require('settings-sharelatex') const _ = require('underscore') const async = require('async') @@ -137,10 +138,10 @@ function getProjectDocsIfMatch(projectId, projectStateHash, callback) { request.post(url, function(error, res, body) { timer.done() if (error) { - logger.warn( - { err: error, url, projectId }, - 'error getting project docs from doc updater' - ) + OError.tag(error, 'error getting project docs from doc updater', { + url, + projectId + }) return callback(error) } if (res.statusCode === 409) { @@ -156,18 +157,17 @@ function getProjectDocsIfMatch(projectId, projectStateHash, callback) { try { docs = JSON.parse(body) } catch (error1) { - error = error1 - return callback(error) + return callback(OError.tag(error1)) } callback(null, docs) } else { - logger.warn( - { projectId, url }, - `doc updater returned a non-success status code: ${res.statusCode}` - ) callback( - new Error( - `doc updater returned a non-success status code: ${res.statusCode}` + new OError( + `doc updater returned a non-success status code: ${res.statusCode}`, + { + projectId, + url + } ) ) } diff --git a/services/web/app/src/Features/Exports/ExportsHandler.js b/services/web/app/src/Features/Exports/ExportsHandler.js index a4753cee65..8e748e7b63 100644 --- a/services/web/app/src/Features/Exports/ExportsHandler.js +++ b/services/web/app/src/Features/Exports/ExportsHandler.js @@ -15,6 +15,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let ExportsHandler, self +const OError = require('@overleaf/o-error') const ProjectGetter = require('../Project/ProjectGetter') const ProjectHistoryHandler = require('../Project/ProjectHistoryHandler') const ProjectLocator = require('../Project/ProjectLocator') @@ -104,17 +105,19 @@ module.exports = ExportsHandler = self = { return async.auto(jobs, function(err, results) { if (err != null) { - logger.warn( - { err, project_id, user_id, brand_variation_id }, - 'error building project export' - ) + OError.tag(err, 'error building project export', { + project_id, + user_id, + brand_variation_id + }) return callback(err) } const { project, rootDoc, user, historyVersion } = results if (!rootDoc || rootDoc[1] == null) { - err = new Error('cannot export project without root doc') - logger.warn({ err, project_id }) + err = new OError('cannot export project without root doc', { + project_id + }) return callback(err) } @@ -175,10 +178,9 @@ module.exports = ExportsHandler = self = { }, function(err, res, body) { if (err != null) { - logger.warn( - { err, export: export_data }, - 'error making request to v1 export' - ) + OError.tag(err, 'error making request to v1 export', { + export: export_data + }) return callback(err) } else if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null, body) @@ -207,24 +209,18 @@ module.exports = ExportsHandler = self = { }, function(err, res, body) { if (err != null) { - logger.warn( - { err, project_id }, - 'error making request to project history' - ) + OError.tag(err, 'error making request to project history', { + project_id + }) return callback(err) } else if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null, body.version) } else { - err = new Error( + err = new OError( `project history version returned a failure status code: ${ res.statusCode - }` - ) - logger.warn( - { err, project_id }, - `project history version returned failure status code: ${ - res.statusCode - }` + }`, + { project_id } ) return callback(err) } @@ -243,20 +239,16 @@ module.exports = ExportsHandler = self = { }, function(err, res, body) { if (err != null) { - logger.warn( - { err, export: export_id }, - 'error making request to v1 export' - ) + OError.tag(err, 'error making request to v1 export', { + export: export_id + }) return callback(err) } else if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null, body) } else { - err = new Error( - `v1 export returned a failure status code: ${res.statusCode}` - ) - logger.warn( - { err, export: export_id }, - `v1 export returned failure status code: ${res.statusCode}` + err = new OError( + `v1 export returned a failure status code: ${res.statusCode}`, + { export: export_id } ) return callback(err) } @@ -277,22 +269,16 @@ module.exports = ExportsHandler = self = { }, function(err, res, body) { if (err != null) { - logger.warn( - { err, export: export_id }, - 'error making request to v1 export' - ) + OError.tag(err, 'error making request to v1 export', { + export: export_id + }) return callback(err) } else if (res.statusCode >= 200 && res.statusCode < 300) { return callback(null, body) } else { - err = new Error( - `v1 export returned a failure status code: ${res.statusCode}` - ) - logger.warn( - { err, export: export_id }, - `v1 export zip fetch returned failure status code: ${ - res.statusCode - }` + err = new OError( + `v1 export returned a failure status code: ${res.statusCode}`, + { export: export_id } ) return callback(err) } diff --git a/services/web/app/src/Features/FileStore/FileStoreHandler.js b/services/web/app/src/Features/FileStore/FileStoreHandler.js index 27696f52c6..c77293b27e 100644 --- a/services/web/app/src/Features/FileStore/FileStoreHandler.js +++ b/services/web/app/src/Features/FileStore/FileStoreHandler.js @@ -47,10 +47,10 @@ const FileStoreHandler = { ), function(err, result) { if (err) { - logger.warn( - { err, projectId, fileArgs }, - 'Error uploading file, retries failed' - ) + OError.tag(err, 'Error uploading file, retries failed', { + projectId, + fileArgs + }) return callback(err) } callback(err, result.url, result.fileRef) @@ -96,14 +96,11 @@ const FileStoreHandler = { }) writeStream.on('response', function(response) { if (![200, 201].includes(response.statusCode)) { - err = new Error( + err = new OError( `non-ok response from filestore for upload: ${ response.statusCode - }` - ) - logger.warn( - { err, statusCode: response.statusCode }, - 'error uploading to filestore' + }`, + { statusCode: response.statusCode } ) return callbackOnce(err) } @@ -145,10 +142,10 @@ const FileStoreHandler = { const url = this._buildUrl(projectId, fileId) request.head(url, (err, res) => { if (err) { - logger.warn( - { err, projectId, fileId }, - 'failed to get file size from filestore' - ) + OError.tag(err, 'failed to get file size from filestore', { + projectId, + fileId + }) return callback(err) } if (res.statusCode === 404) { @@ -224,21 +221,27 @@ const FileStoreHandler = { } return request(opts, function(err, response) { if (err) { - logger.warn( - { err, oldProjectId, oldFileId, newProjectId, newFileId }, - 'something went wrong telling filestore api to copy file' + OError.tag( + err, + 'something went wrong telling filestore api to copy file', + { + oldProjectId, + oldFileId, + newProjectId, + newFileId + } ) return callback(err) } else if (response.statusCode >= 200 && response.statusCode < 300) { // successful response return callback(null, opts.uri) } else { - err = new Error( - `non-ok response from filestore for copyFile: ${response.statusCode}` - ) - logger.warn( - { uri: opts.uri, statusCode: response.statusCode }, - 'error uploading to filestore' + err = new OError( + `non-ok response from filestore for copyFile: ${response.statusCode}`, + { + uri: opts.uri, + statusCode: response.statusCode + } ) return callback(err) } diff --git a/services/web/app/src/Features/InactiveData/InactiveProjectManager.js b/services/web/app/src/Features/InactiveData/InactiveProjectManager.js index 3b60273881..2ea6f2a2a5 100644 --- a/services/web/app/src/Features/InactiveData/InactiveProjectManager.js +++ b/services/web/app/src/Features/InactiveData/InactiveProjectManager.js @@ -11,6 +11,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let InactiveProjectManager +const OError = require('@overleaf/o-error') const async = require('async') const _ = require('underscore') const logger = require('logger-sharelatex') @@ -27,7 +28,9 @@ module.exports = InactiveProjectManager = { project ) { if (err != null) { - logger.warn({ err, project_id }, 'error getting project') + OError.tag(err, 'error getting project', { + project_id + }) return callback(err) } logger.log( @@ -41,10 +44,9 @@ module.exports = InactiveProjectManager = { return DocstoreManager.unarchiveProject(project_id, function(err) { if (err != null) { - logger.warn( - { err, project_id }, - 'error reactivating project in docstore' - ) + OError.tag(err, 'error reactivating project in docstore', { + project_id + }) return callback(err) } return ProjectUpdateHandler.markAsActive(project_id, callback) diff --git a/services/web/app/src/Features/Project/ProjectCreationHandler.js b/services/web/app/src/Features/Project/ProjectCreationHandler.js index 6c9817add0..bde8e98f48 100644 --- a/services/web/app/src/Features/Project/ProjectCreationHandler.js +++ b/services/web/app/src/Features/Project/ProjectCreationHandler.js @@ -13,6 +13,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const logger = require('logger-sharelatex') +const OError = require('@overleaf/o-error') const async = require('async') const metrics = require('metrics-sharelatex') const Settings = require('settings-sharelatex') @@ -260,10 +261,7 @@ const ProjectCreationHandler = { owner_id, function(error, doc) { if (error != null) { - logger.warn( - { err: error }, - 'error adding root doc when creating project' - ) + OError.tag(error, 'error adding root doc when creating project') return callback(error) } return ProjectEntityUpdateHandler.setRootDoc( diff --git a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js index 19d8136025..e1c02fd09e 100644 --- a/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityUpdateHandler.js @@ -1,4 +1,5 @@ const _ = require('lodash') +const OError = require('@overleaf/o-error') const async = require('async') const logger = require('logger-sharelatex') const Settings = require('settings-sharelatex') @@ -128,10 +129,10 @@ const ProjectEntityUpdateHandler = { ranges, (err, modified, rev) => { if (err != null) { - logger.warn( - { err, docId, projectId }, - 'error sending doc to docstore' - ) + OError.tag(err, 'error sending doc to docstore', { + docId, + projectId + }) return callback(err) } logger.log( @@ -215,16 +216,12 @@ const ProjectEntityUpdateHandler = { doc, (err, result, project) => { if (err != null) { - logger.warn( - { - err, - projectId, - folderId, - doc_name: doc != null ? doc.name : undefined, - doc_id: doc != null ? doc._id : undefined - }, - 'error adding file with project' - ) + OError.tag(err, 'error adding file with project', { + projectId, + folderId, + doc_name: doc != null ? doc.name : undefined, + doc_id: doc != null ? doc._id : undefined + }) return callback(err) } TpdsUpdateSender.addDoc( @@ -360,10 +357,12 @@ const ProjectEntityUpdateHandler = { fsPath, (err, fileStoreUrl, fileRef) => { if (err != null) { - logger.warn( - { err, projectId, folderId, file_name: fileName, fileRef }, - 'error uploading image to s3' - ) + OError.tag(err, 'error uploading image to s3', { + projectId, + folderId, + file_name: fileName, + fileRef + }) return callback(err) } callback(null, fileStoreUrl, fileRef) @@ -378,10 +377,12 @@ const ProjectEntityUpdateHandler = { fileRef, (err, result, project) => { if (err != null) { - logger.warn( - { err, projectId, folderId, file_name: fileRef.name, fileRef }, - 'error adding file with project' - ) + OError.tag(err, 'error adding file with project', { + projectId, + folderId, + file_name: fileRef.name, + fileRef + }) return callback(err) } TpdsUpdateSender.addFile( diff --git a/services/web/app/src/Features/Project/ProjectGetter.js b/services/web/app/src/Features/Project/ProjectGetter.js index 0fc426675c..8f41678fe9 100644 --- a/services/web/app/src/Features/Project/ProjectGetter.js +++ b/services/web/app/src/Features/Project/ProjectGetter.js @@ -14,6 +14,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const mongojs = require('../../infrastructure/mongojs') +const OError = require('@overleaf/o-error') const metrics = require('metrics-sharelatex') const { db } = mongojs const { ObjectId } = mongojs @@ -119,7 +120,10 @@ const ProjectGetter = { return db.projects.find(query, projection, function(err, project) { if (err != null) { - logger.warn({ err, query, projection }, 'error getting project') + OError.tag(err, 'error getting project', { + query, + projection + }) return callback(err) } return callback(null, project != null ? project[0] : undefined) diff --git a/services/web/app/src/Features/Referal/ReferalAllocator.js b/services/web/app/src/Features/Referal/ReferalAllocator.js index 25902188d4..aee65d9e6c 100644 --- a/services/web/app/src/Features/Referal/ReferalAllocator.js +++ b/services/web/app/src/Features/Referal/ReferalAllocator.js @@ -1,4 +1,4 @@ -const logger = require('logger-sharelatex') +const OError = require('@overleaf/o-error') const { User } = require('../../models/User') const FeaturesUpdater = require('../Subscription/FeaturesUpdater') @@ -34,10 +34,10 @@ module.exports = { {}, function(err) { if (err != null) { - logger.warn( - { err, referalId, newUserId }, - 'something went wrong allocating referal' - ) + OError.tag(err, 'something went wrong allocating referal', { + referalId, + newUserId + }) return callback(err) } FeaturesUpdater.refreshFeatures(user._id, callback) diff --git a/services/web/app/src/Features/References/ReferencesHandler.js b/services/web/app/src/Features/References/ReferencesHandler.js index 47ab7eb36c..31c92ad267 100644 --- a/services/web/app/src/Features/References/ReferencesHandler.js +++ b/services/web/app/src/Features/References/ReferencesHandler.js @@ -13,6 +13,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let ReferencesHandler +const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const request = require('request') const settings = require('settings-sharelatex') @@ -104,7 +105,9 @@ module.exports = ReferencesHandler = { { rootFolder: true, owner_ref: 1 }, function(err, project) { if (err) { - logger.warn({ err, projectId }, 'error finding project') + OError.tag(err, 'error finding project', { + projectId + }) return callback(err) } logger.log({ projectId }, 'indexing all bib files in project') @@ -130,7 +133,9 @@ module.exports = ReferencesHandler = { { rootFolder: true, owner_ref: 1 }, function(err, project) { if (err) { - logger.warn({ err, projectId }, 'error finding project') + OError.tag(err, 'error finding project', { + projectId + }) return callback(err) } return ReferencesHandler._doIndexOperation( @@ -150,10 +155,9 @@ module.exports = ReferencesHandler = { } return ReferencesHandler._isFullIndex(project, function(err, isFullIndex) { if (err) { - logger.warn( - { err, projectId }, - 'error checking whether to do full index' - ) + OError.tag(err, 'error checking whether to do full index', { + projectId + }) return callback(err) } logger.log( @@ -167,10 +171,10 @@ module.exports = ReferencesHandler = { function(err) { // continue if (err) { - logger.warn( - { err, projectId, docIds }, - 'error flushing docs to mongo' - ) + OError.tag(err, 'error flushing docs to mongo', { + projectId, + docIds + }) return callback(err) } const bibDocUrls = docIds.map(docId => @@ -190,10 +194,9 @@ module.exports = ReferencesHandler = { }, function(err, res, data) { if (err) { - logger.warn( - { err, projectId }, - 'error communicating with references api' - ) + OError.tag(err, 'error communicating with references api', { + projectId + }) return callback(err) } if (res.statusCode >= 200 && res.statusCode < 300) { diff --git a/services/web/app/src/Features/Subscription/FeaturesUpdater.js b/services/web/app/src/Features/Subscription/FeaturesUpdater.js index 77c5383001..d96da4ba17 100644 --- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/FeaturesUpdater.js @@ -1,4 +1,5 @@ const async = require('async') +const OError = require('@overleaf/o-error') const PlansLocator = require('./PlansLocator') const _ = require('underscore') const SubscriptionLocator = require('./SubscriptionLocator') @@ -47,9 +48,12 @@ const FeaturesUpdater = { } async.series(jobs, function(err, results) { if (err) { - logger.warn( - { err, userId }, - 'error getting subscription or group for refreshFeatures' + OError.tag( + err, + 'error getting subscription or group for refreshFeatures', + { + userId + } ) return callback(err) } @@ -287,7 +291,9 @@ const FeaturesUpdater = { user ) { if (err != null) { - logger.warn({ v1UserId }, '[AccountSync] error getting user') + OError.tag(err, '[AccountSync] error getting user', { + v1UserId + }) return callback(err) } if ((user != null ? user._id : undefined) == null) { diff --git a/services/web/app/src/Features/Subscription/LimitationsManager.js b/services/web/app/src/Features/Subscription/LimitationsManager.js index fc75292a2f..2b8f3ed87e 100644 --- a/services/web/app/src/Features/Subscription/LimitationsManager.js +++ b/services/web/app/src/Features/Subscription/LimitationsManager.js @@ -13,6 +13,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let LimitationsManager +const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const ProjectGetter = require('../Project/ProjectGetter') const UserGetter = require('../User/UserGetter') @@ -224,7 +225,9 @@ module.exports = LimitationsManager = { subscription ) { if (err != null) { - logger.warn({ err, subscriptionId }, 'error getting subscription') + OError.tag(err, 'error getting subscription', { + subscriptionId + }) return callback(err) } if (subscription == null) { diff --git a/services/web/app/src/Features/Subscription/RecurlyWrapper.js b/services/web/app/src/Features/Subscription/RecurlyWrapper.js index 6ec1451d4e..d84b00c686 100644 --- a/services/web/app/src/Features/Subscription/RecurlyWrapper.js +++ b/services/web/app/src/Features/Subscription/RecurlyWrapper.js @@ -17,6 +17,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let RecurlyWrapper +const OError = require('@overleaf/o-error') const querystring = require('querystring') const crypto = require('crypto') const request = require('request') @@ -46,9 +47,12 @@ module.exports = RecurlyWrapper = { }, function(error, response, responseBody) { if (error) { - logger.warn( - { error, user_id: user._id }, - 'error response from recurly while checking account' + OError.tag( + error, + 'error response from recurly while checking account', + { + user_id: user._id + } ) return next(error) } @@ -67,7 +71,9 @@ module.exports = RecurlyWrapper = { account ) { if (err) { - logger.warn({ err, user_id: user._id }, 'error parsing account') + OError.tag(err, 'error parsing account', { + user_id: user._id + }) return next(err) } cache.userExists = true @@ -107,9 +113,12 @@ module.exports = RecurlyWrapper = { }, (error, response, responseBody) => { if (error) { - logger.warn( - { error, user_id: user._id }, - 'error response from recurly while creating account' + OError.tag( + error, + 'error response from recurly while creating account', + { + user_id: user._id + } ) return next(error) } @@ -118,7 +127,9 @@ module.exports = RecurlyWrapper = { account ) { if (err) { - logger.warn({ err, user_id: user._id }, 'error creating account') + OError.tag(err, 'error creating account', { + user_id: user._id + }) return next(err) } cache.account = account @@ -149,9 +160,12 @@ module.exports = RecurlyWrapper = { }, (error, response, responseBody) => { if (error) { - logger.warn( - { error, user_id: user._id }, - 'error response from recurly while creating billing info' + OError.tag( + error, + 'error response from recurly while creating billing info', + { + user_id: user._id + } ) return next(error) } @@ -160,10 +174,10 @@ module.exports = RecurlyWrapper = { billingInfo ) { if (err) { - logger.warn( - { err, user_id: user._id, accountCode }, - 'error creating billing info' - ) + OError.tag(err, 'error creating billing info', { + user_id: user._id, + accountCode + }) return next(err) } cache.billingInfo = billingInfo @@ -212,9 +226,12 @@ module.exports = RecurlyWrapper = { }, (error, response, responseBody) => { if (error) { - logger.warn( - { error, user_id: user._id }, - 'error response from recurly while setting address' + OError.tag( + error, + 'error response from recurly while setting address', + { + user_id: user._id + } ) return next(error) } @@ -223,10 +240,9 @@ module.exports = RecurlyWrapper = { billingInfo ) { if (err) { - logger.warn( - { err, user_id: user._id }, - 'error updating billing info' - ) + OError.tag(err, 'error updating billing info', { + user_id: user._id + }) return next(err) } cache.billingInfo = billingInfo @@ -263,9 +279,12 @@ module.exports = RecurlyWrapper = { }, (error, response, responseBody) => { if (error) { - logger.warn( - { error, user_id: user._id }, - 'error response from recurly while creating subscription' + OError.tag( + error, + 'error response from recurly while creating subscription', + { + user_id: user._id + } ) return next(error) } @@ -274,10 +293,9 @@ module.exports = RecurlyWrapper = { subscription ) { if (err) { - logger.warn( - { err, user_id: user._id }, - 'error creating subscription' - ) + OError.tag(err, 'error creating subscription', { + user_id: user._id + }) return next(err) } cache.subscription = subscription @@ -312,18 +330,16 @@ module.exports = RecurlyWrapper = { ], function(err, result) { if (err) { - logger.warn( - { err, user_id: user._id }, - 'error in paypal subscription creation process' - ) + OError.tag(err, 'error in paypal subscription creation process', { + user_id: user._id + }) return callback(err) } if (!result.subscription) { err = new Error('no subscription object in result') - logger.warn( - { err, user_id: user._id }, - 'error in paypal subscription creation process' - ) + OError.tag(err, 'error in paypal subscription creation process', { + user_id: user._id + }) return callback(err) } logger.log( diff --git a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js index be0d490d61..2b8a6c0335 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js +++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js @@ -1,11 +1,11 @@ const { db } = require('../../infrastructure/mongojs') +const OError = require('@overleaf/o-error') const async = require('async') const { promisifyAll } = require('../../util/promises') const { Subscription } = require('../../models/Subscription') const SubscriptionLocator = require('./SubscriptionLocator') const UserGetter = require('../User/UserGetter') const PlansLocator = require('./PlansLocator') -const logger = require('logger-sharelatex') const { ObjectId } = require('mongoose').Types const FeaturesUpdater = require('./FeaturesUpdater') const { DeletedSubscription } = require('../../models/DeletedSubscription') @@ -107,10 +107,10 @@ const SubscriptionUpdater = { const removeOperation = { $pull: { member_ids: userId } } Subscription.updateMany(filter, removeOperation, function(err) { if (err != null) { - logger.warn( - { err, filter, removeOperation }, - 'error removing user from groups' - ) + OError.tag(err, 'error removing user from groups', { + filter, + removeOperation + }) return callback(err) } UserGetter.getUser(userId, function(error, user) { diff --git a/services/web/app/src/Features/SudoMode/SudoModeMiddleware.js b/services/web/app/src/Features/SudoMode/SudoModeMiddleware.js index b86ae9d51f..acf80b224b 100644 --- a/services/web/app/src/Features/SudoMode/SudoModeMiddleware.js +++ b/services/web/app/src/Features/SudoMode/SudoModeMiddleware.js @@ -12,6 +12,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let SudoModeMiddleware +const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const SudoModeHandler = require('./SudoModeHandler') const AuthenticationController = require('../Authentication/AuthenticationController') @@ -33,10 +34,9 @@ module.exports = SudoModeMiddleware = { ) return SudoModeHandler.isSudoModeActive(userId, function(err, isActive) { if (err != null) { - logger.warn( - { err, userId }, - '[SudoMode] error checking if sudo mode is active' - ) + OError.tag(err, '[SudoMode] error checking if sudo mode is active', { + userId + }) return next(err) } if (isActive) { diff --git a/services/web/app/src/Features/Templates/TemplatesManager.js b/services/web/app/src/Features/Templates/TemplatesManager.js index f18d2d69cc..ff4c5b3190 100644 --- a/services/web/app/src/Features/Templates/TemplatesManager.js +++ b/services/web/app/src/Features/Templates/TemplatesManager.js @@ -11,6 +11,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const { Project } = require('../../models/Project') +const OError = require('@overleaf/o-error') const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler') const ProjectOptionsHandler = require('../Project/ProjectOptionsHandler') const ProjectRootDocManager = require('../Project/ProjectRootDocManager') @@ -81,7 +82,9 @@ const TemplatesManager = { attributes, function(err, project) { if (err != null) { - logger.warn({ err, zipReq }, 'problem building project from zip') + OError.tag(err, 'problem building project from zip', { + zipReq + }) return callback(err) } return async.series( diff --git a/services/web/app/src/Features/ThirdPartyDataStore/UpdateMerger.js b/services/web/app/src/Features/ThirdPartyDataStore/UpdateMerger.js index cee2c66756..df693273f1 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/UpdateMerger.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/UpdateMerger.js @@ -12,6 +12,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let UpdateMerger +const OError = require('@overleaf/o-error') const _ = require('underscore') const async = require('async') const fs = require('fs') @@ -196,9 +197,12 @@ module.exports = UpdateMerger = { docLines ) { if (err != null) { - logger.warn( - { project_id }, - 'error reading file into text array for process doc update' + OError.tag( + err, + 'error reading file into text array for process doc update', + { + project_id + } ) return callback(err) } @@ -236,7 +240,9 @@ module.exports = UpdateMerger = { content = '' } if (error != null) { - logger.warn({ path }, 'error reading file into text array') + OError.tag(error, 'error reading file into text array', { + path + }) return callback(error) } const lines = content.split(/\r\n|\n|\r/) diff --git a/services/web/app/src/Features/Uploads/ArchiveManager.js b/services/web/app/src/Features/Uploads/ArchiveManager.js index 0c12c4a802..88b9b631be 100644 --- a/services/web/app/src/Features/Uploads/ArchiveManager.js +++ b/services/web/app/src/Features/Uploads/ArchiveManager.js @@ -13,6 +13,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const logger = require('logger-sharelatex') +const OError = require('@overleaf/o-error') const metrics = require('metrics-sharelatex') const fs = require('fs') const Path = require('path') @@ -165,10 +166,10 @@ const ArchiveManager = { destFile, function(err) { if (err != null) { - logger.warn( - { err, source, destFile }, - 'error unzipping file entry' - ) + OError.tag(err, 'error unzipping file entry', { + source, + destFile + }) zipfile.close() // bail out, stop reading file entries return callback(err) } else { @@ -205,7 +206,7 @@ const ArchiveManager = { return ArchiveManager._isZipTooLarge(source, function(err, isTooLarge) { if (err != null) { - logger.warn({ err }, 'error checking size of zip file') + OError.tag(err, 'error checking size of zip file') return callback(err) } @@ -221,7 +222,10 @@ const ArchiveManager = { ) { timer.done() if (err != null) { - logger.warn({ err, source, destination }, 'unzip failed') + OError.tag(err, 'unzip failed', { + source, + destination + }) return callback(err) } else { return callback() diff --git a/services/web/app/src/Features/User/ThirdPartyIdentityManager.js b/services/web/app/src/Features/User/ThirdPartyIdentityManager.js index d3514597ab..4fd91545bf 100644 --- a/services/web/app/src/Features/User/ThirdPartyIdentityManager.js +++ b/services/web/app/src/Features/User/ThirdPartyIdentityManager.js @@ -1,4 +1,5 @@ const APP_ROOT = '../../../../app/src' +const OError = require('@overleaf/o-error') const EmailHandler = require(`${APP_ROOT}/Features/Email/EmailHandler`) const Errors = require('../Errors/Errors') const _ = require('lodash') @@ -98,7 +99,7 @@ function link( } EmailHandler.sendEmail('securityAlert', emailOptions, error => { if (error != null) { - logger.warn(error) + logger.warn(OError.tag(error)) } return callback(null, res) }) diff --git a/services/web/app/src/Features/User/UserSessionsManager.js b/services/web/app/src/Features/User/UserSessionsManager.js index e4196c63f4..bd85e9193a 100644 --- a/services/web/app/src/Features/User/UserSessionsManager.js +++ b/services/web/app/src/Features/User/UserSessionsManager.js @@ -1,4 +1,5 @@ let UserSessionsManager +const OError = require('@overleaf/o-error') const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') const Async = require('async') @@ -28,9 +29,13 @@ UserSessionsManager = { .pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds .exec(function(err, response) { if (err) { - logger.warn( - { err, user_id: user._id, sessionSetKey }, - 'error while adding session key to UserSessions set' + OError.tag( + err, + 'error while adding session key to UserSessions set', + { + user_id: user._id, + sessionSetKey + } ) return callback(err) } @@ -57,9 +62,13 @@ UserSessionsManager = { .pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds .exec(function(err, response) { if (err) { - logger.warn( - { err, user_id: user._id, sessionSetKey }, - 'error while removing session key from UserSessions set' + OError.tag( + err, + 'error while removing session key from UserSessions set', + { + user_id: user._id, + sessionSetKey + } ) return callback(err) } @@ -73,10 +82,9 @@ UserSessionsManager = { const sessionSetKey = UserSessionsRedis.sessionSetKey(user) rclient.smembers(sessionSetKey, function(err, sessionKeys) { if (err) { - logger.warn( - { user_id: user._id }, - 'error getting all session keys for user from redis' - ) + OError.tag(err, 'error getting all session keys for user from redis', { + user_id: user._id + }) return callback(err) } sessionKeys = _.filter(sessionKeys, k => !_.contains(exclude, k)) @@ -90,10 +98,9 @@ UserSessionsManager = { sessions ) { if (err) { - logger.warn( - { user_id: user._id }, - 'error getting all sessions for user from redis' - ) + OError.tag(err, 'error getting all sessions for user from redis', { + user_id: user._id + }) return callback(err) } @@ -130,10 +137,10 @@ UserSessionsManager = { const sessionSetKey = UserSessionsRedis.sessionSetKey(user) rclient.smembers(sessionSetKey, function(err, sessionKeys) { if (err) { - logger.warn( - { err, user_id: user._id, sessionSetKey }, - 'error getting contents of UserSessions set' - ) + OError.tag(err, 'error getting contents of UserSessions set', { + user_id: user._id, + sessionSetKey + }) return callback(err) } const keysToDelete = _.filter( @@ -156,18 +163,18 @@ UserSessionsManager = { Async.series(deletions, function(err, _result) { if (err) { - logger.warn( - { err, user_id: user._id, sessionSetKey }, - 'errror revoking all sessions for user' - ) + OError.tag(err, 'error revoking all sessions for user', { + user_id: user._id, + sessionSetKey + }) return callback(err) } rclient.srem(sessionSetKey, keysToDelete, function(err) { if (err) { - logger.warn( - { err, user_id: user._id, sessionSetKey }, - 'error removing session set for user' - ) + OError.tag(err, 'error removing session set for user', { + user_id: user._id, + sessionSetKey + }) return callback(err) } callback(null) @@ -186,10 +193,9 @@ UserSessionsManager = { `${Settings.cookieSessionLength}`, // in milliseconds function(err, response) { if (err) { - logger.warn( - { err, user_id: user._id }, - 'error while updating ttl on UserSessions set' - ) + OError.tag(err, 'error while updating ttl on UserSessions set', { + user_id: user._id + }) return callback(err) } callback(null) @@ -204,10 +210,10 @@ UserSessionsManager = { const sessionSetKey = UserSessionsRedis.sessionSetKey(user) rclient.smembers(sessionSetKey, function(err, sessionKeys) { if (err) { - logger.warn( - { err, user_id: user._id, sessionSetKey }, - 'error getting contents of UserSessions set' - ) + OError.tag(err, 'error getting contents of UserSessions set', { + user_id: user._id, + sessionSetKey + }) return callback(err) } Async.series( diff --git a/services/web/app/src/Features/User/UserUpdater.js b/services/web/app/src/Features/User/UserUpdater.js index 6e0dc27ebe..24eedb18c5 100644 --- a/services/web/app/src/Features/User/UserUpdater.js +++ b/services/web/app/src/Features/User/UserUpdater.js @@ -1,4 +1,5 @@ const logger = require('logger-sharelatex') +const OError = require('@overleaf/o-error') const mongojs = require('../../infrastructure/mongojs') const metrics = require('metrics-sharelatex') const { db } = mongojs @@ -26,15 +27,21 @@ const UserUpdater = { UserUpdater.updateUser( { _id: userId, 'emails.email': email }, { $unset: { 'emails.$.affiliationUnchecked': 1 } }, - (error, updated) => { - if (error || (updated && updated.nModified === 0)) { - logger.error( - { userId, email }, - 'could not remove affiliationUnchecked flag for user on create' + error => { + if (error) { + callback( + OError.tag( + error, + 'could not remove affiliationUnchecked flag for user on create', + { + userId, + email + } + ) ) - return callback(error) + } else { + callback() } - return callback(error, updated) } ) }) @@ -104,10 +111,7 @@ const UserUpdater = { addAffiliation(userId, newEmail, affiliationOptions, error => { if (error != null) { - logger.warn( - { error }, - 'problem adding affiliation while adding email' - ) + OError.tag(error, 'problem adding affiliation while adding email') return callback(error) } @@ -123,7 +127,7 @@ const UserUpdater = { } UserUpdater.updateUser(userId, update, error => { if (error != null) { - logger.warn({ error }, 'problem updating users emails') + OError.tag(error, 'problem updating users emails') return callback(error) } callback() @@ -141,7 +145,7 @@ const UserUpdater = { } removeAffiliation(userId, email, error => { if (error != null) { - logger.warn({ error }, 'problem removing affiliation') + OError.tag(error, 'problem removing affiliation') return callback(error) } @@ -149,7 +153,7 @@ const UserUpdater = { const update = { $pull: { emails: { email } } } UserUpdater.updateUser(query, update, (error, res) => { if (error != null) { - logger.warn({ error }, 'problem removing users email') + OError.tag(error, 'problem removing users email') return callback(error) } if (res.n === 0) { @@ -224,10 +228,7 @@ const UserUpdater = { logger.log({ userId, email }, 'confirming user email') addAffiliation(userId, email, { confirmedAt }, error => { if (error != null) { - logger.warn( - { error }, - 'problem adding affiliation while confirming email' - ) + OError.tag(error, 'problem adding affiliation while confirming email') return callback(error) } diff --git a/services/web/app/src/Features/V1/V1Handler.js b/services/web/app/src/Features/V1/V1Handler.js index b2caef269b..b81f94f939 100644 --- a/services/web/app/src/Features/V1/V1Handler.js +++ b/services/web/app/src/Features/V1/V1Handler.js @@ -14,6 +14,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let V1Handler +const OError = require('@overleaf/o-error') const V1Api = require('./V1Api') const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') @@ -32,10 +33,9 @@ module.exports = V1Handler = { }, function(err, response, body) { if (err != null) { - logger.warn( - { email, err }, - '[V1Handler] error while talking to v1 login api' - ) + OError.tag(err, '[V1Handler] error while talking to v1 login api', { + email + }) return callback(err) } if ([200, 403].includes(response.statusCode)) { @@ -80,10 +80,9 @@ module.exports = V1Handler = { }, function(err, response, body) { if (err != null) { - logger.warn( - { v1_user_id, err }, - 'error while talking to v1 password reset api' - ) + OError.tag(err, 'error while talking to v1 password reset api', { + v1_user_id + }) return callback(err, false) } if ([200].includes(response.statusCode)) { diff --git a/services/web/app/src/infrastructure/FileWriter.js b/services/web/app/src/infrastructure/FileWriter.js index 42fcfb14a1..905d62395e 100644 --- a/services/web/app/src/infrastructure/FileWriter.js +++ b/services/web/app/src/infrastructure/FileWriter.js @@ -11,6 +11,7 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ const fs = require('fs') +const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const uuid = require('uuid') const _ = require('underscore') @@ -141,9 +142,13 @@ const FileWriter = { }).withCause(err || {}) } if (err) { - logger.warn( - { err, identifier, fsPath }, - '[writeStreamToDisk] something went wrong writing the stream to disk' + OError.tag( + err, + '[writeStreamToDisk] something went wrong writing the stream to disk', + { + identifier, + fsPath + } ) return callback(err) } diff --git a/services/web/test/unit/src/Contact/ContactManagerTests.js b/services/web/test/unit/src/Contact/ContactManagerTests.js index 373c8a92ca..073640005f 100644 --- a/services/web/test/unit/src/Contact/ContactManagerTests.js +++ b/services/web/test/unit/src/Contact/ContactManagerTests.js @@ -29,12 +29,6 @@ describe('ContactManager', function() { url: 'contacts.sharelatex.com' } } - }), - 'logger-sharelatex': (this.logger = { - log: sinon.stub(), - warn: sinon.stub(), - error: sinon.stub(), - err() {} }) } }) @@ -108,23 +102,6 @@ describe('ContactManager', function() { ) .should.equal(true) }) - - it('should log the error', function() { - this.logger.warn.should.have.been.calledWith( - { - err: sinon.match - .instanceOf(Error) - .and( - sinon.match.has( - 'message', - 'contacts api responded with non-success code: 500' - ) - ), - user_id: this.user_id - }, - 'error getting contacts for user' - ) - }) }) }) @@ -186,26 +163,6 @@ describe('ContactManager', function() { ) .should.equal(true) }) - - it('should log the error', function() { - return this.logger.warn - .calledWith( - { - err: sinon.match - .instanceOf(Error) - .and( - sinon.match.has( - 'message', - 'contacts api responded with non-success code: 500' - ) - ), - user_id: this.user_id, - contact_id: this.contact_id - }, - 'error adding contact for user' - ) - .should.equal(true) - }) }) }) }) diff --git a/services/web/test/unit/src/FileStore/FileStoreHandlerTests.js b/services/web/test/unit/src/FileStore/FileStoreHandlerTests.js index ac55b1a3d2..4a6e785010 100644 --- a/services/web/test/unit/src/FileStore/FileStoreHandlerTests.js +++ b/services/web/test/unit/src/FileStore/FileStoreHandlerTests.js @@ -496,7 +496,7 @@ describe('FileStoreHandler', function() { }) it('should return the err', function(done) { - const error = 'errrror' + const error = new Error('error') this.request.callsArgWith(1, error) this.handler.copyFile( this.projectId, diff --git a/services/web/test/unit/src/InactiveData/InactiveProjectManagerTests.js b/services/web/test/unit/src/InactiveData/InactiveProjectManagerTests.js index cfca8a6d14..eb7a389ab9 100644 --- a/services/web/test/unit/src/InactiveData/InactiveProjectManagerTests.js +++ b/services/web/test/unit/src/InactiveData/InactiveProjectManagerTests.js @@ -79,12 +79,13 @@ describe('InactiveProjectManager', function() { ) }) - it('should not mark project as active if error with unarchinging', function(done) { - this.DocstoreManager.unarchiveProject.callsArgWith(1, 'error') + it('should not mark project as active if error with unarchiving', function(done) { + const error = new Error('error') + this.DocstoreManager.unarchiveProject.callsArgWith(1, error) return this.InactiveProjectManager.reactivateProjectIfRequired( this.project_id, err => { - err.should.equal('error') + err.should.equal(error) this.DocstoreManager.unarchiveProject .calledWith(this.project_id) .should.equal(true) diff --git a/services/web/test/unit/src/User/UserUpdaterTests.js b/services/web/test/unit/src/User/UserUpdaterTests.js index ab12ccd5a2..a2ef0fe57e 100644 --- a/services/web/test/unit/src/User/UserUpdaterTests.js +++ b/services/web/test/unit/src/User/UserUpdaterTests.js @@ -98,9 +98,8 @@ describe('UserUpdater', function() { this.UserUpdater.addAffiliationForNewUser( this.stubbedUser._id, this.newEmail, - (error, updated) => { + error => { should.not.exist(error) - expect(updated).to.deep.equal({ n: 1, nModified: 1, ok: 1 }) sinon.assert.calledOnce(this.UserUpdater.updateUser) sinon.assert.calledWithMatch( this.UserUpdater.updateUser, @@ -217,22 +216,6 @@ describe('UserUpdater', function() { ) }) - it('handle error', function(done) { - this.UserUpdater.updateUser = sinon - .stub() - .callsArgWith(2, new Error('nope')) - - this.UserUpdater.addEmailAddress( - this.stubbedUser._id, - this.newEmail, - err => { - this.logger.warn.called.should.equal(true) - should.exist(err) - done() - } - ) - }) - it('handle affiliation error', function(done) { this.addAffiliation.callsArgWith(3, new Error('nope')) this.UserUpdater.addEmailAddress( diff --git a/services/web/transform/o-error/transform.js b/services/web/transform/o-error/transform.js index fd4e8d712b..82e695b309 100644 --- a/services/web/transform/o-error/transform.js +++ b/services/web/transform/o-error/transform.js @@ -1,92 +1,83 @@ function functionArgsFilter(j, path) { - return ['err', 'error'].includes(path.get('params').value[0].name) + if (path.get('params') && path.get('params').value[0]) { + return ['err', 'error'].includes(path.get('params').value[0].name) + } else { + return false + } +} + +function isReturningFunctionCallWithError(path, errorVarName) { + return ( + path.value.argument && + path.value.argument.arguments && + path.value.argument.arguments[0] && + path.value.argument.arguments[0].name === errorVarName + ) +} + +function expressionIsLoggingError(path) { + return ['warn', 'error', 'err'].includes( + path.get('callee').get('property').value.name + ) +} + +function createTagErrorExpression(j, path, errorVarName) { + let message = 'error' + if (path.value.arguments.length >= 2) { + message = path.value.arguments[1].value || message + } + + let info + try { + info = j.objectExpression( + // add properties from original logger info object to the + // OError info object, filtering out the err object itself, + // which is typically one of the args when doing intermediate + // error logging + // TODO: this can fail when the property name does not match + // the variable name. e.g. { err: error } so need to check + // both in the filter + path + .get('arguments') + .value[0].properties.filter( + property => property.key.name !== errorVarName + ) + ) + } catch (error) { + // if info retrieval fails it remains empty + } + const args = [j.identifier(errorVarName), j.literal(message)] + if (info) { + args.push(info) + } + return j.callExpression( + j.memberExpression(j.identifier('OError'), j.identifier('tag')), + args + ) } function functionBodyProcessor(j, path) { // the error variable should be the first parameter to the function const errorVarName = path.get('params').value[0].name j(path) - // look for if statements - .find(j.IfStatement) - .filter(path => { - let hasReturnError = false + .find(j.IfStatement) // look for if statements + .filter(path => j(path) // find returns inside the if statement where the error from // the args is explicitly returned .find(j.ReturnStatement) - .forEach( - path => - (hasReturnError = - path.value.argument.arguments[0].name === errorVarName) - ) - return hasReturnError - }) + .some(path => isReturningFunctionCallWithError(path, errorVarName)) + ) .forEach(path => { j(path) - // within the selected if blocks find calls to logger .find(j.CallExpression, { callee: { object: { name: 'logger' } } }) - // handle logger.warn, logger.error and logger.err - .filter(path => - ['warn', 'error', 'err'].includes( - path.get('callee').get('property').value.name - ) - ) - // replace the logger call with the constructed OError wrapper + .filter(path => expressionIsLoggingError(path)) .replaceWith(path => { - // extract the error message which is the second arg for logger - const message = - path.value.arguments.length >= 2 - ? path.value.arguments[1].value - : 'Error' - // create: err = new OError(...) - return j.assignmentExpression( - '=', - // assign over the existing error var - j.identifier(errorVarName), - j.callExpression( - j.memberExpression( - // create: new OError - j.newExpression(j.identifier('OError'), [ - // create: { ... } args for new OError() - j.objectExpression([ - // set message property with original error message - j.property( - 'init', - j.identifier('message'), - j.literal(message) - ), - j.property( - 'init', - // set info property with object { info: {} } - j.identifier('info'), - j.objectExpression( - // add properties from original logger info object to the - // OError info object, filtering out the err object itself, - // which is typically one of the args when doing intermediate - // error logging - // TODO: this can fail when the property name does not match - // the variable name. e.g. { err: error } so need to check - // both in the filter - path - .get('arguments') - .value[0].properties.filter( - property => property.key.name !== errorVarName - ) - ) - ) - ]) - ]), - // add: .withCause( ) to OError - j.identifier('withCause') - ), - // add original error var as argument: .withCause(err) - [j.identifier(errorVarName)] - ) - ) + return createTagErrorExpression(j, path, errorVarName) }) }) }