From 3d77143eb819d8e30a80045d556ea2ceca97d311 Mon Sep 17 00:00:00 2001 From: Aleksandr Borovskii Date: Fri, 23 Oct 2026 17:49:11 +0200 Subject: [PATCH 1/4] fix(repeater): stop multiplying the --timeout default by 1000 The repeater redeclares --timeout with a 30000 ms default, but yargs kept applying the global seconds-based coercion to it, so the value became 30000000 ms (about 8h20m). It is used both as the Bright socket connect timeout and as the target request timeout, so a reconnection attempt that never got an upgrade response could hang for hours without a retry. Give the repeater option its own millisecond-based coercion, sharing the duration parsing with the global option through Helpers.parseDuration. This also makes duration strings such as "1m" work for the repeater, which previously parsed to NaN. --- src/Commands/RunRepeater.spec.ts | 69 ++++++++++++++++++++++++++++++++ src/Commands/RunRepeater.ts | 6 ++- src/Config/CliBuilder.ts | 11 +---- src/Utils/Helpers.spec.ts | 23 +++++++++++ src/Utils/Helpers.ts | 16 ++++++++ 5 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 src/Commands/RunRepeater.spec.ts 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/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..feb59f4e 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,21 @@ export class Helpers { }, {}); } + /** + * Converts a duration to milliseconds. Strings such as "10s" or "1m" are + * parsed with `ms`; a bare number is read in `numericUnit`. + */ + public static parseDuration( + value: string | number, + numericUnit: 'ms' | 's' + ): number { + if (isNaN(+value)) { + return ms(value as string); + } + + return numericUnit === 's' ? +value * 1000 : +value; + } + public static portFromURL(url: URL): string { return ( url.port || From e4d84b51542a67b9d1f509117119c0ef19cda5ec Mon Sep 17 00:00:00 2001 From: Aleksandr Borovskii Date: Fri, 23 Oct 2026 17:49:15 +0200 Subject: [PATCH 2/4] fix(repeater): log reconnection attempts as reconnecting The warning is emitted when a reconnection attempt starts, not after it fails, so "Failed to connect to Bright cloud" was misleading. --- src/Repeater/ServerRepeaterLauncher.spec.ts | 60 +++++++++++++++++++++ src/Repeater/ServerRepeaterLauncher.ts | 2 +- 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/Repeater/ServerRepeaterLauncher.spec.ts 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) From c52f7fbbb9cafa26a4cc4b80b4d1b4674c2db5c0 Mon Sep 17 00:00:00 2001 From: Alexander Borovsky Date: Fri, 23 Oct 2026 18:57:00 +0200 Subject: [PATCH 3/4] Update Helpers.ts --- src/Utils/Helpers.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Utils/Helpers.ts b/src/Utils/Helpers.ts index feb59f4e..ec567f73 100644 --- a/src/Utils/Helpers.ts +++ b/src/Utils/Helpers.ts @@ -248,10 +248,6 @@ export class Helpers { }, {}); } - /** - * Converts a duration to milliseconds. Strings such as "10s" or "1m" are - * parsed with `ms`; a bare number is read in `numericUnit`. - */ public static parseDuration( value: string | number, numericUnit: 'ms' | 's' From d1cbe695fe902d72ac38d6f518d619b103b16491 Mon Sep 17 00:00:00 2001 From: Alexander Borovsky Date: Fri, 23 Oct 2026 18:59:24 +0200 Subject: [PATCH 4/4] Apply suggestion from @aborovsky --- src/Utils/Helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/Helpers.ts b/src/Utils/Helpers.ts index ec567f73..a409e854 100644 --- a/src/Utils/Helpers.ts +++ b/src/Utils/Helpers.ts @@ -253,7 +253,7 @@ export class Helpers { numericUnit: 'ms' | 's' ): number { if (isNaN(+value)) { - return ms(value as string); + return ms(String(value)); } return numericUnit === 's' ? +value * 1000 : +value;