From 660f9183d24e0eb94b8981fed395191689adfdee Mon Sep 17 00:00:00 2001 From: frostebite Date: Fri, 14 Aug 2026 20:15:22 +0100 Subject: [PATCH] fix: activate defaulted to a build target platform, and errors printed as {} Two real bugs surfaced by testing unity-activate's thin-wrapper PR against this repo's freshly-merged activate command: - ActivateCommand inherited UnityOptions' build-oriented targetPlatform default (StandaloneWindows64), so a bare `game-ci activate ` - exactly what the thin wrapper calls - threw "Windows-based builds are only supported on 2019.3.X+" for any older Unity version, even though activation doesn't build anything. Defaults to NoTarget instead, which RunnerImageTag already maps to the generic image and skips build-target validation entirely. Explicit --target-platform still works. - log.error()'s JSON.stringify fallback silently produces '{}' for any bare Error, since message/stack/name are non-enumerable own properties. This was masking the actual error above and likely every other uncaught failure in the CLI. Error objects are now special-cased to print their stack. --- src/command/activate/activate-command.test.ts | 33 +++++++++++++++++++ src/command/activate/activate-command.ts | 10 ++++++ src/core/logger/index.test.ts | 28 ++++++++++++++++ src/core/logger/index.ts | 7 ++++ 4 files changed, 78 insertions(+) diff --git a/src/command/activate/activate-command.test.ts b/src/command/activate/activate-command.test.ts index 4951d0dc..fe2c6433 100644 --- a/src/command/activate/activate-command.test.ts +++ b/src/command/activate/activate-command.test.ts @@ -4,6 +4,7 @@ import { Docker } from '../../model/index.ts'; import { MacBuilder } from '../../model/mac-builder.ts'; import { PlatformSetup } from '../../logic/unity/platform-setup/index.ts'; import { PlatformValidation } from '../../logic/unity/platform-validation/platform-validation.ts'; +import { yargs } from '../../dependencies.ts'; const originalDockerRun = Docker.run; const originalMacBuilderRun = MacBuilder.run; @@ -62,4 +63,36 @@ describe('ActivateCommand', () => { expect(baseOptions.activateOnly).toBeUndefined(); }); + + it('defaults targetPlatform to NoTarget, not the build-oriented StandaloneWindows64 default', async () => { + // Real bug (game-ci/unity-activate#111 thin-wrapper CI): `game-ci + // activate ` with no --target-platform inherited UnityOptions' + // build default of StandaloneWindows64, which threw "Windows-based + // builds are only supported on 2019.3.X+" for older Unity versions even + // though activation doesn't build anything. NoTarget maps to the + // generic image and skips that check entirely. + const parser = yargs([]).exitProcess(false).fail((message, error) => { + throw error || new Error(message); + }); + + const command = new ActivateCommand('activate'); + await command.configureOptions(parser as any); + + const argv = await parser.parseAsync(); + + expect(argv.targetPlatform).toBe('NoTarget'); + }); + + it('still honors an explicitly passed targetPlatform', async () => { + const parser = yargs(['--targetPlatform', 'StandaloneLinux64']).exitProcess(false).fail((message, error) => { + throw error || new Error(message); + }); + + const command = new ActivateCommand('activate'); + await command.configureOptions(parser as any); + + const argv = await parser.parseAsync(); + + expect(argv.targetPlatform).toBe('StandaloneLinux64'); + }); }); diff --git a/src/command/activate/activate-command.ts b/src/command/activate/activate-command.ts index 7a4d1ede..ab602795 100644 --- a/src/command/activate/activate-command.ts +++ b/src/command/activate/activate-command.ts @@ -7,6 +7,7 @@ import { UnityOptions } from '../../command-options/unity-options.ts'; import type { YargsInstance, Options } from '../../dependencies.ts'; import { PlatformValidation } from '../../logic/unity/platform-validation/platform-validation.ts'; import { ProjectOptions } from '../../command-options/project-options.ts'; +import { UnityTargetPlatform } from '../../model/unity/target-platform/unity-target-platform.ts'; /** * Activates (and only activates) a Unity license, leaving it active for a @@ -44,6 +45,15 @@ export class ActivateCommand extends CommandBase implements CommandInterface { public async configureOptions(yargs: YargsInstance): Promise { await ProjectOptions.configure(yargs); await UnityOptions.configure(yargs); + // UnityOptions.configure() defaults targetPlatform to StandaloneWindows64 + // (a build-oriented default). Activation doesn't build anything - it + // just needs *an* editor image to run license activation inside - so + // override the default to NoTarget, which RunnerImageTag maps to the + // generic image and skips build-target validation entirely (e.g. the + // "Windows builds need 2019.3+" check, which has nothing to do with + // activating a license). Callers can still pass --target-platform + // explicitly if they have a reason to. + yargs.option('targetPlatform', { default: UnityTargetPlatform.NoTarget }); // Needed by Docker.run() for the container mount path - normally comes // from BuildOptions, but `activate` intentionally doesn't pull in // BuildOptions' build-specific flags (buildName, buildMethod, etc.), diff --git a/src/core/logger/index.test.ts b/src/core/logger/index.test.ts index 55ac29b2..1ece5c7b 100644 --- a/src/core/logger/index.test.ts +++ b/src/core/logger/index.test.ts @@ -61,3 +61,31 @@ describe('logger groups', () => { expect(result).toBe(42); }); }); + +describe('logger error formatting', () => { + const originalConsoleError = console.error; + let errorLines: string[] = []; + + beforeEach(async () => { + errorLines = []; + console.error = mock((...args: any[]) => { + errorLines.push(args.join(' ')); + }); + await configureLogger(Verbosity.normal); + }); + + afterEach(() => { + console.error = originalConsoleError; + }); + + it('prints an Error object\'s message and stack, not "{}"', () => { + // Real bug: Error's message/stack/name are non-enumerable own + // properties, so JSON.stringify(error) - the previous fallback for any + // non-string value - silently produced '{}', masking every uncaught + // failure's actual message (see game-ci/unity-activate#111). + (globalThis as any).log.error(new Error('something specific broke')); + + expect(errorLines.some((line) => line.includes('something specific broke'))).toBe(true); + expect(errorLines.some((line) => line.trim() === '[ERROR] {}')).toBe(false); + }); +}); diff --git a/src/core/logger/index.ts b/src/core/logger/index.ts index c8bd9496..34a8e14a 100644 --- a/src/core/logger/index.ts +++ b/src/core/logger/index.ts @@ -48,6 +48,13 @@ export const configureLogger = async (verbosity: Verbosity) => { if (value === undefined) return 'undefined'; if (value === null) return 'null'; if (typeof value === 'string') return value; + // Error's message/stack/name are non-enumerable own properties, so + // JSON.stringify(error) below silently produces '{}' for any bare Error + // - masking every uncaught failure's actual message. Must be handled + // before the JSON.stringify fallback, not caught by it. + if (value instanceof Error) { + return value.stack || `${value.name}: ${value.message}`; + } try { return JSON.stringify(value, null, 2); } catch {