chore(eslint): payload logger usage (#8578)

Payload uses `pino` for a logger. When using the error logger
`payload.logger.error` it is possible to pass any number of arguments
like this: `payload.logger.error('Some error ocurred', err)`. However,
in this scenario, the full error will not be serialized by `pino`. It
must be passed as the `err` property inside of an object in order to be
properly serialized.

This rule ensures that a user is using this function call to properly serialize the error.

eslint.config.js

@@ -53,6 +53,7 @@ export const rootEslintConfig = [
       'payload/no-relative-monorepo-imports': 'error',
       'payload/no-imports-from-exports-dir': 'error',
       'payload/no-imports-from-self': 'error',
+      'payload/proper-payload-logger-usage': 'error',
     },
   },
   {

packages/eslint-plugin/customRules/proper-payload-logger-usage.js

@@ -0,0 +1,89 @@
+export const rule = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Disallow improper usage of payload.logger.error',
+      recommended: 'error',
+    },
+    messages: {
+      improperUsage: 'Improper logger usage. Pass { msg, err } so full error stack is logged.',
+      wrongErrorField: 'Improper usage. Use { err } instead of { error }.',
+      wrongMessageField: 'Improper usage. Use { msg } instead of { message }.',
+    },
+    schema: [],
+  },
+  create(context) {
+    return {
+      CallExpression(node) {
+        const callee = node.callee
+
+        // Function to check if the expression ends with `payload.logger.error`
+        function isPayloadLoggerError(expression) {
+          return (
+            expression.type === 'MemberExpression' &&
+            expression.property.name === 'error' && // must be `.error`
+            expression.object.type === 'MemberExpression' &&
+            expression.object.property.name === 'logger' && // must be `.logger`
+            (expression.object.object.name === 'payload' || // handles just `payload`
+              (expression.object.object.type === 'MemberExpression' &&
+                expression.object.object.property.name === 'payload')) // handles `*.payload`
+          )
+        }
+
+        // Check if the function being called is `payload.logger.error` or `*.payload.logger.error`
+        if (isPayloadLoggerError(callee)) {
+          const args = node.arguments
+
+          // Case 1: Single string is passed as the argument
+          if (
+            args.length === 1 &&
+            args[0].type === 'Literal' &&
+            typeof args[0].value === 'string'
+          ) {
+            return // Valid: single string argument
+          }
+
+          // Case 2: Object is passed as the first argument
+          if (args.length > 0 && args[0].type === 'ObjectExpression') {
+            const properties = args[0].properties
+
+            // Ensure no { error } key, only { err } is allowed
+            properties.forEach((prop) => {
+              if (prop.key.type === 'Identifier' && prop.key.name === 'error') {
+                context.report({
+                  node: prop,
+                  messageId: 'wrongErrorField',
+                })
+              }
+
+              // Ensure no { message } key, only { msg } is allowed
+              if (prop.key.type === 'Identifier' && prop.key.name === 'message') {
+                context.report({
+                  node: prop,
+                  messageId: 'wrongMessageField',
+                })
+              }
+            })
+            return // Valid object, checked for 'err'/'error' keys
+          }
+
+          // Case 3: Improper usage (string + error or additional err/error)
+          if (
+            args.length > 1 &&
+            args[0].type === 'Literal' &&
+            typeof args[0].value === 'string' &&
+            args[1].type === 'Identifier' &&
+            (args[1].name === 'err' || args[1].name === 'error')
+          ) {
+            context.report({
+              node,
+              messageId: 'improperUsage',
+            })
+          }
+        }
+      },
+    }
+  },
+}
+
+export default rule

packages/eslint-plugin/index.mjs

@@ -4,6 +4,7 @@ import noRelativeMonorepoImports from './customRules/no-relative-monorepo-import
 import noImportsFromExportsDir from './customRules/no-imports-from-exports-dir.js'
 import noFlakyAssertions from './customRules/no-flaky-assertions.js'
 import noImportsFromSelf from './customRules/no-imports-from-self.js'
+import properPinoLoggerErrorUsage from './customRules/proper-payload-logger-usage.js'
 
 /**
  * @type {import('eslint').ESLint.Plugin}
@@ -11,10 +12,13 @@ import noImportsFromSelf from './customRules/no-imports-from-self.js'
 const index = {
   rules: {
     'no-jsx-import-statements': noJsxImportStatements,
-    'no-non-retryable-assertions': noNonRetryableAssertions,
     'no-relative-monorepo-imports': noRelativeMonorepoImports,
     'no-imports-from-exports-dir': noImportsFromExportsDir,
     'no-imports-from-self': noImportsFromSelf,
+    'proper-payload-logger-usage': properPinoLoggerErrorUsage,
+
+    // Testing-related
+    'no-non-retryable-assertions': noNonRetryableAssertions,
     'no-flaky-assertions': noFlakyAssertions,
     'no-wait-function': {
       create: function (context) {

packages/eslint-plugin/tests/proper-payload-logger-usage.js

@@ -0,0 +1,72 @@
+import { RuleTester } from 'eslint'
+import rule from '../customRules/proper-payload-logger-usage.js'
+
+const ruleTester = new RuleTester()
+
+// Example tests for the rule
+ruleTester.run('no-improper-payload-logger-error', rule, {
+  valid: [
+    // Valid: payload.logger.error with object containing { msg, err }
+    {
+      code: "payload.logger.error({ msg: 'some message', err })",
+    },
+    // Valid: payload.logger.error with a single string
+    {
+      code: "payload.logger.error('Some error message')",
+    },
+    // Valid: *.payload.logger.error with object
+    {
+      code: "this.payload.logger.error({ msg: 'another message', err })",
+    },
+    {
+      code: "args.req.payload.logger.error({ msg: 'different message', err })",
+    },
+  ],
+
+  invalid: [
+    // Invalid: payload.logger.error with both string and error
+    {
+      code: "payload.logger.error('Some error message', err)",
+      errors: [
+        {
+          messageId: 'improperUsage',
+        },
+      ],
+    },
+    // Invalid: *.payload.logger.error with both string and error
+    {
+      code: "this.payload.logger.error('Some error message', error)",
+      errors: [
+        {
+          messageId: 'improperUsage',
+        },
+      ],
+    },
+    {
+      code: "args.req.payload.logger.error('Some error message', err)",
+      errors: [
+        {
+          messageId: 'improperUsage',
+        },
+      ],
+    },
+    // Invalid: payload.logger.error with object containing 'message' key
+    {
+      code: "payload.logger.error({ message: 'not the right property name' })",
+      errors: [
+        {
+          messageId: 'wrongMessageField',
+        },
+      ],
+    },
+    // Invalid: *.payload.logger.error with object containing 'error' key
+    {
+      code: "this.payload.logger.error({ msg: 'another message', error })",
+      errors: [
+        {
+          messageId: 'wrongErrorField',
+        },
+      ],
+    },
+  ],
+})