fix: enhance error handling and output for non-interactive authentication in JSON and STREAM_JSON modes

This commit is contained in:
mingholy.lmh
2025-11-18 11:34:46 +08:00
parent 93999e45e7
commit cfa7f43572
6 changed files with 478 additions and 72 deletions

1
.gitignore vendored
View File

@@ -57,4 +57,3 @@ gha-creds-*.json
# Log files
patch_output.log
QWEN.md

View File

@@ -74,35 +74,236 @@ describe('JSON output', () => {
expect(thrown).toBeDefined();
const message = (thrown as Error).message;
// The error JSON is written to stderr, so it should be in the error message
// Use a regex to find the first complete JSON object in the string
const jsonMatch = message.match(/{[\s\S]*}/);
// Fail if no JSON-like text was found
// The error JSON is written to stdout as a CLIResultMessageError
// Extract stdout from the error message
const stdoutMatch = message.match(/Stdout:\n([\s\S]*?)(?:\n\nStderr:|$)/);
expect(
jsonMatch,
'Expected to find a JSON object in the error output',
stdoutMatch,
'Expected to find stdout in the error message',
).toBeTruthy();
let payload;
const stdout = stdoutMatch![1];
let parsed: unknown[];
try {
// Parse the matched JSON string
payload = JSON.parse(jsonMatch![0]);
// Parse the JSON array from stdout
parsed = JSON.parse(stdout);
} catch (parseError) {
console.error('Failed to parse the following JSON:', jsonMatch![0]);
console.error('Failed to parse the following JSON:', stdout);
throw new Error(
`Test failed: Could not parse JSON from error message. Details: ${parseError}`,
`Test failed: Could not parse JSON from stdout. Details: ${parseError}`,
);
}
// The JsonFormatter.formatError() outputs: { error: { type, message, code } }
expect(payload).toHaveProperty('error');
expect(payload.error).toBeDefined();
expect(payload.error.type).toBe('Error');
expect(payload.error.code).toBe(1);
expect(payload.error.message).toContain(
// The output should be an array of messages
expect(Array.isArray(parsed)).toBe(true);
expect(parsed.length).toBeGreaterThan(0);
// Find the result message with error
const resultMessage = parsed.find(
(msg: unknown) =>
typeof msg === 'object' &&
msg !== null &&
'type' in msg &&
msg.type === 'result' &&
'is_error' in msg &&
msg.is_error === true,
) as {
type: string;
is_error: boolean;
subtype: string;
error?: { message: string; type?: string };
};
expect(resultMessage).toBeDefined();
expect(resultMessage.is_error).toBe(true);
expect(resultMessage).toHaveProperty('subtype');
expect(resultMessage.subtype).toBe('error_during_execution');
expect(resultMessage).toHaveProperty('error');
expect(resultMessage.error).toBeDefined();
expect(resultMessage.error?.message).toContain(
'configured auth type is qwen-oauth',
);
expect(payload.error.message).toContain('current auth type is openai');
expect(resultMessage.error?.message).toContain(
'current auth type is openai',
);
});
it('should return line-delimited JSON messages for stream-json output format', async () => {
const result = await rig.run(
'What is the capital of France?',
'--output-format',
'stream-json',
);
// Stream-json output is line-delimited JSON (one JSON object per line)
const lines = result
.trim()
.split('\n')
.filter((line) => line.trim());
expect(lines.length).toBeGreaterThan(0);
// Parse each line as a JSON object
const messages: unknown[] = [];
for (const line of lines) {
try {
const parsed = JSON.parse(line);
messages.push(parsed);
} catch (parseError) {
throw new Error(
`Failed to parse JSON line: ${line}. Error: ${parseError}`,
);
}
}
// Should have at least system, assistant, and result messages
expect(messages.length).toBeGreaterThanOrEqual(3);
// Find system message
const systemMessage = messages.find(
(msg: unknown) =>
typeof msg === 'object' &&
msg !== null &&
'type' in msg &&
msg.type === 'system',
);
expect(systemMessage).toBeDefined();
expect(systemMessage).toHaveProperty('subtype');
expect(systemMessage).toHaveProperty('session_id');
// Find assistant message
const assistantMessage = messages.find(
(msg: unknown) =>
typeof msg === 'object' &&
msg !== null &&
'type' in msg &&
msg.type === 'assistant',
);
expect(assistantMessage).toBeDefined();
expect(assistantMessage).toHaveProperty('message');
expect(assistantMessage).toHaveProperty('session_id');
// Find result message (should be the last message)
const resultMessage = messages[messages.length - 1] as {
type: string;
is_error: boolean;
result: string;
};
expect(resultMessage).toBeDefined();
expect(
typeof resultMessage === 'object' &&
resultMessage !== null &&
'type' in resultMessage &&
resultMessage.type === 'result',
).toBe(true);
expect(resultMessage).toHaveProperty('is_error');
expect(resultMessage.is_error).toBe(false);
expect(resultMessage).toHaveProperty('result');
expect(typeof resultMessage.result).toBe('string');
expect(resultMessage.result.toLowerCase()).toContain('paris');
});
it('should include stream events when using stream-json with include-partial-messages', async () => {
const result = await rig.run(
'What is the capital of France?',
'--output-format',
'stream-json',
'--include-partial-messages',
);
// Stream-json output is line-delimited JSON (one JSON object per line)
const lines = result
.trim()
.split('\n')
.filter((line) => line.trim());
expect(lines.length).toBeGreaterThan(0);
// Parse each line as a JSON object
const messages: unknown[] = [];
for (const line of lines) {
try {
const parsed = JSON.parse(line);
messages.push(parsed);
} catch (parseError) {
throw new Error(
`Failed to parse JSON line: ${line}. Error: ${parseError}`,
);
}
}
// Should have more messages than without include-partial-messages
// because we're including stream events
expect(messages.length).toBeGreaterThan(3);
// Find stream_event messages
const streamEvents = messages.filter(
(msg: unknown) =>
typeof msg === 'object' &&
msg !== null &&
'type' in msg &&
msg.type === 'stream_event',
);
expect(streamEvents.length).toBeGreaterThan(0);
// Verify stream event structure
const firstStreamEvent = streamEvents[0];
expect(firstStreamEvent).toHaveProperty('event');
expect(firstStreamEvent).toHaveProperty('session_id');
expect(firstStreamEvent).toHaveProperty('uuid');
// Check for expected stream event types
const eventTypes = streamEvents.map((event: unknown) =>
typeof event === 'object' &&
event !== null &&
'event' in event &&
typeof event.event === 'object' &&
event.event !== null &&
'type' in event.event
? event.event.type
: null,
);
// Should have message_start event
expect(eventTypes).toContain('message_start');
// Should have content_block_start event
expect(eventTypes).toContain('content_block_start');
// Should have content_block_delta events
expect(eventTypes).toContain('content_block_delta');
// Should have content_block_stop event
expect(eventTypes).toContain('content_block_stop');
// Should have message_stop event
expect(eventTypes).toContain('message_stop');
// Verify that we still have the complete assistant message
const assistantMessage = messages.find(
(msg: unknown) =>
typeof msg === 'object' &&
msg !== null &&
'type' in msg &&
msg.type === 'assistant',
);
expect(assistantMessage).toBeDefined();
expect(assistantMessage).toHaveProperty('message');
// Verify that we still have the result message
const resultMessage = messages[messages.length - 1] as {
type: string;
is_error: boolean;
result: string;
};
expect(resultMessage).toBeDefined();
expect(
typeof resultMessage === 'object' &&
resultMessage !== null &&
'type' in resultMessage &&
resultMessage.type === 'result',
).toBe(true);
expect(resultMessage).toHaveProperty('is_error');
expect(resultMessage.is_error).toBe(false);
expect(resultMessage).toHaveProperty('result');
expect(resultMessage.result.toLowerCase()).toContain('paris');
});
});

View File

@@ -340,7 +340,8 @@ export class TestRig {
// as it would corrupt the JSON
const isJsonOutput =
commandArgs.includes('--output-format') &&
commandArgs.includes('json');
(commandArgs.includes('json') ||
commandArgs.includes('stream-json'));
// If we have stderr output and it's not a JSON test, include that also
if (stderr && !isJsonOutput) {
@@ -348,9 +349,25 @@ export class TestRig {
}
resolve(result);
} else {
// Check if this is a JSON output test - for JSON errors, the error is in stdout
const isJsonOutputOnError =
commandArgs.includes('--output-format') &&
(commandArgs.includes('json') ||
commandArgs.includes('stream-json'));
// For JSON output tests, include stdout in the error message
// as the error JSON is written to stdout
if (isJsonOutputOnError && stdout) {
reject(
new Error(
`Process exited with code ${code}:\nStdout:\n${stdout}\n\nStderr:\n${stderr}`,
),
);
} else {
reject(new Error(`Process exited with code ${code}:\n${stderr}`));
}
}
});
});

View File

@@ -16,10 +16,6 @@
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js"
},
"./protocol": {
"types": "./dist/src/types/protocol.d.ts",
"import": "./dist/src/types/protocol.js"
}
},
"scripts": {

View File

@@ -10,6 +10,9 @@ import { AuthType, OutputFormat } from '@qwen-code/qwen-code-core';
import type { Config } from '@qwen-code/qwen-code-core';
import * as auth from './config/auth.js';
import { type LoadedSettings } from './config/settings.js';
import * as JsonOutputAdapterModule from './nonInteractive/io/JsonOutputAdapter.js';
import * as StreamJsonOutputAdapterModule from './nonInteractive/io/StreamJsonOutputAdapter.js';
import * as cleanupModule from './utils/cleanup.js';
describe('validateNonInterActiveAuth', () => {
let originalEnvGeminiApiKey: string | undefined;
@@ -17,8 +20,8 @@ describe('validateNonInterActiveAuth', () => {
let originalEnvGcp: string | undefined;
let originalEnvOpenAiApiKey: string | undefined;
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
let processExitSpy: ReturnType<typeof vi.spyOn>;
let refreshAuthMock: vi.Mock;
let processExitSpy: ReturnType<typeof vi.spyOn<[code?: number], never>>;
let refreshAuthMock: ReturnType<typeof vi.fn>;
let mockSettings: LoadedSettings;
beforeEach(() => {
@@ -33,7 +36,7 @@ describe('validateNonInterActiveAuth', () => {
consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
processExitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => {
throw new Error(`process.exit(${code}) called`);
});
}) as ReturnType<typeof vi.spyOn<[code?: number], never>>;
refreshAuthMock = vi.fn().mockResolvedValue('refreshed');
mockSettings = {
system: { path: '', settings: {} },
@@ -235,7 +238,24 @@ describe('validateNonInterActiveAuth', () => {
});
describe('JSON output mode', () => {
it('prints JSON error when no auth is configured and exits with code 1', async () => {
let emitResultMock: ReturnType<typeof vi.fn>;
let runExitCleanupMock: ReturnType<typeof vi.fn>;
beforeEach(() => {
emitResultMock = vi.fn();
runExitCleanupMock = vi.fn().mockResolvedValue(undefined);
vi.spyOn(JsonOutputAdapterModule, 'JsonOutputAdapter').mockImplementation(
() =>
({
emitResult: emitResultMock,
}) as unknown as JsonOutputAdapterModule.JsonOutputAdapter,
);
vi.spyOn(cleanupModule, 'runExitCleanup').mockImplementation(
runExitCleanupMock,
);
});
it('emits error result and exits when no auth is configured', async () => {
const nonInteractiveConfig = {
refreshAuth: refreshAuthMock,
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON),
@@ -244,7 +264,6 @@ describe('validateNonInterActiveAuth', () => {
.mockReturnValue({ authType: undefined }),
} as unknown as Config;
let thrown: Error | undefined;
try {
await validateNonInteractiveAuth(
undefined,
@@ -252,21 +271,27 @@ describe('validateNonInterActiveAuth', () => {
nonInteractiveConfig,
mockSettings,
);
expect.fail('Should have exited');
} catch (e) {
thrown = e as Error;
expect((e as Error).message).toContain('process.exit(1) called');
}
expect(thrown?.message).toBe('process.exit(1) called');
const errorArg = consoleErrorSpy.mock.calls[0]?.[0] as string;
const payload = JSON.parse(errorArg);
expect(payload.error.type).toBe('Error');
expect(payload.error.code).toBe(1);
expect(payload.error.message).toContain(
expect(emitResultMock).toHaveBeenCalledWith({
isError: true,
errorMessage: expect.stringContaining(
'Please set an Auth method in your',
);
),
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
expect(runExitCleanupMock).toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
it('prints JSON error when enforced auth mismatches current auth and exits with code 1', async () => {
it('emits error result and exits when enforced auth mismatches current auth', async () => {
mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH;
process.env['OPENAI_API_KEY'] = 'fake-key';
@@ -278,7 +303,6 @@ describe('validateNonInterActiveAuth', () => {
.mockReturnValue({ authType: undefined }),
} as unknown as Config;
let thrown: Error | undefined;
try {
await validateNonInteractiveAuth(
undefined,
@@ -286,23 +310,27 @@ describe('validateNonInterActiveAuth', () => {
nonInteractiveConfig,
mockSettings,
);
expect.fail('Should have exited');
} catch (e) {
thrown = e as Error;
expect((e as Error).message).toContain('process.exit(1) called');
}
expect(thrown?.message).toBe('process.exit(1) called');
{
const errorArg = consoleErrorSpy.mock.calls[0]?.[0] as string;
const payload = JSON.parse(errorArg);
expect(payload.error.type).toBe('Error');
expect(payload.error.code).toBe(1);
expect(payload.error.message).toContain(
expect(emitResultMock).toHaveBeenCalledWith({
isError: true,
errorMessage: expect.stringContaining(
'The configured auth type is qwen-oauth, but the current auth type is openai.',
);
}
),
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
expect(runExitCleanupMock).toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
it('prints JSON error when validateAuthMethod fails and exits with code 1', async () => {
it('emits error result and exits when validateAuthMethod fails', async () => {
vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!');
process.env['OPENAI_API_KEY'] = 'fake-key';
@@ -314,7 +342,6 @@ describe('validateNonInterActiveAuth', () => {
.mockReturnValue({ authType: undefined }),
} as unknown as Config;
let thrown: Error | undefined;
try {
await validateNonInteractiveAuth(
AuthType.USE_OPENAI,
@@ -322,18 +349,159 @@ describe('validateNonInterActiveAuth', () => {
nonInteractiveConfig,
mockSettings,
);
expect.fail('Should have exited');
} catch (e) {
thrown = e as Error;
expect((e as Error).message).toContain('process.exit(1) called');
}
expect(thrown?.message).toBe('process.exit(1) called');
{
const errorArg = consoleErrorSpy.mock.calls[0]?.[0] as string;
const payload = JSON.parse(errorArg);
expect(payload.error.type).toBe('Error');
expect(payload.error.code).toBe(1);
expect(payload.error.message).toBe('Auth error!');
expect(emitResultMock).toHaveBeenCalledWith({
isError: true,
errorMessage: 'Auth error!',
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
expect(runExitCleanupMock).toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
});
describe('STREAM_JSON output mode', () => {
let emitResultMock: ReturnType<typeof vi.fn>;
let runExitCleanupMock: ReturnType<typeof vi.fn>;
beforeEach(() => {
emitResultMock = vi.fn();
runExitCleanupMock = vi.fn().mockResolvedValue(undefined);
vi.spyOn(
StreamJsonOutputAdapterModule,
'StreamJsonOutputAdapter',
).mockImplementation(
() =>
({
emitResult: emitResultMock,
}) as unknown as StreamJsonOutputAdapterModule.StreamJsonOutputAdapter,
);
vi.spyOn(cleanupModule, 'runExitCleanup').mockImplementation(
runExitCleanupMock,
);
});
it('emits error result and exits when no auth is configured', async () => {
const nonInteractiveConfig = {
refreshAuth: refreshAuthMock,
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON),
getIncludePartialMessages: vi.fn().mockReturnValue(false),
getContentGeneratorConfig: vi
.fn()
.mockReturnValue({ authType: undefined }),
} as unknown as Config;
try {
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
mockSettings,
);
expect.fail('Should have exited');
} catch (e) {
expect((e as Error).message).toContain('process.exit(1) called');
}
expect(emitResultMock).toHaveBeenCalledWith({
isError: true,
errorMessage: expect.stringContaining(
'Please set an Auth method in your',
),
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
expect(runExitCleanupMock).toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
it('emits error result and exits when enforced auth mismatches current auth', async () => {
mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH;
process.env['OPENAI_API_KEY'] = 'fake-key';
const nonInteractiveConfig = {
refreshAuth: refreshAuthMock,
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON),
getIncludePartialMessages: vi.fn().mockReturnValue(false),
getContentGeneratorConfig: vi
.fn()
.mockReturnValue({ authType: undefined }),
} as unknown as Config;
try {
await validateNonInteractiveAuth(
undefined,
undefined,
nonInteractiveConfig,
mockSettings,
);
expect.fail('Should have exited');
} catch (e) {
expect((e as Error).message).toContain('process.exit(1) called');
}
expect(emitResultMock).toHaveBeenCalledWith({
isError: true,
errorMessage: expect.stringContaining(
'The configured auth type is qwen-oauth, but the current auth type is openai.',
),
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
expect(runExitCleanupMock).toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
it('emits error result and exits when validateAuthMethod fails', async () => {
vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!');
process.env['OPENAI_API_KEY'] = 'fake-key';
const nonInteractiveConfig = {
refreshAuth: refreshAuthMock,
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON),
getIncludePartialMessages: vi.fn().mockReturnValue(false),
getContentGeneratorConfig: vi
.fn()
.mockReturnValue({ authType: undefined }),
} as unknown as Config;
try {
await validateNonInteractiveAuth(
AuthType.USE_OPENAI,
undefined,
nonInteractiveConfig,
mockSettings,
);
expect.fail('Should have exited');
} catch (e) {
expect((e as Error).message).toContain('process.exit(1) called');
}
expect(emitResultMock).toHaveBeenCalledWith({
isError: true,
errorMessage: 'Auth error!',
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
expect(runExitCleanupMock).toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(consoleErrorSpy).not.toHaveBeenCalled();
});
});
});

View File

@@ -9,7 +9,9 @@ import { AuthType, OutputFormat } from '@qwen-code/qwen-code-core';
import { USER_SETTINGS_PATH } from './config/settings.js';
import { validateAuthMethod } from './config/auth.js';
import { type LoadedSettings } from './config/settings.js';
import { handleError } from './utils/errors.js';
import { JsonOutputAdapter } from './nonInteractive/io/JsonOutputAdapter.js';
import { StreamJsonOutputAdapter } from './nonInteractive/io/StreamJsonOutputAdapter.js';
import { runExitCleanup } from './utils/cleanup.js';
function getAuthTypeFromEnv(): AuthType | undefined {
if (process.env['OPENAI_API_KEY']) {
@@ -27,7 +29,7 @@ export async function validateNonInteractiveAuth(
useExternalAuth: boolean | undefined,
nonInteractiveConfig: Config,
settings: LoadedSettings,
) {
): Promise<Config> {
try {
const enforcedType = settings.merged.security?.auth?.enforcedType;
if (enforcedType) {
@@ -58,15 +60,38 @@ export async function validateNonInteractiveAuth(
await nonInteractiveConfig.refreshAuth(authType);
return nonInteractiveConfig;
} catch (error) {
if (nonInteractiveConfig.getOutputFormat() === OutputFormat.JSON) {
handleError(
error instanceof Error ? error : new Error(String(error)),
nonInteractiveConfig,
1,
);
const outputFormat = nonInteractiveConfig.getOutputFormat();
// In JSON and STREAM_JSON modes, emit error result and exit
if (
outputFormat === OutputFormat.JSON ||
outputFormat === OutputFormat.STREAM_JSON
) {
let adapter;
if (outputFormat === OutputFormat.JSON) {
adapter = new JsonOutputAdapter(nonInteractiveConfig);
} else {
adapter = new StreamJsonOutputAdapter(
nonInteractiveConfig,
nonInteractiveConfig.getIncludePartialMessages(),
);
}
const errorMessage =
error instanceof Error ? error.message : String(error);
adapter.emitResult({
isError: true,
errorMessage,
durationMs: 0,
apiDurationMs: 0,
numTurns: 0,
usage: undefined,
});
await runExitCleanup();
process.exit(1);
}
// For other modes (text), use existing error handling
console.error(error instanceof Error ? error.message : String(error));
process.exit(1);
}
}
}