NgTech-LTD/gns3-server
2
0
Fork 0
mirror of https://github.com/GNS3/gns3-server.git synced 2026-08-27 12:30:13 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: unify the device tool error contract

Browse Source 
The device tools reported failures in three shapes: topology-level
entries with only an error key, per-device entries with status 'error'
plus the reason under output (VPCS tool only), and raw exceptions
leaking out of template rendering. Every in-band error entry now
carries status 'failed' and an error message, and invalid Jinja2
templates are reported in-band instead of escaping the handler.
This commit is contained in:
YueGuobin 2026-08-26 00:39:38 +08:00
parent 2951af6eab
commit 6703e50487
No known key found for this signature in database
6 changed files with 125 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
gns3server/agent/gns3_copilot/tools_v2/config_tools_nornir.py
View File

@ -227,7 +227,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
)
except ValueError as e:
logger.error("Failed to prepare device hosts data: %s", e)
return [{"error": str(e)}]
return [{"status": "failed", "error": str(e)}]
# Check if any devices have errors (e.g., missing device_type tag)
error_devices = {
@ -254,7 +254,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
dynamic_nr = self._initialize_nornir(hosts_data)
except ValueError as e:
logger.error("Failed to initialize Nornir: %s", e)
return [{"error": str(e)}]
return [{"status": "failed", "error": str(e)}]
results = []
@ -278,7 +278,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
logger.error(
"Error executing configurations on all devices: %s", e
)
return [{"error": f"Execution error: {str(e)}"}]
return [{"status": "failed", "error": f"Execution error: {str(e)}"}]
logger.info(
"Multiple device configuration execution completed. Results: %s",
@ -368,7 +368,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
"Invalid JSON string received as tool input: %s", e
)
return (
[{"error": f"Invalid JSON string input from model: {e}"}],
[{"status": "failed", "error": f"Invalid JSON string input from model: {e}"}],
None,
)
else:
@ -389,7 +389,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
if not project_id:
error_msg = "Missing required 'project_id' field in input"
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
if not self._validate_project_id(project_id):
error_msg = (
@ -397,13 +397,13 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
"Expected UUID format."
)
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
# Validate device_configs
if not isinstance(device_configs, list):
error_msg = "'device_configs' must be an array"
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
if not device_configs:
logger.warning("Device configs list is empty.")
@ -426,7 +426,7 @@ class ExecuteMultipleDeviceConfigCommands(BaseTool):
f"{type(parsed_input).__name__}"
)
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
def _validate_project_id(self, project_id: str) -> bool:
"""

16
gns3server/agent/gns3_copilot/tools_v2/display_tools_nornir.py
View File

@ -223,7 +223,7 @@ class ExecuteMultipleDeviceCommands(BaseTool):
)
except ValueError as e:
logger.error("Failed to prepare device hosts data: %s", e)
return [{"error": str(e)}]
return [{"status": "failed", "error": str(e)}]
# Check if any devices have errors (e.g., missing device_type tag)
error_devices = {
@ -250,7 +250,7 @@ class ExecuteMultipleDeviceCommands(BaseTool):
dynamic_nr = self._initialize_nornir(hosts_data)
except ValueError as e:
logger.error("Failed to initialize Nornir: %s", e)
return [{"error": str(e)}]
return [{"status": "failed", "error": str(e)}]
results = []
@ -272,7 +272,7 @@ class ExecuteMultipleDeviceCommands(BaseTool):
except Exception as e:
# Overall execution failed
logger.error("Error executing display on all devices: %s", e)
return [{"error": f"Execution error: {str(e)}"}]
return [{"status": "failed", "error": f"Execution error: {str(e)}"}]
logger.debug(
"Multiple device display execution completed. Results: %s",
@ -370,7 +370,7 @@ class ExecuteMultipleDeviceCommands(BaseTool):
"Invalid JSON string received as tool input: %s", e
)
return (
[{"error": f"Invalid JSON string input from model: {e}"}],
[{"status": "failed", "error": f"Invalid JSON string input from model: {e}"}],
None,
)
else:
@ -391,18 +391,18 @@ class ExecuteMultipleDeviceCommands(BaseTool):
if not project_id:
error_msg = "Missing required 'project_id' field in input"
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
if not self._validate_project_id(project_id):
error_msg = f"Invalid project_id: {project_id}. Expected UUID."
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
# Validate device_configs
if not isinstance(device_configs, list):
error_msg = "'device_configs' must be an array"
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
if not device_configs:
logger.warning("Device configs list is empty.")
@ -423,7 +423,7 @@ class ExecuteMultipleDeviceCommands(BaseTool):
f"or legacy JSON array, got {type(parsed_input).__name__}"
)
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
def _validate_project_id(self, project_id: str) -> bool:
"""

28
gns3server/agent/gns3_copilot/tools_v2/vpcs_tools_netmiko.py
View File

@ -196,7 +196,7 @@ class VPCSCommands(BaseTool):
)
except ValueError as e:
logger.error("Failed to prepare device hosts data: %s", e)
return [{"error": str(e)}]
return [{"status": "failed", "error": str(e)}]
# Check if any devices have errors (e.g., missing device)
error_devices = {
@ -223,7 +223,7 @@ class VPCSCommands(BaseTool):
dynamic_nr = self._initialize_nornir(hosts_data)
except ValueError as e:
logger.error("Failed to initialize Nornir: %s", e)
return [{"error": str(e)}]
return [{"status": "failed", "error": str(e)}]
results = []
@ -244,7 +244,7 @@ class VPCSCommands(BaseTool):
except Exception as e:
# Overall execution failed
logger.error("Error executing commands on all VPCS devices: %s", e)
return [{"error": f"Execution error: {str(e)}"}]
return [{"status": "failed", "error": f"Execution error: {str(e)}"}]
logger.debug(
"VPCS command execution completed. Results: %s",
@ -338,7 +338,7 @@ class VPCSCommands(BaseTool):
"Invalid JSON string received as tool input: %s", e
)
return (
[{"error": f"Invalid JSON string input from model: {e}"}],
[{"status": "failed", "error": f"Invalid JSON string input from model: {e}"}],
None,
)
else:
@ -357,18 +357,18 @@ class VPCSCommands(BaseTool):
if not project_id:
error_msg = "Missing required 'project_id' field in input"
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
if not self._validate_project_id(project_id):
error_msg = f"Invalid project_id: {project_id}. Expected UUID."
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
# Validate device_configs
if not isinstance(device_configs, list):
error_msg = "'device_configs' must be an array"
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
if not device_configs:
logger.warning("Device configs list is empty.")
@ -382,7 +382,7 @@ class VPCSCommands(BaseTool):
f"got {type(parsed_input).__name__}"
)
logger.error(error_msg)
return ([{"error": error_msg}], None)
return ([{"status": "failed", "error": error_msg}], None)
def _validate_project_id(self, project_id: str) -> bool:
"""
@ -578,8 +578,8 @@ class VPCSCommands(BaseTool):
if device_name in hosts_data and "error" in hosts_data[device_name]:
results.append({
"device_name": device_name,
"status": "error",
"output": hosts_data[device_name]["error"],
"status": "failed",
"error": hosts_data[device_name]["error"],
"commands": device_config["commands"],
})
continue
@ -593,8 +593,8 @@ class VPCSCommands(BaseTool):
error_msg = str(host_result.result) if host_result.result else "Unknown error"
results.append({
"device_name": device_name,
"status": "error",
"output": error_msg,
"status": "failed",
"error": error_msg,
"commands": device_config["commands"],
})
else:
@ -609,8 +609,8 @@ class VPCSCommands(BaseTool):
# Device not in task result (shouldn't happen)
results.append({
"device_name": device_name,
"status": "error",
"output": f"Device '{device_name}' not in task results",
"status": "failed",
"error": f"Device '{device_name}' not in task results",
"commands": device_config["commands"],
})

10
gns3server/agent/mcp/__init__.py
View File

@ -1471,6 +1471,10 @@ async def device_config_send(
Devices must be started first (use node_start or node_start_all).
Device type is auto-detected from the 'device_type:<type>' tag on each node.
Common device types: cisco_ios_telnet, cisco_xr_telnet, huawei_telnet, gns3_huawei_telnet_ce
Error contract: every failure is reported in-band as an entry with
status "failed" and an "error" message (per-device entries also carry
device_name and commands).
"""
params = {"project_id": project_id, "device_configs": device_configs}
if template is not None:
@ -1501,6 +1505,10 @@ async def device_show_run(
(e.g. device_type:cisco_ios_telnet, device_type:gns3_huawei_telnet_ce).
Nodes without this tag will fail with "device_type tag not found".
Docker/Linux nodes are not supported (use node_console instead).
Error contract: every failure is reported in-band as an entry with
status "failed" and an "error" message (per-device entries also carry
device_name and commands).
"""
params = {"project_id": project_id, "device_configs": device_configs}
if template is not None:
@ -1519,6 +1527,8 @@ async def vpcs_config_set(
Only VPCS nodes are accepted: any other node type in device_configs fails
with a per-device error instead of typing VPCS syntax into its CLI.
Every failure is reported in-band as an entry with status "failed"
and an "error" message.
VPCS-specific configuration commands:
- ip <address>/<mask> <gateway> Set IP and gateway

16
gns3server/agent/mcp/device_config.py
View File

@ -51,7 +51,13 @@ def _render_template(template: str, device_configs: list[dict], commands_field:
Args:
commands_field: field name for the rendered commands, e.g. "config_commands", "commands"
"""
jinja = JinjaTemplate(template)
try:
jinja = JinjaTemplate(template)
except JinjaError as e:
# a syntactically invalid template must not escape as a raw exception
error_msg = f"Template rendering failed: {e}"
log.error(error_msg)
return [{"status": "failed", "error": error_msg}]
merged: dict[str, dict] = {}
for dev in device_configs:
name = dev.get("device_name")
@ -69,7 +75,7 @@ def _render_template(template: str, device_configs: list[dict], commands_field:
except JinjaError as e:
error_msg = f"Template rendering failed for '{name}': {e}"
log.error(error_msg)
return [{"error": error_msg}]
return [{"status": "failed", "error": error_msg}]
return list(merged.values())
@ -81,7 +87,7 @@ def device_config_send_handler(params: dict[str, Any], gns3_ctx: dict[str, Any])
device_configs = params.get("device_configs")
template = params.get("template")
if not project_id or not device_configs:
return [{"error": "project_id and device_configs are required"}]
return [{"status": "failed", "error": "project_id and device_configs are required"}]
if template:
device_configs = _render_template(template, device_configs, commands_field="config_commands")
@ -108,7 +114,7 @@ def device_show_run_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) ->
device_configs = params.get("device_configs")
template = params.get("template")
if not project_id or not device_configs:
return [{"error": "project_id and device_configs (list of {device_name, commands}) are required"}]
return [{"status": "failed", "error": "project_id and device_configs (list of {device_name, commands}) are required"}]
if template:
device_configs = _render_template(template, device_configs, commands_field="commands")
@ -134,7 +140,7 @@ def vpcs_config_set_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) ->
project_id = params.get("project_id")
device_configs = params.get("device_configs")
if not project_id or not device_configs:
return [{"error": "project_id and device_configs are required"}]
return [{"status": "failed", "error": "project_id and device_configs are required"}]
from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands

74
tests/agent/mcp/test_device_tools.py
View File

@ -63,3 +63,77 @@ class TestVPCSNodeTypeGuard:
}))
assert result[0]["status"] == "success"
assert result[0]["output"] == "OK"
def test_execution_failure_reports_failed_with_error(self):
from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands
tool = VPCSCommands()
nornir = MagicMock()
host_result = MagicMock(failed=True)
host_result.result = "Command failed (ReadTimeout)"
nornir.run.return_value = {"PC1": host_result}
with patch(f"{VPCS_MOD}.get_device_ports_from_topology",
return_value=_topology_ports("vpcs")), \
patch.object(VPCSCommands, "_initialize_nornir", return_value=nornir):
result = tool._run(json.dumps({
"project_id": "0c0fde25-6ead-4413-a283-ea8fd2324291",
"device_configs": [{"device_name": "PC1", "commands": ["ip 10.0.0.1/24"]}],
}))
assert result[0]["status"] == "failed"
assert result[0]["error"] == "Command failed (ReadTimeout)"
assert "output" not in result[0]
class TestDeviceToolErrorContract:
"""
Every in-band error entry carries status "failed" plus an "error"
message, whether it is topology-level (no device) or per-device.
"""
def test_topology_level_error_has_status(self):
from gns3server.agent.gns3_copilot.tools_v2.vpcs_tools_netmiko import VPCSCommands
with patch(f"{VPCS_MOD}.get_device_ports_from_topology",
side_effect=ValueError("topology unreachable")):
result = VPCSCommands()._run(json.dumps({
"project_id": "0c0fde25-6ead-4413-a283-ea8fd2324291",
"device_configs": [{"device_name": "PC1", "commands": ["ip 10.0.0.1/24"]}],
}))
assert result == [{"status": "failed", "error": "topology unreachable"}]
def test_config_tool_topology_level_error_has_status(self):
from gns3server.agent.gns3_copilot.tools_v2.config_tools_nornir import (
ExecuteMultipleDeviceConfigCommands,
)
with patch("gns3server.agent.gns3_copilot.tools_v2.config_tools_nornir"
".get_device_ports_from_topology",
side_effect=ValueError("no valid devices")):
result = ExecuteMultipleDeviceConfigCommands()._run(json.dumps({
"project_id": "0c0fde25-6ead-4413-a283-ea8fd2324291",
"device_configs": [{"device_name": "R1", "config_commands": ["int lo0"]}],
}))
assert result == [{"status": "failed", "error": "no valid devices"}]
def test_mcp_handler_param_error_has_status(self, ctx=None):
from gns3server.agent.mcp.device_config import (
device_config_send_handler,
device_show_run_handler,
vpcs_config_set_handler,
)
for handler in (device_config_send_handler, device_show_run_handler, vpcs_config_set_handler):
result = handler({}, {"server_url": "http://x", "jwt_token": "t"})
assert result == [{
"status": "failed",
"error": result[0]["error"], # message text may differ per handler
}]
assert "required" in result[0]["error"]
def test_template_render_error_has_status(self):
from gns3server.agent.mcp.device_config import _render_template
result = _render_template("{{ unclosed", [{"device_name": "R1", "vars": {"n": 1}}])
assert len(result) == 1
assert result[0]["status"] == "failed"
assert "Template rendering failed" in result[0]["error"]