[misc] fix logger.error(err) and logger.warn(err) calls (#23369)

* [misc] fix logger.error(err) calls

The signature is "logger.error({ err }, 'MESSAGE')".

* [project-history] remove duplicate logger.err calls in health check

The call-site is already logging any errors. Also, the logger.err call
signature was not quite right.

* [web] log userId when removeDropbox/removeGithub hook fails

* [misc] fix logger.warn(err) calls

The signature is "logger.warn({ err }, 'MESSAGE')".

* [misc] fix logger.error(OError.tag(err)) calls

* [web] make eslint happy

GitOrigin-RevId: 7f528113a3f7e9f6293b7d2d45adc079380325bb

services/history-v1/api/controllers/project_import.js

@@ -59,7 +59,7 @@ exports.importChanges = function importChanges(req, res, next) {
   try {
     changes = rawChanges.map(Change.fromRaw)
   } catch (err) {
-    logger.error(err)
+    logger.error({ err, projectId }, err.message)
     return render.unprocessableEntity(res)
   }
 
@@ -129,7 +129,7 @@ exports.importChanges = function importChanges(req, res, next) {
       ) {
         // If we failed to apply operations, that's probably because they were
         // invalid.
-        logger.error(err)
+        logger.error({ err, projectId }, err.message)
         render.unprocessableEntity(res)
       } else if (err instanceof Chunk.NotFoundError) {
         render.notFound(res)

services/history-v1/app.js

@@ -84,16 +84,17 @@ function setupErrorHandling() {
 
   // Handle Swagger errors.
   app.use(function (err, req, res, next) {
+    const projectId = req.swagger?.params?.project_id?.value
     if (res.headersSent) {
       return next(err)
     }
 
     if (err.code === 'SCHEMA_VALIDATION_FAILED') {
-      logger.error(err)
+      logger.error({ err, projectId }, err.message)
       return res.status(HTTPStatus.UNPROCESSABLE_ENTITY).json(err.results)
     }
     if (err.code === 'INVALID_TYPE' || err.code === 'PATTERN') {
-      logger.error(err)
+      logger.error({ err, projectId }, err.message)
       return res.status(HTTPStatus.UNPROCESSABLE_ENTITY).json({
         message: 'invalid type: ' + err.paramName,
       })
@@ -112,7 +113,8 @@ function setupErrorHandling() {
   })
 
   app.use(function (err, req, res, next) {
-    logger.error(err)
+    const projectId = req.swagger?.params?.project_id?.value
+    logger.error({ err, projectId }, err.message)
 
     if (res.headersSent) {
       return next(err)

services/project-history/app.js

@@ -14,10 +14,11 @@ mongoClient
   .connect()
   .then(() => {
     app.listen(port, host, error => {
-      if (error != null) {
-        logger.error(OError.tag(error, 'could not start history server'))
+      if (error) {
+        error = OError.tag(error, 'could not start history server')
+        logger.error({ error }, error.message)
       } else {
-        logger.debug(`history starting up, listening on ${host}:${port}`)
+        logger.debug({}, `history starting up, listening on ${host}:${port}`)
       }
     })
   })

services/project-history/app/js/HealthChecker.js

@@ -29,7 +29,6 @@ export function check(callback) {
             OError.tag(err, 'error checking lock for health check', {
               project_id: projectId,
             })
-            logger.err(err)
             return cb(err)
           } else if ((res != null ? res.statusCode : undefined) !== 200) {
             return cb(new Error(`status code not 200, it's ${res.statusCode}`))
@@ -46,7 +45,6 @@ export function check(callback) {
             OError.tag(err, 'error flushing for health check', {
               project_id: projectId,
             })
-            logger.err(err)
             return cb(err)
           } else if ((res != null ? res.statusCode : undefined) !== 204) {
             return cb(new Error(`status code not 204, it's ${res.statusCode}`))
@@ -63,7 +61,6 @@ export function check(callback) {
             OError.tag(err, 'error getting updates for health check', {
               project_id: projectId,
             })
-            logger.err(err)
             return cb(err)
           } else if ((res != null ? res.statusCode : undefined) !== 200) {
             return cb(new Error(`status code not 200, it's ${res.statusCode}`))

services/project-history/app/js/HistoryStoreManager.js

@@ -250,7 +250,7 @@ export function sendChanges(
           statusCode: error.statusCode,
           body: error.body,
         })
-        logger.warn(error)
+        logger.warn({ error, projectId, historyId, endVersion }, error.message)
         return callback(error)
       }
       callback()

services/web/app/src/Features/Subscription/FeaturesUpdater.js

@@ -57,7 +57,7 @@ async function refreshFeatures(userId, reason) {
     try {
       await Modules.promises.hooks.fire('removeDropbox', userId, reason)
     } catch (err) {
-      logger.error(err)
+      logger.error({ err, userId }, 'removeDropbox hook failed')
     }
   }
 
@@ -66,7 +66,7 @@ async function refreshFeatures(userId, reason) {
     try {
       await Modules.promises.hooks.fire('removeGithub', userId, reason)
     } catch (err) {
-      logger.error(err)
+      logger.error({ err, userId }, 'removeGithub hook failed')
     }
   }
 

services/web/app/src/Features/User/UserController.js

@@ -266,7 +266,8 @@ async function tryDeleteUser(req, res, next) {
       errorData.info.public = {
         error: 'SubscriptionAdminDeletionError',
       }
-      logger.warn(OError.tag(err, errorData.message, errorData.info))
+      const error = OError.tag(err, errorData.message, errorData.info)
+      logger.warn({ error, req }, error.message)
       return HttpErrorHandler.unprocessableEntity(
         req,
         res,

services/web/app/src/Features/User/UserCreator.js

@@ -100,7 +100,8 @@ async function createNewUser(attributes, options = {}) {
         await UserDeleter.promises.deleteMongoUser(user._id)
         throw OError.tag(error)
       } else {
-        logger.error(OError.tag(error))
+        const err = OError.tag(error, 'adding affiliations failed')
+        logger.error({ err, userId: user._id }, err.message)
       }
     }
   }

services/web/app/src/Features/User/UserPagesController.mjs

@@ -79,7 +79,8 @@ async function settingsPage(req, res) {
     )
     personalAccessTokens = results?.[0] ?? []
   } catch (error) {
-    logger.error(OError.tag(error))
+    const err = OError.tag(error, 'listPersonalAccessTokens hook failed')
+    logger.error({ err, userId }, err.message)
   }
 
   let currentManagedUserAdminEmail

services/web/app/src/infrastructure/QueueWorkers.js

@@ -66,7 +66,7 @@ function start() {
       await EmailHandler.promises.sendEmail(emailType, opts)
     } catch (e) {
       const error = OError.tag(e, 'failed to send deferred email')
-      logger.warn(error)
+      logger.warn({ error, emailType }, error.message)
       throw error
     }
   })
@@ -84,7 +84,7 @@ function start() {
         e,
         'failed to send scheduled Group SSO account linking reminder'
       )
-      logger.warn(error)
+      logger.warn({ error, userId, subscriptionId }, error.message)
       throw error
     }
   })