fix: fall back to 1d for unrecognized SF_LOG_ROTATION_PERIOD values @W-23050508@ (#1314)

Author: WillieRuemmele

src/logger/logger.ts

@@ -9,7 +9,7 @@ import * as path from 'node:path';
 
 import { type Logger as PinoLogger, pino } from 'pino';
 import { Env } from '@salesforce/kit';
-import { ensureString, isKeyOf, isString } from '@salesforce/ts-types';
+import { isKeyOf, isString } from '@salesforce/ts-types';
 import { Global, Mode } from '../global';
 import { SfError } from '../sfError';
 import { unwrapArray } from '../util/unwrapArray';
@@ -451,7 +451,7 @@ export class Logger {
 }
 
 /** return various streams that the logger could send data to, depending on the options and env  */
-const getWriteStream = (level = 'warn'): pino.TransportSingleOptions => {
+export const getWriteStream = (level = 'warn'): pino.TransportSingleOptions => {
   const env = new Env();
   // used when debug mode, writes to stdout (colorized)
   if (process.env.DEBUG) {
@@ -474,14 +474,17 @@ const getWriteStream = (level = 'warn'): pino.TransportSingleOptions => {
   ]);
   const logRotationPeriod = env.getString('SF_LOG_ROTATION_PERIOD') ?? '1d';
 
+  if (logRotationPeriod && !rotator.has(logRotationPeriod)) {
+    process.stderr.write(
+      `Warning: Unrecognized SF_LOG_ROTATION_PERIOD value "${logRotationPeriod}". Expected 1m, 1h, or 1d. Falling back to 1d.\n`
+    );
+  }
+
   return {
     // write to a rotating file
     target: 'pino/file',
     options: {
-      destination: path.join(
-        Global.SF_DIR,
-        `sf-${ensureString(rotator.get(logRotationPeriod)) ?? rotator.get('1d')}.log`
-      ),
+      destination: path.join(Global.SF_DIR, `sf-${rotator.get(logRotationPeriod) ?? rotator.get('1d')}.log`),
       mkdir: true,
       level,
     },

test/unit/logger/logger.test.ts

@@ -10,7 +10,7 @@
 
 import { expect, config as chaiConfig } from 'chai';
 import { isString } from '@salesforce/ts-types';
-import { Logger, LoggerLevel, computeLevel } from '../../../src/logger/logger';
+import { Logger, LoggerLevel, computeLevel, getWriteStream } from '../../../src/logger/logger';
 import { shouldThrowSync, TestContext } from '../../../src/testSetup';
 
 // NOTE: These tests still use 'await' which is how it use to work and were left to make
@@ -236,4 +236,59 @@ describe('Logger', () => {
       expect(records[2]).to.have.property('level', LoggerLevel.INFO);
     });
   });
+
+  describe('getWriteStream', () => {
+    let stderrOutput: string;
+    let originalWrite: typeof process.stderr.write;
+
+    beforeEach(() => {
+      stderrOutput = '';
+      originalWrite = process.stderr.write;
+      process.stderr.write = ((chunk: string) => {
+        stderrOutput += chunk;
+        return true;
+      }) as typeof process.stderr.write;
+    });
+
+    afterEach(() => {
+      process.stderr.write = originalWrite;
+    });
+
+    it('should not throw for unrecognized SF_LOG_ROTATION_PERIOD values and emit a warning', () => {
+      process.env.SF_LOG_ROTATION_PERIOD = '2d';
+      const result = getWriteStream();
+      expect(result).to.have.property('target', 'pino/file');
+      // falls back to 1d rotation format (YYYY-MM-DD)
+      const expected1dSuffix = new Date().toISOString().split('T')[0];
+      expect((result.options as { destination: string }).destination).to.include(`sf-${expected1dSuffix}.log`);
+      expect(stderrOutput).to.include('Unrecognized SF_LOG_ROTATION_PERIOD');
+      expect(stderrOutput).to.include('2d');
+      expect(stderrOutput).to.include('Falling back to 1d');
+    });
+
+    it('should treat empty string as unset and not warn', () => {
+      process.env.SF_LOG_ROTATION_PERIOD = '';
+      const result = getWriteStream();
+      expect(result).to.have.property('target', 'pino/file');
+      const expected1dSuffix = new Date().toISOString().split('T')[0];
+      expect((result.options as { destination: string }).destination).to.include(`sf-${expected1dSuffix}.log`);
+      expect(stderrOutput).to.equal('');
+    });
+
+    it('should use the recognized period when valid and not warn', () => {
+      process.env.SF_LOG_ROTATION_PERIOD = '1h';
+      const result = getWriteStream();
+      expect(result).to.have.property('target', 'pino/file');
+      expect((result.options as { destination: string }).destination).to.include('.log');
+      expect(stderrOutput).to.equal('');
+    });
+
+    it('should default to 1d when SF_LOG_ROTATION_PERIOD is not set', () => {
+      delete process.env.SF_LOG_ROTATION_PERIOD;
+      const result = getWriteStream();
+      expect(result).to.have.property('target', 'pino/file');
+      expect((result.options as { destination: string }).destination).to.include('.log');
+      expect(stderrOutput).to.equal('');
+    });
+  });
 });