Skip to content

Fix set-alias endpoint: return HTTP 200 with normalized error key on failure#1600

Merged
jokob-sk merged 2 commits intonext_releasefrom
copilot/fix-api-error-response
Apr 8, 2026
Merged

Fix set-alias endpoint: return HTTP 200 with normalized error key on failure#1600
jokob-sk merged 2 commits intonext_releasefrom
copilot/fix-api-error-response

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

📌 Description

POST /device/<mac>/set-alias was returning HTTP 404 when updateDeviceColumn reported failure. The test test_set_device_alias_not_found correctly expects HTTP 200 with the error in the JSON body — consistent with every other endpoint in this API that uses the {"success": false, "error": "..."} envelope pattern rather than HTTP status codes to communicate domain errors.

Change in api_device_set_alias:

# Before
if not result.get("success"):
    return jsonify(result), 404  # wrong status; doesn't normalize error key

# After
if not result.get("success"):
    err = result.get("error") or result.get("message") or f"Failed to update alias for device {mac}"
    return jsonify({"success": False, "error": err})  # HTTP 200, normalized error key
  • Always returns HTTP 200; domain success/failure lives in success + error fields
  • Normalizes the error key: checks error first, falls back to message, then a contextual default
  • Success path is untouched

🔍 Related Issues


📋 Type of Change

  • 🐛 Bug fix

📷 Screenshots or Logs (if applicable)

Before: FAILED test/api_endpoints/test_mcp_tools_endpoints.py::test_set_device_alias_not_foundAssertionError: assert 404 == 200

After: Test passes; CodeQL: 0 alerts.


🧪 Testing Steps

  • test_set_device_alias_not_found — mocks updateDeviceColumn returning {"success": False, "error": "Device not found"}, asserts status 200 and data["error"] contains "Device not found"
  • test_set_device_alias — success path unchanged, still passes

✅ Checklist

  • I have read the Contribution Guidelines
  • I have tested my changes locally
  • I have updated relevant documentation (if applicable)
  • I have verified my changes do not break existing behavior
  • I am willing to respond to requested changes and feedback

🙋 Additional Notes

The error/message fallback is intentional: updateDeviceColumn may return either key depending on the failure path; this handler normalises both into error for a consistent API surface.

Original prompt

Fix failing GitHub Actions job 70519425927 in netalertx/NetAlertX on branch next_release.

Context:

  • Workflow: .github/workflows/run-all-tests.yml
  • Failing test: test/api_endpoints/test_mcp_tools_endpoints.py::test_set_device_alias_not_found
  • Test mocks models.device_instance.DeviceInstance.updateDeviceColumn to return { "success": False, "error": "Device not found" } and expects JSON response contains success == False and data["error"] includes "Device not found".
  • Actual API endpoint /device/<mac>/set-alias likely returns error under a different key (e.g., message) or otherwise doesn’t pass through the error key.

Task:

  1. Locate the API route handler for POST /device/<mac>/set-alias (likely in server/api endpoints code).
  2. Ensure that when DeviceInstance.updateDeviceColumn(...) returns success: False, the HTTP JSON response includes an error key containing the message (use result.get("error") first, and fallback to result.get("message") if needed).
  3. Keep the endpoint success behavior unchanged.
  4. Run/update tests as needed so test_set_device_alias_not_found passes.
  5. Prefer fixing production code rather than weakening the test.

Reference failing job log URL:

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The job is failing because one test expectation doesn’t match the API’s actual behavior for the “device not found” case on the set-alias endpoint.

What failed

From the job log, the only failing test is:

  • FAILED test/api_endpoints/test_mcp_tools_endpoints.py::test_set_device_alias_not_found (ref e3a4c62d5f93591a533b011bc861eee1067ccfa7)

In that test, DeviceInstance.updateDeviceColumn is mocked to return:

{"success": False, "error": "Device not found"}

but the endpoint likely returns a different error field name (commonly message) or a different status code for “not found”.

Fix (recommended): make the endpoint return error consistently

Since multiple tests already assert data["error"] for failures (including this one), the cleanest solution is: ensure the set-alias endpoint always returns errors under the error key when success is False.

Patch idea (API route)

In the handler for:
POST /device/<mac>/set-alias

change the response mapping to something like:

result = DeviceInstance().updateDeviceColumn(mac, "devName", alias)

if not result.get("success", False):
    # normalize error key
    err = result.get("error") or result.get("message") or "Unknown error"
    return jsonify({"success": False, "error": err}), 200

return jsonify({"success": True, "message": result.get("message", "Device alias updated")}), 200

This ensures data["error"] exists exactly as the test expects.

Alternative fix: adjust the test to accept either error or message

If you prefer not to enforce error as the canonical field, update the failing assertion in test/api_endpoints/test_mcp_tools_endpoints.py (ref e3a4c62...), test test_set_device_alias_not_found:

Replace:

assert "Device not found" in data["error"]

with:

err = data.get("error") or data.get("message") or ""
assert "Device not found" in err

Notes (non-blocking but visible in logs)

You also have a container startup warning in the logs:

🚨 ALERT: Python execution capabilities (NET_RAW/NET_ADMIN) are missing... Restart with: --cap-add=NET_RAW --cap-add=NET_ADMIN

This did not cause the test failure (pytest ran and only 1 test failed), but it’s worth addressing in your Docker test harness later to avoid flaky behavior for network-related features.

Quick summary

  • Root cause: API error payload shape mismatch for set-alias “not found”.
  • Best fix: normalize to {"success": False, "error": ...} in the endpoint when update fails.
  • Fastest workaround: make the test accept error or message.

User: Fix in next_release branch

Assistant: I’ve queued the fix against the next_release branch and am now waiting for you to accept/confirm the automated change so I can proceed and provide the patch/PR details.

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Fix error response for set alias endpoint Fix set-alias endpoint: return HTTP 200 with normalized error key on failure Apr 8, 2026
Copilot AI requested a review from jokob-sk April 8, 2026 23:29
@jokob-sk jokob-sk marked this pull request as ready for review April 8, 2026 23:58
@jokob-sk jokob-sk merged commit 16e2249 into next_release Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants