Merge pull request #3114 from overleaf/msm-remove-logger-warn-controllers

Replaced logger statements with error tagging in Controllers

GitOrigin-RevId: c5231913c82f95a325f4c3ab406d89cb534835a4
This commit is contained in:
Jakob Ackermann 2020-08-19 11:11:32 +02:00 committed by Copybot
parent 674954f96f
commit a22e252666
17 changed files with 213 additions and 150 deletions

View file

@ -1,4 +1,5 @@
const AuthenticationManager = require('./AuthenticationManager')
const OError = require('@overleaf/o-error')
const LoginRateLimiter = require('../Security/LoginRateLimiter')
const UserUpdater = require('../User/UserUpdater')
const Metrics = require('metrics-sharelatex')
@ -471,7 +472,9 @@ function _afterLoginSessionSetup(req, user, callback) {
}
req.login(user, function(err) {
if (err) {
logger.warn({ user_id: user._id, err }, 'error from req.login')
OError.tag(err, 'error from req.login', {
user_id: user._id
})
return callback(err)
}
// Regenerate the session to get a new sessionID (cookie value) to
@ -479,10 +482,9 @@ function _afterLoginSessionSetup(req, user, callback) {
const oldSession = req.session
req.session.destroy(function(err) {
if (err) {
logger.warn(
{ user_id: user._id, err },
'error when trying to destroy old session'
)
OError.tag(err, 'error when trying to destroy old session', {
user_id: user._id
})
return callback(err)
}
req.sessionStore.generate(req)
@ -496,10 +498,9 @@ function _afterLoginSessionSetup(req, user, callback) {
}
req.session.save(function(err) {
if (err) {
logger.warn(
{ user_id: user._id },
'error saving regenerated session after login'
)
OError.tag(err, 'error saving regenerated session after login', {
user_id: user._id
})
return callback(err)
}
UserSessionsManager.trackSession(user, req.sessionID, function() {})

View file

@ -1,4 +1,5 @@
const BetaProgramHandler = require('./BetaProgramHandler')
const OError = require('@overleaf/o-error')
const UserGetter = require('../User/UserGetter')
const Settings = require('settings-sharelatex')
const logger = require('logger-sharelatex')
@ -38,7 +39,9 @@ const BetaProgramController = {
logger.log({ user_id: userId }, 'showing beta participation page for user')
UserGetter.getUser(userId, function(err, user) {
if (err) {
logger.warn({ err, userId }, 'error fetching user')
OError.tag(err, 'error fetching user', {
userId
})
return next(err)
}
res.render('beta_program/opt_in', {

View file

@ -13,6 +13,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let CollaboratorsInviteController
const OError = require('@overleaf/o-error')
const ProjectGetter = require('../Project/ProjectGetter')
const LimitationsManager = require('../Subscription/LimitationsManager')
const UserGetter = require('../User/UserGetter')
@ -37,7 +38,9 @@ module.exports = CollaboratorsInviteController = {
invites
) {
if (err != null) {
logger.warn({ projectId }, 'error getting invites for project')
OError.tag(err, 'error getting invites for project', {
projectId
})
return next(err)
}
return res.json({ invites })
@ -144,9 +147,14 @@ module.exports = CollaboratorsInviteController = {
email,
function(err, shouldAllowInvite) {
if (err != null) {
logger.warn(
{ err, email, projectId, sendingUserId },
'error checking if we can invite this email address'
OError.tag(
err,
'error checking if we can invite this email address',
{
email,
projectId,
sendingUserId
}
)
return next(err)
}
@ -167,10 +175,11 @@ module.exports = CollaboratorsInviteController = {
privileges,
function(err, invite) {
if (err != null) {
logger.warn(
{ projectId, email, sendingUserId },
'error creating project invite'
)
OError.tag(err, 'error creating project invite', {
projectId,
email,
sendingUserId
})
return next(err)
}
logger.log(
@ -202,7 +211,10 @@ module.exports = CollaboratorsInviteController = {
inviteId,
function(err) {
if (err != null) {
logger.warn({ projectId, inviteId }, 'error revoking invite')
OError.tag(err, 'error revoking invite', {
projectId,
inviteId
})
return next(err)
}
EditorRealTimeController.emitToRoom(
@ -235,7 +247,10 @@ module.exports = CollaboratorsInviteController = {
inviteId,
function(err) {
if (err != null) {
logger.warn({ projectId, inviteId }, 'error resending invite')
OError.tag(err, 'error resending invite', {
projectId,
inviteId
})
return next(err)
}
return res.sendStatus(201)
@ -262,10 +277,9 @@ module.exports = CollaboratorsInviteController = {
projectId,
function(err, isMember) {
if (err != null) {
logger.warn(
{ err, projectId },
'error checking if user is member of project'
)
OError.tag(err, 'error checking if user is member of project', {
projectId
})
return next(err)
}
if (isMember) {
@ -281,7 +295,10 @@ module.exports = CollaboratorsInviteController = {
token,
function(err, invite) {
if (err != null) {
logger.warn({ projectId, token }, 'error getting invite by token')
OError.tag(err, 'error getting invite by token', {
projectId,
token
})
return next(err)
}
// check if invite is gone, or otherwise non-existent
@ -295,7 +312,9 @@ module.exports = CollaboratorsInviteController = {
{ email: 1, first_name: 1, last_name: 1 },
function(err, owner) {
if (err != null) {
logger.warn({ err, projectId }, 'error getting project owner')
OError.tag(err, 'error getting project owner', {
projectId
})
return next(err)
}
if (owner == null) {
@ -308,7 +327,9 @@ module.exports = CollaboratorsInviteController = {
project
) {
if (err != null) {
logger.warn({ err, projectId }, 'error getting project')
OError.tag(err, 'error getting project', {
projectId
})
return next(err)
}
if (project == null) {
@ -345,7 +366,10 @@ module.exports = CollaboratorsInviteController = {
currentUser,
function(err) {
if (err != null) {
logger.warn({ projectId, token }, 'error accepting invite by token')
OError.tag(err, 'error accepting invite by token', {
projectId,
token
})
return next(err)
}
EditorRealTimeController.emitToRoom(

View file

@ -14,6 +14,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let CompileController
const OError = require('@overleaf/o-error')
const Metrics = require('metrics-sharelatex')
const ProjectGetter = require('../Project/ProjectGetter')
const CompileManager = require('./CompileManager')
@ -467,7 +468,7 @@ module.exports = CompileController = {
return ClsiCookieManager.getCookieJar(project_id, function(err, jar) {
let qs
if (err != null) {
logger.warn({ err }, 'error getting cookie jar for clsi request')
OError.tag(err, 'error getting cookie jar for clsi request')
return callback(err)
}
// expand any url parameter passed in as {url:..., qs:...}

View file

@ -13,6 +13,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const ProjectGetter = require('../Project/ProjectGetter')
const OError = require('@overleaf/o-error')
const ProjectLocator = require('../Project/ProjectLocator')
const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler')
@ -42,10 +43,10 @@ module.exports = {
{ project, element_id: doc_id, type: 'doc' },
function(error, doc, path) {
if (error != null) {
logger.warn(
{ err: error, doc_id, project_id },
'error finding element for getDocument'
)
OError.tag(error, 'error finding element for getDocument', {
doc_id,
project_id
})
return next(error)
}
return ProjectEntityHandler.getDoc(project_id, doc_id, function(
@ -56,9 +57,13 @@ module.exports = {
ranges
) {
if (error != null) {
logger.warn(
{ err: error, doc_id, project_id },
'error finding doc contents for getDocument'
OError.tag(
error,
'error finding doc contents for getDocument',
{
doc_id,
project_id
}
)
return next(error)
}
@ -106,10 +111,10 @@ module.exports = {
lastUpdatedBy,
function(error) {
if (error != null) {
logger.warn(
{ err: error, doc_id, project_id },
'error finding element for getDocument'
)
OError.tag(error, 'error finding element for getDocument', {
doc_id,
project_id
})
return next(error)
}
logger.log(

View file

@ -14,6 +14,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 sanitize = require('sanitizer')
const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandler')
@ -68,10 +69,10 @@ const EditorController = {
user_id,
(err, doc, folder_id) => {
if (err != null) {
logger.warn(
{ err, project_id, docName },
'error adding doc without lock'
)
OError.tag(err, 'error adding doc without lock', {
project_id,
docName
})
return callback(err)
}
EditorRealTimeController.emitToRoom(
@ -110,10 +111,11 @@ const EditorController = {
user_id,
(err, fileRef, folder_id) => {
if (err != null) {
logger.warn(
{ err, project_id, folder_id, fileName },
'error adding file without lock'
)
OError.tag(err, 'error adding file without lock', {
project_id,
folder_id,
fileName
})
return callback(err)
}
EditorRealTimeController.emitToRoom(
@ -314,10 +316,12 @@ const EditorController = {
folderName,
(err, folder, folder_id) => {
if (err != null) {
logger.warn(
{ err, project_id, folder_id, folderName, source },
'could not add folder'
)
OError.tag(err, 'could not add folder', {
project_id,
folder_id,
folderName,
source
})
return callback(err)
}
return EditorController._notifyProjectUsersOfNewFolder(
@ -345,7 +349,10 @@ const EditorController = {
path,
(err, newFolders, lastFolder) => {
if (err != null) {
logger.warn({ err, project_id, path }, 'could not mkdirp')
OError.tag(err, 'could not mkdirp', {
project_id,
path
})
return callback(err)
}
@ -375,10 +382,11 @@ const EditorController = {
userId,
function(err) {
if (err != null) {
logger.warn(
{ err, project_id, entity_id, entityType },
'could not delete entity'
)
OError.tag(err, 'could not delete entity', {
project_id,
entity_id,
entityType
})
return callback(err)
}
logger.log(
@ -426,9 +434,13 @@ const EditorController = {
description,
function(err) {
if (err != null) {
logger.warn(
{ err, project_id, description },
'something went wrong setting the project description'
OError.tag(
err,
'something went wrong setting the project description',
{
project_id,
description
}
)
return callback(err)
}
@ -461,10 +473,12 @@ const EditorController = {
userId,
function(err) {
if (err != null) {
logger.warn(
{ err, project_id, entity_id, entityType, newName },
'error renaming entity'
)
OError.tag(err, 'error renaming entity', {
project_id,
entity_id,
entityType,
newName
})
return callback(err)
}
if (newName.length > 0) {
@ -493,10 +507,11 @@ const EditorController = {
userId,
function(err) {
if (err != null) {
logger.warn(
{ err, project_id, entity_id, folder_id },
'error moving entity'
)
OError.tag(err, 'error moving entity', {
project_id,
entity_id,
folder_id
})
return callback(err)
}
EditorRealTimeController.emitToRoom(
@ -518,7 +533,10 @@ const EditorController = {
err
) {
if (err != null) {
logger.warn({ err, project_id, newName }, 'error renaming project')
OError.tag(err, 'error renaming project', {
project_id,
newName
})
return callback(err)
}
EditorRealTimeController.emitToRoom(

View file

@ -15,6 +15,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let HistoryController
const OError = require('@overleaf/o-error')
const _ = require('lodash')
const async = require('async')
const logger = require('logger-sharelatex')
@ -339,12 +340,6 @@ module.exports = HistoryController = {
error = new Error(
`history api responded with non-success code: ${response.statusCode}`
)
logger.warn(
{ err: error },
`project-history api responded with non-success code: ${
response.statusCode
}`
)
return callback(error)
}
})
@ -399,7 +394,10 @@ module.exports = HistoryController = {
}
return request(options, function(err, response, body) {
if (err) {
logger.warn({ err, v1_project_id, version }, 'history API error')
OError.tag(err, 'history API error', {
v1_project_id,
version
})
return next(err)
}
if (req.aborted) {
@ -468,10 +466,11 @@ module.exports = HistoryController = {
}, retryDelay),
function(err) {
if (err) {
logger.warn(
{ err, v1_project_id, version, retryAttempt },
'history s3 download failed'
)
OError.tag(err, 'history s3 download failed', {
v1_project_id,
version,
retryAttempt
})
return next(err)
}
}

View file

@ -1,4 +1,4 @@
const logger = require('logger-sharelatex')
const OError = require('@overleaf/o-error')
const UserGetter = require('../User/UserGetter')
const { addAffiliation } = require('../Institutions/InstitutionsAPI')
const FeaturesUpdater = require('../Subscription/FeaturesUpdater')
@ -28,7 +28,7 @@ var affiliateUsers = function(hostname, callback) {
users
) {
if (error) {
logger.warn({ error }, 'problem fetching users by hostname')
OError.tag(error, 'problem fetching users by hostname')
return callback(error)
}
@ -59,8 +59,8 @@ var affiliateUserByReversedHostname = function(
{ confirmedAt: email.confirmedAt },
error => {
if (error) {
logger.warn(
{ error },
OError.tag(
error,
'problem adding affiliation while confirming hostname'
)
return innerCallback(error)

View file

@ -12,6 +12,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let MetaController
const OError = require('@overleaf/o-error')
const EditorRealTimeController = require('../Editor/EditorRealTimeController')
const MetaHandler = require('./MetaHandler')
const logger = require('logger-sharelatex')
@ -25,9 +26,12 @@ module.exports = MetaController = {
projectMeta
) {
if (err != null) {
logger.warn(
{ project_id, err },
'[MetaController] error getting all labels from project'
OError.tag(
err,
'[MetaController] error getting all labels from project',
{
project_id
}
)
return next(err)
}
@ -44,10 +48,10 @@ module.exports = MetaController = {
docMeta
) {
if (err != null) {
logger.warn(
{ project_id, doc_id, err },
'[MetaController] error getting labels from doc'
)
OError.tag(err, '[MetaController] error getting labels from doc', {
project_id,
doc_id
})
return next(err)
}
EditorRealTimeController.emitToRoom(project_id, 'broadcastDocMeta', {

View file

@ -1,4 +1,5 @@
const Path = require('path')
const OError = require('@overleaf/o-error')
const fs = require('fs')
const crypto = require('crypto')
const async = require('async')
@ -257,10 +258,10 @@ const ProjectController = {
projectName,
(err, project) => {
if (err != null) {
logger.warn(
{ err, projectId, userId: currentUser._id },
'error cloning project'
)
OError.tag(err, 'error cloning project', {
projectId,
userId: currentUser._id
})
return next(err)
}
res.send({
@ -430,7 +431,7 @@ const ProjectController = {
},
(err, results) => {
if (err != null) {
logger.warn({ err }, 'error getting data for project list page')
OError.tag(err, 'error getting data for project list page')
return next(err)
}
const { notifications, user, userAffiliations } = results
@ -705,7 +706,7 @@ const ProjectController = {
},
(err, results) => {
if (err != null) {
logger.warn({ err }, 'error getting details for project page')
OError.tag(err, 'error getting details for project page')
return next(err)
}
const { project } = results

View file

@ -268,10 +268,9 @@ module.exports = SubscriptionController = {
logger.log({ user_id: user._id }, 'canceling subscription')
return SubscriptionHandler.cancelSubscription(user, function(err) {
if (err != null) {
logger.warn(
{ err, user_id: user._id },
'something went wrong canceling subscription'
)
OError.tag(err, 'something went wrong canceling subscription', {
user_id: user._id
})
return next(err)
}
// Note: this redirect isn't used in the main flow as the redirection is
@ -292,10 +291,9 @@ module.exports = SubscriptionController = {
logger.log({ user_id }, 'canceling v1 subscription')
return V1SubscriptionManager.cancelV1Subscription(user_id, function(err) {
if (err != null) {
logger.warn(
{ err, user_id },
'something went wrong canceling v1 subscription'
)
OError.tag(err, 'something went wrong canceling v1 subscription', {
user_id
})
return next(err)
}
return res.redirect('/user/subscription')
@ -322,10 +320,9 @@ module.exports = SubscriptionController = {
null,
function(err) {
if (err != null) {
logger.warn(
{ err, user_id: user._id },
'something went wrong updating subscription'
)
OError.tag(err, 'something went wrong updating subscription', {
user_id: user._id
})
return next(err)
}
return res.redirect('/user/subscription')
@ -350,10 +347,9 @@ module.exports = SubscriptionController = {
logger.log({ user_id: user._id }, 'reactivating subscription')
return SubscriptionHandler.reactivateSubscription(user, function(err) {
if (err != null) {
logger.warn(
{ err, user_id: user._id },
'something went wrong reactivating subscription'
)
OError.tag(err, 'something went wrong reactivating subscription', {
user_id: user._id
})
return next(err)
}
return res.redirect('/user/subscription')
@ -447,7 +443,9 @@ module.exports = SubscriptionController = {
coupon_code,
function(err) {
if (err != null) {
logger.warn({ err, user_id: user._id }, 'error updating subscription')
OError.tag(err, 'error updating subscription', {
user_id: user._id
})
return next(err)
}
return res.sendStatus(200)

View file

@ -12,6 +12,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const SubscriptionGroupHandler = require('./SubscriptionGroupHandler')
const OError = require('@overleaf/o-error')
const logger = require('logger-sharelatex')
const SubscriptionLocator = require('./SubscriptionLocator')
const AuthenticationController = require('../Authentication/AuthenticationController')
@ -31,10 +32,10 @@ module.exports = {
userToRemove_id,
function(err) {
if (err != null) {
logger.warn(
{ err, subscriptionId: subscription._id, userToRemove_id },
'error removing user from group'
)
OError.tag(err, 'error removing user from group', {
subscriptionId: subscription._id,
userToRemove_id
})
return next(err)
}
return res.sendStatus(200)

View file

@ -12,6 +12,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let SudoModeController
const OError = require('@overleaf/o-error')
const logger = require('logger-sharelatex')
const SudoModeHandler = require('./SudoModeHandler')
const AuthenticationController = require('../Authentication/AuthenticationController')
@ -31,10 +32,9 @@ module.exports = SudoModeController = {
logger.log({ userId }, '[SudoMode] rendering sudo mode password page')
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) {
@ -68,12 +68,13 @@ module.exports = SudoModeController = {
userRecord
) {
if (err != null) {
logger.warn({ err, userId }, '[SudoMode] error getting user')
OError.tag(err, '[SudoMode] error getting user', {
userId
})
return next(err)
}
if (userRecord == null) {
err = new Error('user not found')
logger.warn({ err, userId }, '[SudoMode] user not found')
err = new OError('[SudoMode] user not found', { userId })
return next(err)
}
return SudoModeHandler.authenticate(userRecord.email, password, function(
@ -81,7 +82,9 @@ module.exports = SudoModeController = {
user
) {
if (err != null) {
logger.warn({ err, userId }, '[SudoMode] error authenticating user')
OError.tag(err, '[SudoMode] error authenticating user', {
userId
})
return next(err)
}
if (user != null) {
@ -91,10 +94,9 @@ module.exports = SudoModeController = {
)
return SudoModeHandler.activateSudoMode(userId, function(err) {
if (err != null) {
logger.warn(
{ err, userId },
'[SudoMode] error activating sudo mode'
)
OError.tag(err, '[SudoMode] error activating sudo mode', {
userId
})
return next(err)
}
return res.json({

View file

@ -188,9 +188,12 @@ const UserController = {
password,
(err, user) => {
if (err != null) {
logger.warn(
{ userId },
'error authenticating during attempt to delete account'
OError.tag(
err,
'error authenticating during attempt to delete account',
{
userId
}
)
return next(err)
}
@ -230,7 +233,7 @@ const UserController = {
}
req.session.destroy(err => {
if (err != null) {
logger.warn({ err }, 'error destorying session')
OError.tag(err, 'error destroying session')
return next(err)
}
UserSessionsManager.untrackSession(user, sessionId)
@ -398,7 +401,7 @@ const UserController = {
} // passport logout
req.session.destroy(err => {
if (err) {
logger.warn({ err }, 'error destorying session')
OError.tag(err, 'error destroying session')
return cb(err)
}
if (user != null) {

View file

@ -1,4 +1,5 @@
const UserGetter = require('./UserGetter')
const OError = require('@overleaf/o-error')
const UserSessionsManager = require('./UserSessionsManager')
const logger = require('logger-sharelatex')
const Settings = require('settings-sharelatex')
@ -142,7 +143,9 @@ const UserPagesController = {
[req.sessionID],
(err, sessions) => {
if (err != null) {
logger.warn({ userId: user._id }, 'error getting all user sessions')
OError.tag(err, 'error getting all user sessions', {
userId: user._id
})
return next(err)
}
res.render('user/sessions', {

View file

@ -12,6 +12,7 @@
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let LaunchpadController
const OError = require('@overleaf/o-error')
const Settings = require('settings-sharelatex')
const Path = require('path')
const Url = require('url')
@ -105,7 +106,9 @@ module.exports = LaunchpadController = {
const emailOptions = { to: email }
return EmailHandler.sendEmail('testEmail', emailOptions, function(err) {
if (err != null) {
logger.warn({ email }, 'error sending test email')
OError.tag(err, 'error sending test email', {
email
})
return next(err)
}
logger.log({ email }, 'sent test email')
@ -158,10 +161,10 @@ module.exports = LaunchpadController = {
user
) {
if (err != null) {
logger.warn(
{ err, email, authMethod },
'error with registerNewUser'
)
OError.tag(err, 'error with registerNewUser', {
email,
authMethod
})
return next(err)
}
@ -173,10 +176,9 @@ module.exports = LaunchpadController = {
},
function(err) {
if (err != null) {
logger.warn(
{ user_id: user._id, err },
'error setting user to admin'
)
OError.tag(err, 'error setting user to admin', {
user_id: user._id
})
return next(err)
}
@ -233,10 +235,9 @@ module.exports = LaunchpadController = {
},
function(err) {
if (err != null) {
logger.err(
{ user_id: user._id, err },
'error setting user to admin'
)
OError.tag(err, 'error setting user to admin', {
user_id: user._id
})
return next(err)
}

View file

@ -3,7 +3,6 @@ npx jscodeshift \
-t transform/o-error/transform.js \
--ignore-pattern=frontend/js/libraries.js \
--ignore-pattern=frontend/js/vendor \
--ignore-pattern=**/*Controller*js \
$1
# replace blank lines in staged changed with token
git diff --ignore-all-space --ignore-blank-lines | sed 's/^\+$/\+REMOVE_ME_IM_A_BLANK_LINE/g' | git apply --reject --cached --ignore-space-change