fix(repeater): keep --timeout in milliseconds and reword the reconnect log

src/Commands/RunRepeater.spec.ts

@@ -0,0 +1,69 @@
+import 'reflect-metadata';
+import { RunRepeater } from './RunRepeater';
+import { CliBuilder, CliInfo, ConfigReader } from '../Config';
+import { DefaultRepeaterServerOptions } from '../Repeater';
+import { RequestExecutorOptions } from '../RequestExecutor';
+import container from '../container';
+import { instance, mock } from 'ts-mockito';
+import { Arguments, Argv } from 'yargs';
+
+jest.mock('../Config/SystemConfigManager', () => ({
+  SystemConfigManager: jest.fn().mockImplementation(() => ({
+    read: jest.fn().mockResolvedValue({}),
+    enableBackgroundRotation: jest.fn()
+  }))
+}));
+
+describe('RunRepeater', () => {
+  const repeaterId = '8PY6wKetVqmfMC9K9g4A7Y';
+  const handler = jest.fn<Promise<void>, [Arguments]>();
+
+  let cli: Argv;
+
+  beforeAll(() => {
+    const runRepeater = new RunRepeater();
+    runRepeater.handler = handler;
+
+    cli = new CliBuilder({
+      info: new CliInfo(process.cwd()),
+      configReader: instance(mock<ConfigReader>())
+    }).build({ commands: [runRepeater] });
+  });
+
+  afterEach(() => handler.mockReset());
+
+  describe('builder', () => {
+    it.each([
+      { flags: [], expected: 30_000 },
+      { flags: ['--timeout', '45000'], expected: 45_000 },
+      { flags: ['--timeout', '1m'], expected: 60_000 }
+    ])(
+      'should resolve the timeout to $expected ms given [$flags]',
+      async ({ flags, expected }) => {
+        // act
+        await cli.parse([
+          'repeater',
+          '--id',
+          repeaterId,
+          '--token',
+          'token',
+          ...flags
+        ]);
+
+        // assert
+        expect(handler).toHaveBeenCalledWith(
+          expect.objectContaining({ timeout: expected })
+        );
+        expect(
+          container.resolve<DefaultRepeaterServerOptions>(
+            DefaultRepeaterServerOptions
+          ).connectTimeout
+        ).toBe(expected);
+        expect(
+          container.resolve<RequestExecutorOptions>(RequestExecutorOptions)
+            .timeout
+        ).toBe(expected);
+      }
+    );
+  });
+});

src/Commands/RunRepeater.ts

@@ -38,11 +38,13 @@ export class RunRepeater implements CommandModule {
         }
       })
       .option('timeout', {
-        number: true,
         requiresArg: true,
         default: 30000,
         describe:
-          'Time to wait for a server to send response headers (and start the response body) before aborting the request.'
+          'Time to wait for a server to send response headers (and start the response body) before aborting the request, in milliseconds or a duration string (e.g. 30s, 1m).',
+        // Without its own coerce, yargs applies the global seconds-based
+        // --timeout coercion to this millisecond default.
+        coerce: (arg: string | number) => Helpers.parseDuration(arg, 'ms')
       })
       .option('header', {
         alias: 'H',

src/Config/CliBuilder.ts

@@ -4,7 +4,6 @@ import { SystemConfigManager } from './SystemConfigManager';
 import { CliInfo } from './CliInfo';
 import { Arguments, Argv, CommandModule } from 'yargs';
 import { init, runWithAsyncContext, setContext } from '@sentry/node';
-import ms from 'ms';
 import process from 'node:process';
 
 export interface CliBuilderOptions {
@@ -123,15 +122,7 @@ export class CliBuilder {
         describe:
           'Request timeout in seconds or a duration string (e.g. 10s, 1m, 1h, 10h, 1y).',
         default: 30,
-        coerce(arg: string) {
-          // if arg is not a number, then it's a duration string
-          // convert duration string to milliseconds
-          if (isNaN(+arg)) {
-            return ms(arg);
-          }
-
-          return +arg * 1000;
-        }
+        coerce: (arg: string | number) => Helpers.parseDuration(arg, 's')
       })
       .conflicts({
         proxy: ['proxy-bright', 'proxy-target'],

src/Repeater/ServerRepeaterLauncher.spec.ts

@@ -0,0 +1,60 @@
+import 'reflect-metadata';
+import { ServerRepeaterLauncher } from './ServerRepeaterLauncher';
+import { RepeaterServer, RepeaterServerEvents } from './RepeaterServer';
+import { RuntimeDetector } from './RuntimeDetector';
+import { RepeaterCommandHub } from './RepeaterCommandHub';
+import { ScriptLoader, VirtualScripts } from '../Scripts';
+import { StartupManager } from '../StartupScripts';
+import { Certificates, RequestExecutorOptions } from '../RequestExecutor';
+import { CliInfo } from '../Config';
+import { Logger, logger } from '../Utils';
+import { anything, instance, mock, reset, spy, verify, when } from 'ts-mockito';
+
+describe('ServerRepeaterLauncher', () => {
+  const mockedRepeaterServer = mock<RepeaterServer>();
+  const handlers = new Map<string, (payload?: unknown) => unknown>();
+
+  let loggerSpy: Logger;
+  let launcher: ServerRepeaterLauncher;
+
+  beforeEach(() => {
+    loggerSpy = spy(logger);
+    when(loggerSpy.warn(anything(), anything())).thenReturn();
+    when(mockedRepeaterServer.on(anything(), anything())).thenCall(
+      (event: string, handler: (payload?: unknown) => unknown) =>
+        handlers.set(event, handler)
+    );
+
+    launcher = new ServerRepeaterLauncher(
+      instance(mock<RuntimeDetector>()),
+      instance(mock<VirtualScripts>()),
+      instance(mockedRepeaterServer),
+      instance(mock<StartupManager>()),
+      instance(mock<RepeaterCommandHub>()),
+      instance(mock<Certificates>()),
+      instance(mock<ScriptLoader>()),
+      {} as RequestExecutorOptions,
+      { version: '1.0.0' } as CliInfo
+    );
+  });
+
+  afterEach(() => {
+    handlers.clear();
+    reset<RepeaterServer | Logger>(mockedRepeaterServer, loggerSpy);
+  });
+
+  describe('run', () => {
+    it('should log a reconnecting warning on a reconnection attempt', async () => {
+      // arrange
+      await launcher.run('8PY6wKetVqmfMC9K9g4A7Y');
+
+      // act
+      handlers.get(RepeaterServerEvents.RECONNECT_ATTEMPT)?.({ attempt: 2 });
+
+      // assert
+      verify(
+        loggerSpy.warn('Reconnecting to Bright cloud (attempt %d)', 2)
+      ).once();
+    });
+  });
+});

src/Repeater/ServerRepeaterLauncher.ts

@@ -157,7 +157,7 @@ export class ServerRepeaterLauncher implements RepeaterLauncher {
     this.repeaterServer.on(
       RepeaterServerEvents.RECONNECT_ATTEMPT,
       ({ attempt }) =>
-        logger.warn('Failed to connect to Bright cloud (attempt %d)', attempt)
+        logger.warn('Reconnecting to Bright cloud (attempt %d)', attempt)
     );
     this.repeaterServer.on(RepeaterServerEvents.RECONNECTION_SUCCEEDED, () =>
       logger.log('The Repeater (%s) connected', this.info.version)

src/Utils/Helpers.spec.ts

@@ -293,6 +293,29 @@ describe('Helpers', () => {
     });
   });
 
+  describe('parseDuration', () => {
+    it.each([
+      { input: 30, unit: 's', expected: 30_000 },
+      { input: '30', unit: 's', expected: 30_000 },
+      { input: '0.5', unit: 's', expected: 500 },
+      { input: 30_000, unit: 'ms', expected: 30_000 },
+      { input: '45000', unit: 'ms', expected: 45_000 },
+      { input: '10s', unit: 's', expected: 10_000 },
+      { input: '10s', unit: 'ms', expected: 10_000 },
+      { input: '1m', unit: 'ms', expected: 60_000 },
+      { input: '1h', unit: 's', expected: 3_600_000 }
+    ] as const)(
+      'should convert $input with numeric unit "$unit" to $expected milliseconds',
+      ({ input, unit, expected }) => {
+        // act
+        const actual = Helpers.parseDuration(input, unit);
+
+        // assert
+        expect(actual).toBe(expected);
+      }
+    );
+  });
+
   describe('portFromURL', () => {
     it('should return port 80 by default for http', () => {
       // arrange

src/Utils/Helpers.ts

@@ -1,3 +1,4 @@
+import ms from 'ms';
 import { ok } from 'node:assert';
 import { ChildProcess, spawn } from 'node:child_process';
 import { normalize } from 'node:path';
@@ -247,6 +248,17 @@ export class Helpers {
     }, {});
   }
 
+  public static parseDuration(
+    value: string | number,
+    numericUnit: 'ms' | 's'
+  ): number {
+    if (isNaN(+value)) {
+      return ms(String(value));
+    }
+
+    return numericUnit === 's' ? +value * 1000 : +value;
+  }
+
   public static portFromURL(url: URL): string {
     return (
       url.port ||