From 01188589f82d36bf4eb3dd5ddf7536dabc10ace0 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Thu, 27 Jun 2024 10:10:31 +0200 Subject: [PATCH] Add some JSDoc types to `@overleaf/logger` (#19153) * Add some JSDoc types to `@overleaf/logger` * Update `logger.error` calls * Fixup `logger.err` JSDoc * Update `logger.err` calls * Fix `args` type * Remove "Error message" description * Replace `arguments` by actual arguments of the method * Fix: "ESLint: Unnecessary '.apply()'.(no-useless-call)" * Add JSDoc params to `debug` `info` `warn` * Remove extra `args` param in JSDoc so developers aren't invited to use it Not sure if this is the best thing to do because it creates a warning in the IDE: "Parameter args is not described in JSDoc" * Add comment about serialization of `err` `req` `res` * Allow strings as first param in `debug` `info` `warn` * Fix syntax for optional parameters in JSDoc * Add 2 signatures, to avoid "string, string" params * Fix `@signature` names copy-pastes Co-authored-by: Jakob Ackermann * Revert the double `@param attributes`. It doesn't work --------- Co-authored-by: Jakob Ackermann GitOrigin-RevId: 086dee8bbf30d577c5e1f844a9df5e518c46aca7 --- libraries/logger/logging-manager.js | 50 ++++++++++++++++--- services/filestore/app.js | 2 +- .../Features/Spelling/SpellingController.js | 5 +- .../collect_paypal_past_due_invoice.js | 2 +- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/libraries/logger/logging-manager.js b/libraries/logger/logging-manager.js index 33754a918d..7268fd258a 100644 --- a/libraries/logger/logging-manager.js +++ b/libraries/logger/logging-manager.js @@ -9,6 +9,9 @@ const { } = require('./log-level-checker') const LoggingManager = { + /** + * @param {string} name - The name of the logger + */ initialize(name) { this.isProduction = (process.env.NODE_ENV || '').toLowerCase() === 'production' @@ -34,14 +37,30 @@ const LoggingManager = { this.sentryManager = new SentryManager() }, - debug() { - return this.logger.debug.apply(this.logger, arguments) + /** + * @param {Record|string} attributes - Attributes to log (nice serialization for err, req, res) + * @param {string} [message] - Optional message + * @signature `debug(attributes, message)` + * @signature `debug(message)` + */ + debug(attributes, message, ...args) { + return this.logger.debug(attributes, message, ...args) }, - info() { - return this.logger.info.apply(this.logger, arguments) + /** + * @param {Record|string} attributes - Attributes to log (nice serialization for err, req, res) + * @param {string} [message] + * @signature `info(attributes, message)` + * @signature `info(message)` + */ + info(attributes, message, ...args) { + return this.logger.info(attributes, message, ...args) }, + /** + * @param {Record} attributes - Attributes to log (nice serialization for err, req, res) + * @param {string} [message] + */ error(attributes, message, ...args) { if (this.ringBuffer !== null && Array.isArray(this.ringBuffer.records)) { attributes.logBuffer = this.ringBuffer.records.filter(function (record) { @@ -54,14 +73,29 @@ const LoggingManager = { } }, - err() { - return this.error.apply(this, arguments) + /** + * Alias to the error method. + * @param {Record} attributes - Attributes to log (nice serialization for err, req, res) + * @param {string} [message] + */ + err(attributes, message, ...args) { + return this.error(attributes, message, ...args) }, - warn() { - return this.logger.warn.apply(this.logger, arguments) + /** + * @param {Record|string} attributes - Attributes to log (nice serialization for err, req, res) + * @param {string} [message] + * @signature `warn(attributes, message)` + * @signature `warn(message)` + */ + warn(attributes, message, ...args) { + return this.logger.warn(attributes, message, ...args) }, + /** + * @param {Record} attributes - Attributes to log (nice serialization for err, req, res) + * @param {string} [message] + */ fatal(attributes, message) { this.logger.fatal(attributes, message) if (this.sentryManager) { diff --git a/services/filestore/app.js b/services/filestore/app.js index ad681ba436..b01d5a2200 100644 --- a/services/filestore/app.js +++ b/services/filestore/app.js @@ -165,7 +165,7 @@ if (!module.parent) { // Called directly app.listen(port, host, error => { if (error) { - logger.error('Error starting Filestore', error) + logger.error({ err: error }, 'Error starting Filestore') throw error } logger.debug(`Filestore starting up, listening on ${host}:${port}`) diff --git a/services/web/app/src/Features/Spelling/SpellingController.js b/services/web/app/src/Features/Spelling/SpellingController.js index 6c0dc15e89..59008378cd 100644 --- a/services/web/app/src/Features/Spelling/SpellingController.js +++ b/services/web/app/src/Features/Spelling/SpellingController.js @@ -35,7 +35,10 @@ module.exports = { if (url === '/check') { if (!language) { - logger.error('"language" field should be included for spell checking') + logger.error( + {}, + '"language" field should be included for spell checking' + ) return res.status(422).json({ misspellings: [] }) } diff --git a/services/web/scripts/recurly/collect_paypal_past_due_invoice.js b/services/web/scripts/recurly/collect_paypal_past_due_invoice.js index 0d60f7b50f..650285e3ac 100644 --- a/services/web/scripts/recurly/collect_paypal_past_due_invoice.js +++ b/services/web/scripts/recurly/collect_paypal_past_due_invoice.js @@ -119,7 +119,7 @@ const main = async () => { if (require.main === module) { main() .then(() => { - logger.error('Done.') + logger.info('Done.') process.exit(0) }) .catch(err => {