fix: data transfer logging (#20335)

* fix: only create one log file and only as needed
* test: add regression test for multiple logs

packages/core/strapi/src/cli/utils/__tests__/logger.test.ts

@@ -0,0 +1,89 @@
+import { type Logger, createLogger } from '@strapi/logger';
+import { formatDiagnostic } from '../data-transfer';
+
+jest.mock('@strapi/logger', () => {
+  const actualWinston = jest.requireActual('winston');
+  return {
+    ...actualWinston,
+    createLogger: jest.fn(),
+    configs: {
+      createOutputFileConfiguration: jest.fn(),
+    },
+  };
+});
+
+// Mock console methods to avoid cluttering test output
+const consoleMock = {
+  error: jest.spyOn(console, 'error').mockImplementation(() => {}),
+  warn: jest.spyOn(console, 'warn').mockImplementation(() => {}),
+  info: jest.spyOn(console, 'info').mockImplementation(() => {}),
+};
+
+describe('logger', () => {
+  let mockLogger: jest.Mocked<Logger>;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+
+    // Create a mock logger with jest.fn() methods
+    mockLogger = {
+      error: jest.fn(),
+      warn: jest.fn(),
+      info: jest.fn(),
+      debug: jest.fn(),
+    } as any;
+
+    // Mock createLogger to return the mock logger
+    (createLogger as jest.Mock).mockReturnValue(mockLogger);
+  });
+
+  afterEach(() => {
+    // Clear console mocks after each test
+    consoleMock.error.mockClear();
+    consoleMock.warn.mockClear();
+    consoleMock.info.mockClear();
+  });
+
+  describe('formatDiagnostic', () => {
+    it('only creates a single logger', () => {
+      const diagnosticReporter = formatDiagnostic('export');
+
+      // Use the diagnostic reporter to log different levels of messages
+      diagnosticReporter({
+        kind: 'error',
+        details: {
+          message: 'Test error',
+          createdAt: new Date(),
+          name: '',
+          severity: 'error',
+          error: new Error(),
+        },
+      });
+      diagnosticReporter({
+        kind: 'warning',
+        details: {
+          message: 'Test warning',
+          createdAt: new Date(),
+        },
+      });
+      diagnosticReporter({
+        kind: 'info',
+        details: {
+          message: 'Test info',
+          createdAt: new Date(),
+        },
+      });
+
+      // Verify createLogger is called only once
+      expect(createLogger).toHaveBeenCalledTimes(1);
+
+      // Verify the mock logger received the logs
+      expect(mockLogger.error).toHaveBeenCalledTimes(1);
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('Test error'));
+      expect(mockLogger.warn).toHaveBeenCalledTimes(1);
+      expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('Test warning'));
+      expect(mockLogger.info).toHaveBeenCalledTimes(1);
+      expect(mockLogger.info).toHaveBeenCalledWith(expect.stringContaining('Test info'));
+    });
+  });
+});

packages/core/strapi/src/cli/utils/data-transfer.ts

@@ -1,7 +1,7 @@
 import chalk from 'chalk';
 import Table from 'cli-table3';
 import { Command, Option } from 'commander';
-import { configs, createLogger } from '@strapi/logger';
+import { configs, createLogger, type winston } from '@strapi/logger';
 import { createStrapi, compileStrapi } from '@strapi/core';
 import ora from 'ora';
 import { merge } from 'lodash/fp';
@@ -209,39 +209,49 @@ const errorColors = {
   silly: chalk.yellow,
 } as const;
 
-const formatDiagnostic =
-  (
-    operation: string
-  ): Parameters<engineDataTransfer.TransferEngine['diagnostics']['onDiagnostic']>[0] =>
-  ({ details, kind }) => {
-    const logger = createLogger(
-      configs.createOutputFileConfiguration(`${operation}_error_log_${Date.now()}.log`)
-    );
+const formatDiagnostic = (
+  operation: string
+): Parameters<engineDataTransfer.TransferEngine['diagnostics']['onDiagnostic']>[0] => {
+  // Create log file for all incoming diagnostics
+  let logger: undefined | winston.Logger;
+  const getLogger = () => {
+    if (!logger) {
+      logger = createLogger(
+        configs.createOutputFileConfiguration(`${operation}_${Date.now()}.log`, { level: 'info' })
+      );
+    }
+    return logger;
+  };
+
+  // We don't want to write a log file until there is something to be logged
+
+  return ({ details, kind }) => {
     try {
       if (kind === 'error') {
         const { message, severity = 'fatal' } = details;
 
         const colorizeError = errorColors[severity];
         const errorMessage = colorizeError(`[${severity.toUpperCase()}] ${message}`);
 
-        logger.error(errorMessage);
+        getLogger().error(errorMessage);
       }
       if (kind === 'info') {
         const { message, params } = details;
 
         const msg = `${message}\n${params ? JSON.stringify(params, null, 2) : ''}`;
 
-        logger.info(msg);
+        getLogger().info(msg);
       }
       if (kind === 'warning') {
         const { origin, message } = details;
 
-        logger.warn(`(${origin ?? 'transfer'}) ${message}`);
+        getLogger().warn(`(${origin ?? 'transfer'}) ${message}`);
       }
     } catch (err) {
-      logger.error(err);
+      getLogger().error(err);
     }
   };
+};
 
 type Loaders = {
   [key in engineDataTransfer.TransferStage]: ora.Ora;

packages/utils/logger/src/configs/output-file-configuration.ts

@@ -3,14 +3,22 @@ import { transports, LoggerOptions } from 'winston';
 import { LEVEL_LABEL, LEVELS } from '../constants';
 import { prettyPrint, excludeColors } from '../formats';
 
-export default (filename: string): LoggerOptions => {
+export default (
+  filename: string,
+  fileTransportOptions: transports.FileTransportOptions = {}
+): LoggerOptions => {
   return {
     level: LEVEL_LABEL,
     levels: LEVELS,
     format: prettyPrint(),
     transports: [
       new transports.Console(),
-      new transports.File({ level: 'error', filename, format: excludeColors }),
+      new transports.File({
+        level: 'error',
+        filename,
+        format: excludeColors,
+        ...fileTransportOptions,
+      }),
     ],
   };
 };