mirror of
https://github.com/QwenLM/qwen-code.git
synced 2025-12-20 16:57:46 +00:00
[Fix Telemetry for tool calls, PR 1/n] Propagate tool reported errors via ToolCallResponseInfo and ToolResult (#5222)
This commit is contained in:
@@ -27,6 +27,7 @@ vi.mock('../utils/editor.js', () => ({
|
||||
import { describe, it, expect, beforeEach, afterEach, vi, Mock } from 'vitest';
|
||||
import { EditTool, EditToolParams } from './edit.js';
|
||||
import { FileDiff } from './tools.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import path from 'path';
|
||||
import fs from 'fs';
|
||||
import os from 'os';
|
||||
@@ -627,6 +628,98 @@ describe('EditTool', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Error Scenarios', () => {
|
||||
const testFile = 'error_test.txt';
|
||||
let filePath: string;
|
||||
|
||||
beforeEach(() => {
|
||||
filePath = path.join(rootDir, testFile);
|
||||
});
|
||||
|
||||
it('should return FILE_NOT_FOUND error', async () => {
|
||||
const params: EditToolParams = {
|
||||
file_path: filePath,
|
||||
old_string: 'any',
|
||||
new_string: 'new',
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.FILE_NOT_FOUND);
|
||||
});
|
||||
|
||||
it('should return ATTEMPT_TO_CREATE_EXISTING_FILE error', async () => {
|
||||
fs.writeFileSync(filePath, 'existing content', 'utf8');
|
||||
const params: EditToolParams = {
|
||||
file_path: filePath,
|
||||
old_string: '',
|
||||
new_string: 'new content',
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(
|
||||
ToolErrorType.ATTEMPT_TO_CREATE_EXISTING_FILE,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return NO_OCCURRENCE_FOUND error', async () => {
|
||||
fs.writeFileSync(filePath, 'content', 'utf8');
|
||||
const params: EditToolParams = {
|
||||
file_path: filePath,
|
||||
old_string: 'not-found',
|
||||
new_string: 'new',
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.EDIT_NO_OCCURRENCE_FOUND);
|
||||
});
|
||||
|
||||
it('should return EXPECTED_OCCURRENCE_MISMATCH error', async () => {
|
||||
fs.writeFileSync(filePath, 'one one two', 'utf8');
|
||||
const params: EditToolParams = {
|
||||
file_path: filePath,
|
||||
old_string: 'one',
|
||||
new_string: 'new',
|
||||
expected_replacements: 3,
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(
|
||||
ToolErrorType.EDIT_EXPECTED_OCCURRENCE_MISMATCH,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return NO_CHANGE error', async () => {
|
||||
fs.writeFileSync(filePath, 'content', 'utf8');
|
||||
const params: EditToolParams = {
|
||||
file_path: filePath,
|
||||
old_string: 'content',
|
||||
new_string: 'content',
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.EDIT_NO_CHANGE);
|
||||
});
|
||||
|
||||
it('should return INVALID_PARAMETERS error for relative path', async () => {
|
||||
const params: EditToolParams = {
|
||||
file_path: 'relative/path.txt',
|
||||
old_string: 'a',
|
||||
new_string: 'b',
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.INVALID_TOOL_PARAMS);
|
||||
});
|
||||
|
||||
it('should return FILE_WRITE_FAILURE on write error', async () => {
|
||||
fs.writeFileSync(filePath, 'content', 'utf8');
|
||||
// Make file readonly to trigger a write error
|
||||
fs.chmodSync(filePath, '444');
|
||||
|
||||
const params: EditToolParams = {
|
||||
file_path: filePath,
|
||||
old_string: 'content',
|
||||
new_string: 'new content',
|
||||
};
|
||||
const result = await tool.execute(params, new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getDescription', () => {
|
||||
it('should return "No file changes to..." if old_string and new_string are the same', () => {
|
||||
const testFileName = 'test.txt';
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
ToolResult,
|
||||
ToolResultDisplay,
|
||||
} from './tools.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import { Type } from '@google/genai';
|
||||
import { SchemaValidator } from '../utils/schemaValidator.js';
|
||||
import { makeRelative, shortenPath } from '../utils/paths.js';
|
||||
@@ -62,7 +63,7 @@ interface CalculatedEdit {
|
||||
currentContent: string | null;
|
||||
newContent: string;
|
||||
occurrences: number;
|
||||
error?: { display: string; raw: string };
|
||||
error?: { display: string; raw: string; type: ToolErrorType };
|
||||
isNewFile: boolean;
|
||||
}
|
||||
|
||||
@@ -191,7 +192,9 @@ Expectation for required parameters:
|
||||
let finalNewString = params.new_string;
|
||||
let finalOldString = params.old_string;
|
||||
let occurrences = 0;
|
||||
let error: { display: string; raw: string } | undefined = undefined;
|
||||
let error:
|
||||
| { display: string; raw: string; type: ToolErrorType }
|
||||
| undefined = undefined;
|
||||
|
||||
try {
|
||||
currentContent = fs.readFileSync(params.file_path, 'utf8');
|
||||
@@ -214,6 +217,7 @@ Expectation for required parameters:
|
||||
error = {
|
||||
display: `File not found. Cannot apply edit. Use an empty old_string to create a new file.`,
|
||||
raw: `File not found: ${params.file_path}`,
|
||||
type: ToolErrorType.FILE_NOT_FOUND,
|
||||
};
|
||||
} else if (currentContent !== null) {
|
||||
// Editing an existing file
|
||||
@@ -233,11 +237,13 @@ Expectation for required parameters:
|
||||
error = {
|
||||
display: `Failed to edit. Attempted to create a file that already exists.`,
|
||||
raw: `File already exists, cannot create: ${params.file_path}`,
|
||||
type: ToolErrorType.ATTEMPT_TO_CREATE_EXISTING_FILE,
|
||||
};
|
||||
} else if (occurrences === 0) {
|
||||
error = {
|
||||
display: `Failed to edit, could not find the string to replace.`,
|
||||
raw: `Failed to edit, 0 occurrences found for old_string in ${params.file_path}. No edits made. The exact text in old_string was not found. Ensure you're not escaping content incorrectly and check whitespace, indentation, and context. Use ${ReadFileTool.Name} tool to verify.`,
|
||||
type: ToolErrorType.EDIT_NO_OCCURRENCE_FOUND,
|
||||
};
|
||||
} else if (occurrences !== expectedReplacements) {
|
||||
const occurrenceTerm =
|
||||
@@ -246,11 +252,13 @@ Expectation for required parameters:
|
||||
error = {
|
||||
display: `Failed to edit, expected ${expectedReplacements} ${occurrenceTerm} but found ${occurrences}.`,
|
||||
raw: `Failed to edit, Expected ${expectedReplacements} ${occurrenceTerm} but found ${occurrences} for old_string in file: ${params.file_path}`,
|
||||
type: ToolErrorType.EDIT_EXPECTED_OCCURRENCE_MISMATCH,
|
||||
};
|
||||
} else if (finalOldString === finalNewString) {
|
||||
error = {
|
||||
display: `No changes to apply. The old_string and new_string are identical.`,
|
||||
raw: `No changes to apply. The old_string and new_string are identical in file: ${params.file_path}`,
|
||||
type: ToolErrorType.EDIT_NO_CHANGE,
|
||||
};
|
||||
}
|
||||
} else {
|
||||
@@ -258,6 +266,7 @@ Expectation for required parameters:
|
||||
error = {
|
||||
display: `Failed to read content of file.`,
|
||||
raw: `Failed to read content of existing file: ${params.file_path}`,
|
||||
type: ToolErrorType.READ_CONTENT_FAILURE,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -374,6 +383,10 @@ Expectation for required parameters:
|
||||
return {
|
||||
llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
|
||||
returnDisplay: `Error: ${validationError}`,
|
||||
error: {
|
||||
message: validationError,
|
||||
type: ToolErrorType.INVALID_TOOL_PARAMS,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -385,6 +398,10 @@ Expectation for required parameters:
|
||||
return {
|
||||
llmContent: `Error preparing edit: ${errorMsg}`,
|
||||
returnDisplay: `Error preparing edit: ${errorMsg}`,
|
||||
error: {
|
||||
message: errorMsg,
|
||||
type: ToolErrorType.EDIT_PREPARATION_FAILURE,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -392,6 +409,10 @@ Expectation for required parameters:
|
||||
return {
|
||||
llmContent: editData.error.raw,
|
||||
returnDisplay: `Error: ${editData.error.display}`,
|
||||
error: {
|
||||
message: editData.error.raw,
|
||||
type: editData.error.type,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -442,6 +463,10 @@ Expectation for required parameters:
|
||||
return {
|
||||
llmContent: `Error executing edit: ${errorMsg}`,
|
||||
returnDisplay: `Error writing file: ${errorMsg}`,
|
||||
error: {
|
||||
message: errorMsg,
|
||||
type: ToolErrorType.FILE_WRITE_FAILURE,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
28
packages/core/src/tools/tool-error.ts
Normal file
28
packages/core/src/tools/tool-error.ts
Normal file
@@ -0,0 +1,28 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
/**
|
||||
* A type-safe enum for tool-related errors.
|
||||
*/
|
||||
export enum ToolErrorType {
|
||||
// General Errors
|
||||
INVALID_TOOL_PARAMS = 'invalid_tool_params',
|
||||
UNKNOWN = 'unknown',
|
||||
UNHANDLED_EXCEPTION = 'unhandled_exception',
|
||||
TOOL_NOT_REGISTERED = 'tool_not_registered',
|
||||
|
||||
// File System Errors
|
||||
FILE_NOT_FOUND = 'file_not_found',
|
||||
FILE_WRITE_FAILURE = 'file_write_failure',
|
||||
READ_CONTENT_FAILURE = 'read_content_failure',
|
||||
ATTEMPT_TO_CREATE_EXISTING_FILE = 'attempt_to_create_existing_file',
|
||||
|
||||
// Edit-specific Errors
|
||||
EDIT_PREPARATION_FAILURE = 'edit_preparation_failure',
|
||||
EDIT_NO_OCCURRENCE_FOUND = 'edit_no_occurrence_found',
|
||||
EDIT_EXPECTED_OCCURRENCE_MISMATCH = 'edit_expected_occurrence_mismatch',
|
||||
EDIT_NO_CHANGE = 'edit_no_change',
|
||||
}
|
||||
@@ -5,6 +5,7 @@
|
||||
*/
|
||||
|
||||
import { FunctionDeclaration, PartListUnion, Schema } from '@google/genai';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
/**
|
||||
* Interface representing the base Tool functionality
|
||||
@@ -217,6 +218,14 @@ export interface ToolResult {
|
||||
* For now, we keep it as the core logic in ReadFileTool currently produces it.
|
||||
*/
|
||||
returnDisplay: ToolResultDisplay;
|
||||
|
||||
/**
|
||||
* If this property is present, the tool call is considered a failure.
|
||||
*/
|
||||
error?: {
|
||||
message: string; // raw error message
|
||||
type?: ToolErrorType; // An optional machine-readable error type (e.g., 'FILE_NOT_FOUND').
|
||||
};
|
||||
}
|
||||
|
||||
export type ToolResultDisplay = string | FileDiff;
|
||||
|
||||
Reference in New Issue
Block a user