refactor: Improve error handling and reporting in tool executions

This commit is contained in:
Amar Sood (tekacs) 2025-04-12 12:54:57 -04:00
parent 10d0f90cb5
commit 9b56e1f099
4 changed files with 104 additions and 143 deletions

View file

@ -1,6 +1,6 @@
import os import os
import traceback import traceback
from .tool_utils import generate_unified_diff_snippet from .tool_utils import ToolError, generate_unified_diff_snippet, handle_tool_error, format_tool_result, apply_change
def _execute_delete_line(coder, file_path, line_number, change_id=None, dry_run=False): def _execute_delete_line(coder, file_path, line_number, change_id=None, dry_run=False):
""" """
@ -15,6 +15,8 @@ def _execute_delete_line(coder, file_path, line_number, change_id=None, dry_run=
Returns a result message. Returns a result message.
""" """
tool_name = "DeleteLine"
try: try:
# Get absolute file path # Get absolute file path
abs_path = coder.abs_root_path(file_path) abs_path = coder.abs_root_path(file_path)
@ -22,23 +24,19 @@ def _execute_delete_line(coder, file_path, line_number, change_id=None, dry_run=
# Check if file exists # Check if file exists
if not os.path.isfile(abs_path): if not os.path.isfile(abs_path):
coder.io.tool_error(f"File '{file_path}' not found") raise ToolError(f"File '{file_path}' not found")
return f"Error: File not found"
# Check if file is in editable context # Check if file is in editable context
if abs_path not in coder.abs_fnames: if abs_path not in coder.abs_fnames:
if abs_path in coder.abs_read_only_fnames: if abs_path in coder.abs_read_only_fnames:
coder.io.tool_error(f"File '{file_path}' is read-only. Use MakeEditable first.") raise ToolError(f"File '{file_path}' is read-only. Use MakeEditable first.")
return f"Error: File is read-only. Use MakeEditable first."
else: else:
coder.io.tool_error(f"File '{file_path}' not in context") raise ToolError(f"File '{file_path}' not in context")
return f"Error: File not in context"
# Reread file content immediately before modification # Reread file content immediately before modification
file_content = coder.io.read_text(abs_path) file_content = coder.io.read_text(abs_path)
if file_content is None: if file_content is None:
coder.io.tool_error(f"Could not read file '{file_path}' before DeleteLine operation.") raise ToolError(f"Could not read file '{file_path}'")
return f"Error: Could not read file '{file_path}'"
lines = file_content.splitlines() lines = file_content.splitlines()
original_content = file_content original_content = file_content
@ -47,11 +45,10 @@ def _execute_delete_line(coder, file_path, line_number, change_id=None, dry_run=
try: try:
line_num_int = int(line_number) line_num_int = int(line_number)
if line_num_int < 1 or line_num_int > len(lines): if line_num_int < 1 or line_num_int > len(lines):
raise ValueError(f"Line number {line_num_int} is out of range (1-{len(lines)})") raise ToolError(f"Line number {line_num_int} is out of range (1-{len(lines)})")
line_idx = line_num_int - 1 # Convert to 0-based index line_idx = line_num_int - 1 # Convert to 0-based index
except ValueError as e: except ValueError:
coder.io.tool_error(f"Invalid line_number: {e}") raise ToolError(f"Invalid line_number value: '{line_number}'. Must be an integer.")
return f"Error: Invalid line_number '{line_number}'"
# Prepare the deletion # Prepare the deletion
deleted_line = lines[line_idx] deleted_line = lines[line_idx]
@ -62,40 +59,34 @@ def _execute_delete_line(coder, file_path, line_number, change_id=None, dry_run=
coder.io.tool_warning(f"No changes made: deleting line {line_num_int} would not change file") coder.io.tool_warning(f"No changes made: deleting line {line_num_int} would not change file")
return f"Warning: No changes made (deleting line {line_num_int} would not change file)" return f"Warning: No changes made (deleting line {line_num_int} would not change file)"
# Generate diff snippet (using the existing delete block helper for simplicity) # Generate diff snippet
diff_snippet = generate_unified_diff_snippet(original_content, new_content, rel_path) diff_snippet = generate_unified_diff_snippet(original_content, new_content, rel_path)
# Handle dry run # Handle dry run
if dry_run: if dry_run:
coder.io.tool_output(f"Dry run: Would delete line {line_num_int} in {file_path}") dry_run_message = f"Dry run: Would delete line {line_num_int} in {file_path}"
return f"Dry run: Would delete line {line_num_int}. Diff snippet:\n{diff_snippet}" return format_tool_result(coder, tool_name, "", dry_run=True, dry_run_message=dry_run_message, diff_snippet=diff_snippet)
# --- Apply Change (Not dry run) --- # --- Apply Change (Not dry run) ---
coder.io.write_text(abs_path, new_content) metadata = {
'line_number': line_num_int,
# Track the change 'deleted_content': deleted_line
try: }
metadata = { final_change_id = apply_change(
'line_number': line_num_int, coder, abs_path, rel_path, original_content, new_content, 'deleteline', metadata, change_id
'deleted_content': deleted_line )
}
change_id = coder.change_tracker.track_change(
file_path=rel_path,
change_type='deleteline',
original_content=original_content,
new_content=new_content,
metadata=metadata,
change_id=change_id
)
except Exception as track_e:
coder.io.tool_error(f"Error tracking change for DeleteLine: {track_e}")
change_id = "TRACKING_FAILED"
coder.aider_edited_files.add(rel_path) coder.aider_edited_files.add(rel_path)
coder.io.tool_output(f"✅ Deleted line {line_num_int} in {file_path} (change_id: {change_id})") # Format and return result
return f"Successfully deleted line {line_num_int} (change_id: {change_id}). Diff snippet:\n{diff_snippet}" success_message = f"Deleted line {line_num_int} in {file_path}"
return format_tool_result(
coder, tool_name, success_message, change_id=final_change_id, diff_snippet=diff_snippet
)
except ToolError as e:
# Handle errors raised by utility functions (expected errors)
return handle_tool_error(coder, tool_name, e, add_traceback=False)
except Exception as e: except Exception as e:
coder.io.tool_error(f"Error in DeleteLine: {str(e)}\n{traceback.format_exc()}") # Handle unexpected errors
return f"Error: {str(e)}" return handle_tool_error(coder, tool_name, e)

View file

@ -1,6 +1,6 @@
import os import os
import traceback import traceback
from .tool_utils import generate_unified_diff_snippet from .tool_utils import ToolError, generate_unified_diff_snippet, handle_tool_error, format_tool_result, apply_change
def _execute_delete_lines(coder, file_path, start_line, end_line, change_id=None, dry_run=False): def _execute_delete_lines(coder, file_path, start_line, end_line, change_id=None, dry_run=False):
""" """
@ -16,6 +16,7 @@ def _execute_delete_lines(coder, file_path, start_line, end_line, change_id=None
Returns a result message. Returns a result message.
""" """
tool_name = "DeleteLines"
try: try:
# Get absolute file path # Get absolute file path
abs_path = coder.abs_root_path(file_path) abs_path = coder.abs_root_path(file_path)
@ -23,23 +24,19 @@ def _execute_delete_lines(coder, file_path, start_line, end_line, change_id=None
# Check if file exists # Check if file exists
if not os.path.isfile(abs_path): if not os.path.isfile(abs_path):
coder.io.tool_error(f"File '{file_path}' not found") raise ToolError(f"File '{file_path}' not found")
return f"Error: File not found"
# Check if file is in editable context # Check if file is in editable context
if abs_path not in coder.abs_fnames: if abs_path not in coder.abs_fnames:
if abs_path in coder.abs_read_only_fnames: if abs_path in coder.abs_read_only_fnames:
coder.io.tool_error(f"File '{file_path}' is read-only. Use MakeEditable first.") raise ToolError(f"File '{file_path}' is read-only. Use MakeEditable first.")
return f"Error: File is read-only. Use MakeEditable first."
else: else:
coder.io.tool_error(f"File '{file_path}' not in context") raise ToolError(f"File '{file_path}' not in context")
return f"Error: File not in context"
# Reread file content immediately before modification # Reread file content immediately before modification
file_content = coder.io.read_text(abs_path) file_content = coder.io.read_text(abs_path)
if file_content is None: if file_content is None:
coder.io.tool_error(f"Could not read file '{file_path}' before DeleteLines operation.") raise ToolError(f"Could not read file '{file_path}'")
return f"Error: Could not read file '{file_path}'"
lines = file_content.splitlines() lines = file_content.splitlines()
original_content = file_content original_content = file_content
@ -50,17 +47,16 @@ def _execute_delete_lines(coder, file_path, start_line, end_line, change_id=None
end_line_int = int(end_line) end_line_int = int(end_line)
if start_line_int < 1 or start_line_int > len(lines): if start_line_int < 1 or start_line_int > len(lines):
raise ValueError(f"Start line {start_line_int} is out of range (1-{len(lines)})") raise ToolError(f"Start line {start_line_int} is out of range (1-{len(lines)})")
if end_line_int < 1 or end_line_int > len(lines): if end_line_int < 1 or end_line_int > len(lines):
raise ValueError(f"End line {end_line_int} is out of range (1-{len(lines)})") raise ToolError(f"End line {end_line_int} is out of range (1-{len(lines)})")
if start_line_int > end_line_int: if start_line_int > end_line_int:
raise ValueError(f"Start line {start_line_int} cannot be after end line {end_line_int}") raise ToolError(f"Start line {start_line_int} cannot be after end line {end_line_int}")
start_idx = start_line_int - 1 # Convert to 0-based index start_idx = start_line_int - 1 # Convert to 0-based index
end_idx = end_line_int - 1 # Convert to 0-based index end_idx = end_line_int - 1 # Convert to 0-based index
except ValueError as e: except ValueError:
coder.io.tool_error(f"Invalid line numbers: {e}") raise ToolError(f"Invalid line numbers: '{start_line}', '{end_line}'. Must be integers.")
return f"Error: Invalid line numbers '{start_line}', '{end_line}'"
# Prepare the deletion # Prepare the deletion
deleted_lines = lines[start_idx:end_idx+1] deleted_lines = lines[start_idx:end_idx+1]
@ -76,37 +72,31 @@ def _execute_delete_lines(coder, file_path, start_line, end_line, change_id=None
# Handle dry run # Handle dry run
if dry_run: if dry_run:
coder.io.tool_output(f"Dry run: Would delete lines {start_line_int}-{end_line_int} in {file_path}") dry_run_message = f"Dry run: Would delete lines {start_line_int}-{end_line_int} in {file_path}"
return f"Dry run: Would delete lines {start_line_int}-{end_line_int}. Diff snippet:\n{diff_snippet}" return format_tool_result(coder, tool_name, "", dry_run=True, dry_run_message=dry_run_message, diff_snippet=diff_snippet)
# --- Apply Change (Not dry run) --- # --- Apply Change (Not dry run) ---
coder.io.write_text(abs_path, new_content) metadata = {
'start_line': start_line_int,
# Track the change 'end_line': end_line_int,
try: 'deleted_content': '\n'.join(deleted_lines)
metadata = { }
'start_line': start_line_int,
'end_line': end_line_int, final_change_id = apply_change(
'deleted_content': '\n'.join(deleted_lines) coder, abs_path, rel_path, original_content, new_content, 'deletelines', metadata, change_id
} )
change_id = coder.change_tracker.track_change(
file_path=rel_path,
change_type='deletelines',
original_content=original_content,
new_content=new_content,
metadata=metadata,
change_id=change_id
)
except Exception as track_e:
coder.io.tool_error(f"Error tracking change for DeleteLines: {track_e}")
change_id = "TRACKING_FAILED"
coder.aider_edited_files.add(rel_path) coder.aider_edited_files.add(rel_path)
num_deleted = end_idx - start_idx + 1 num_deleted = end_idx - start_idx + 1
coder.io.tool_output(f"✅ Deleted {num_deleted} lines ({start_line_int}-{end_line_int}) in {file_path} (change_id: {change_id})") # Format and return result
return f"Successfully deleted {num_deleted} lines ({start_line_int}-{end_line_int}) (change_id: {change_id}). Diff snippet:\n{diff_snippet}" success_message = f"Deleted {num_deleted} lines ({start_line_int}-{end_line_int}) in {file_path}"
return format_tool_result(
coder, tool_name, success_message, change_id=final_change_id, diff_snippet=diff_snippet
)
except ToolError as e:
# Handle errors raised by utility functions (expected errors)
return handle_tool_error(coder, tool_name, e, add_traceback=False)
except Exception as e: except Exception as e:
coder.io.tool_error(f"Error in DeleteLines: {str(e)}\n{traceback.format_exc()}") # Handle unexpected errors
return f"Error: {str(e)}" return handle_tool_error(coder, tool_name, e)

View file

@ -116,16 +116,9 @@ def _execute_indent_lines(coder, file_path, start_pattern, end_pattern=None, lin
return format_tool_result( return format_tool_result(
coder, tool_name, success_message, change_id=final_change_id, diff_snippet=diff_snippet coder, tool_name, success_message, change_id=final_change_id, diff_snippet=diff_snippet
) )
except ToolError as e: except ToolError as e:
# Handle errors raised by utility functions (expected errors) # Handle errors raised by utility functions (expected errors)
return handle_tool_error(coder, tool_name, e, add_traceback=False) return handle_tool_error(coder, tool_name, e, add_traceback=False)
except Exception as e: except Exception as e:
# Handle unexpected errors # Handle unexpected errors
return handle_tool_error(coder, tool_name, e) return handle_tool_error(coder, tool_name, e)
coder.io.tool_output(f"{action} {num_lines} lines (from {occurrence_str}start pattern) by {levels} {level_text} in {file_path} (change_id: {change_id})")
return f"Successfully {action.lower()} {num_lines} lines by {levels} {level_text} (change_id: {change_id}). Diff snippet:\n{diff_snippet}"
except Exception as e:
coder.io.tool_error(f"Error in IndentLines: {str(e)}\n{traceback.format_exc()}")
return f"Error: {str(e)}"

View file

@ -1,7 +1,6 @@
import os import os
import traceback import traceback
from .tool_utils import generate_unified_diff_snippet from .tool_utils import ToolError, generate_unified_diff_snippet, handle_tool_error, format_tool_result, apply_change
from .tool_utils import generate_unified_diff_snippet
def _execute_replace_lines(coder, file_path, start_line, end_line, new_content, change_id=None, dry_run=False): def _execute_replace_lines(coder, file_path, start_line, end_line, new_content, change_id=None, dry_run=False):
""" """
@ -18,6 +17,7 @@ def _execute_replace_lines(coder, file_path, start_line, end_line, new_content,
Returns a result message. Returns a result message.
""" """
tool_name = "ReplaceLines"
try: try:
# Get absolute file path # Get absolute file path
abs_path = coder.abs_root_path(file_path) abs_path = coder.abs_root_path(file_path)
@ -25,38 +25,30 @@ def _execute_replace_lines(coder, file_path, start_line, end_line, new_content,
# Check if file exists # Check if file exists
if not os.path.isfile(abs_path): if not os.path.isfile(abs_path):
coder.io.tool_error(f"File '{file_path}' not found") raise ToolError(f"File '{file_path}' not found")
return f"Error: File not found"
# Check if file is in editable context # Check if file is in editable context
if abs_path not in coder.abs_fnames: if abs_path not in coder.abs_fnames:
if abs_path in coder.abs_read_only_fnames: if abs_path in coder.abs_read_only_fnames:
coder.io.tool_error(f"File '{file_path}' is read-only. Use MakeEditable first.") raise ToolError(f"File '{file_path}' is read-only. Use MakeEditable first.")
return f"Error: File is read-only. Use MakeEditable first."
else: else:
coder.io.tool_error(f"File '{file_path}' not in context") raise ToolError(f"File '{file_path}' not in context")
return f"Error: File not in context"
# Reread file content immediately before modification # Reread file content immediately before modification
file_content = coder.io.read_text(abs_path) file_content = coder.io.read_text(abs_path)
if file_content is None: if file_content is None:
coder.io.tool_error(f"Could not read file '{file_path}' before ReplaceLines operation.") raise ToolError(f"Could not read file '{file_path}'")
return f"Error: Could not read file '{file_path}'"
# Convert line numbers to integers if needed # Convert line numbers to integers if needed
if not isinstance(start_line, int): try:
try: start_line = int(start_line)
start_line = int(start_line) except ValueError:
except ValueError: raise ToolError(f"Invalid start_line value: '{start_line}'. Must be an integer.")
coder.io.tool_error(f"Invalid start_line value: '{start_line}'. Must be an integer.")
return f"Error: Invalid start_line value '{start_line}'"
if not isinstance(end_line, int): try:
try: end_line = int(end_line)
end_line = int(end_line) except ValueError:
except ValueError: raise ToolError(f"Invalid end_line value: '{end_line}'. Must be an integer.")
coder.io.tool_error(f"Invalid end_line value: '{end_line}'. Must be an integer.")
return f"Error: Invalid end_line value '{end_line}'"
# Split into lines # Split into lines
lines = file_content.splitlines() lines = file_content.splitlines()
@ -64,14 +56,13 @@ def _execute_replace_lines(coder, file_path, start_line, end_line, new_content,
# Convert 1-based line numbers to 0-based indices # Convert 1-based line numbers to 0-based indices
start_idx = start_line - 1 start_idx = start_line - 1
end_idx = end_line - 1 end_idx = end_line - 1
# Validate line numbers # Validate line numbers
if start_idx < 0 or start_idx >= len(lines): if start_idx < 0 or start_idx >= len(lines):
coder.io.tool_error(f"Start line {start_line} is out of range for file '{file_path}' (has {len(lines)} lines).") raise ToolError(f"Start line {start_line} is out of range for file '{file_path}' (has {len(lines)} lines).")
return f"Error: Start line {start_line} out of range"
if end_idx < start_idx or end_idx >= len(lines): if end_idx < start_idx or end_idx >= len(lines):
coder.io.tool_error(f"End line {end_line} is out of range for file '{file_path}' (must be >= start line {start_line} and <= {len(lines)}).") raise ToolError(f"End line {end_line} is out of range for file '{file_path}' (must be >= start line {start_line} and <= {len(lines)}).")
return f"Error: End line {end_line} out of range"
# Store original content for change tracking # Store original content for change tracking
original_content = file_content original_content = file_content
@ -87,6 +78,8 @@ def _execute_replace_lines(coder, file_path, start_line, end_line, new_content,
if original_content == new_content_full: if original_content == new_content_full:
coder.io.tool_warning("No changes made: new content is identical to original") coder.io.tool_warning("No changes made: new content is identical to original")
return f"Warning: No changes made (new content identical to original)" return f"Warning: No changes made (new content identical to original)"
# Generate diff snippet
diff_snippet = generate_unified_diff_snippet(original_content, new_content_full, rel_path) diff_snippet = generate_unified_diff_snippet(original_content, new_content_full, rel_path)
# Create a readable diff for the lines replacement # Create a readable diff for the lines replacement
@ -102,40 +95,34 @@ def _execute_replace_lines(coder, file_path, start_line, end_line, new_content,
# Handle dry run # Handle dry run
if dry_run: if dry_run:
coder.io.tool_output(f"Dry run: Would replace lines {start_line}-{end_line} in {file_path}") dry_run_message = f"Dry run: Would replace lines {start_line}-{end_line} in {file_path}"
return f"Dry run: Would replace lines {start_line}-{end_line}. Diff snippet:\n{diff_snippet}" return format_tool_result(coder, tool_name, "", dry_run=True, dry_run_message=dry_run_message, diff_snippet=diff_snippet)
# --- Apply Change (Not dry run) --- # --- Apply Change (Not dry run) ---
coder.io.write_text(abs_path, new_content_full) metadata = {
'start_line': start_line,
# Track the change 'end_line': end_line,
try: 'replaced_lines': replaced_lines,
metadata = { 'new_lines': new_lines
'start_line': start_line, }
'end_line': end_line,
'replaced_lines': replaced_lines, final_change_id = apply_change(
'new_lines': new_lines coder, abs_path, rel_path, original_content, new_content_full, 'replacelines', metadata, change_id
} )
change_id = coder.change_tracker.track_change(
file_path=rel_path,
change_type='replacelines',
original_content=original_content,
new_content=new_content_full,
metadata=metadata,
change_id=change_id
)
except Exception as track_e:
coder.io.tool_error(f"Error tracking change for ReplaceLines: {track_e}")
change_id = "TRACKING_FAILED"
coder.aider_edited_files.add(rel_path) coder.aider_edited_files.add(rel_path)
replaced_count = end_line - start_line + 1 replaced_count = end_line - start_line + 1
new_count = len(new_lines) new_count = len(new_lines)
# Improve feedback # Format and return result
coder.io.tool_output(f"✅ Replaced lines {start_line}-{end_line} ({replaced_count} lines) with {new_count} new lines in {file_path} (change_id: {change_id})") success_message = f"Replaced lines {start_line}-{end_line} ({replaced_count} lines) with {new_count} new lines in {file_path}"
return f"Successfully replaced lines {start_line}-{end_line} with {new_count} new lines (change_id: {change_id}). Diff:\n{diff}" return format_tool_result(
coder, tool_name, success_message, change_id=final_change_id, diff_snippet=diff_snippet
)
except ToolError as e:
# Handle errors raised by utility functions (expected errors)
return handle_tool_error(coder, tool_name, e, add_traceback=False)
except Exception as e: except Exception as e:
coder.io.tool_error(f"Error in ReplaceLines: {str(e)}\n{traceback.format_exc()}") # Handle unexpected errors
return f"Error: {str(e)}" return handle_tool_error(coder, tool_name, e)