diff --git a/src/Commands/RunRepeater.spec.ts b/src/Commands/RunRepeater.spec.ts new file mode 100644 index 00000000..ebce6e9d --- /dev/null +++ b/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, [Arguments]>(); + + let cli: Argv; + + beforeAll(() => { + const runRepeater = new RunRepeater(); + runRepeater.handler = handler; + + cli = new CliBuilder({ + info: new CliInfo(process.cwd()), + configReader: instance(mock()) + }).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 + ).connectTimeout + ).toBe(expected); + expect( + container.resolve(RequestExecutorOptions) + .timeout + ).toBe(expected); + } + ); + }); +}); diff --git a/src/Commands/RunRepeater.ts b/src/Commands/RunRepeater.ts index 8b86c89f..e976596c 100644 --- a/src/Commands/RunRepeater.ts +++ b/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', diff --git a/src/Config/CliBuilder.ts b/src/Config/CliBuilder.ts index 46b9874c..b709c2b1 100644 --- a/src/Config/CliBuilder.ts +++ b/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'], diff --git a/src/Repeater/ServerRepeaterLauncher.spec.ts b/src/Repeater/ServerRepeaterLauncher.spec.ts new file mode 100644 index 00000000..45bd1f5e --- /dev/null +++ b/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(); + const handlers = new Map 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()), + instance(mock()), + instance(mockedRepeaterServer), + instance(mock()), + instance(mock()), + instance(mock()), + instance(mock()), + {} as RequestExecutorOptions, + { version: '1.0.0' } as CliInfo + ); + }); + + afterEach(() => { + handlers.clear(); + reset(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(); + }); + }); +}); diff --git a/src/Repeater/ServerRepeaterLauncher.ts b/src/Repeater/ServerRepeaterLauncher.ts index a72d6757..b59ce149 100644 --- a/src/Repeater/ServerRepeaterLauncher.ts +++ b/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) diff --git a/src/Utils/Helpers.spec.ts b/src/Utils/Helpers.spec.ts index 1b8b13f5..029e5681 100644 --- a/src/Utils/Helpers.spec.ts +++ b/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 diff --git a/src/Utils/Helpers.ts b/src/Utils/Helpers.ts index d8c5d58b..a409e854 100644 --- a/src/Utils/Helpers.ts +++ b/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 ||