diff --git a/.changes/unreleased/optimization-20260507-101040.yaml b/.changes/unreleased/optimization-20260507-101040.yaml new file mode 100644 index 000000000..4aceb9b2f --- /dev/null +++ b/.changes/unreleased/optimization-20260507-101040.yaml @@ -0,0 +1,6 @@ +kind: optimization +body: Shows explicit API error message +time: 2026-05-07T10:10:40.73557+03:00 +custom: + Author: v-alexmoraru + AuthorLink: https://github.com/v-alexmoraru diff --git a/src/fabric_cli/core/fab_exceptions.py b/src/fabric_cli/core/fab_exceptions.py index b3d6cd5e4..1bd1d3cd0 100644 --- a/src/fabric_cli/core/fab_exceptions.py +++ b/src/fabric_cli/core/fab_exceptions.py @@ -8,13 +8,17 @@ # Default error constants - avoids circular imports DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation" DEFAULT_ERROR_CODE = "UnknownError" +NOT_SET = object() class FabricCLIError(Exception): - def __init__(self, message=None, status_code=None): - # Use default values if not provided + def __init__(self, message=None, status_code=NOT_SET): + # message: values like (None, "") fall back to the default. + # status_code: default is applied only when omitted entirely; + # an explicit None is preserved (e.g. fallback paths that have no code). message = message or DEFAULT_ERROR_MESSAGE - status_code = status_code or DEFAULT_ERROR_CODE + if status_code is NOT_SET: + status_code = DEFAULT_ERROR_CODE super().__init__(message) self.message = message.rstrip(".") @@ -78,12 +82,23 @@ def __init__(self, response_text): related_resource (dict): Details about the main related resource, if available. request_id (str): The ID of the request associated with the error. """ - response = self._parse_json_response(response_text) - - message = response.get("message") - error_code = response.get("errorCode") - self.more_details: list[dict] = response.get("moreDetails", []) - self.request_id = response.get("requestId") + try: + response = json.loads(response_text) + if not isinstance(response, dict): + raise ValueError("Unexpected JSON shape") + message = ( + response.get("message") + if response.get("message") is not None + else response_text + ) + error_code = response.get("errorCode") + self.more_details: list[dict] = response.get("moreDetails", []) + self.request_id = response.get("requestId") + except (json.JSONDecodeError, TypeError, ValueError): + message = response_text + error_code = None + self.more_details = [] + self.request_id = None super().__init__(message, error_code) @@ -105,7 +120,10 @@ def formatted_message(self, verbose=False): else f"{base_message}\n{detailed_message}" ) - return f"{final_message}\n∟ Request Id: {self.request_id}" + if self.request_id: + final_message += f"\n∟ Request Id: {self.request_id}" + + return final_message class OnelakeAPIError(FabricCLIError): diff --git a/tests/test_utils/test_fab_custom_exception.py b/tests/test_utils/test_fab_custom_exception.py index d321717b4..69f09e1e7 100644 --- a/tests/test_utils/test_fab_custom_exception.py +++ b/tests/test_utils/test_fab_custom_exception.py @@ -1,7 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from fabric_cli.core.fab_exceptions import FabricCLIError +import json + +from fabric_cli.core.fab_exceptions import FabricAPIError, FabricCLIError def test_custom_error_message(): @@ -17,3 +19,68 @@ def test_custom_error_message_without_period(): def test_custom_error_formatted_message_with_status_code(): error = FabricCLIError("An error occurred.", status_code=404) assert error.formatted_message() == "[404] An error occurred" + + +def test_fabric_api_error_valid_json_with_request_id(): + payload = json.dumps( + { + "errorCode": "ItemNotFound", + "message": "The requested item was not found.", + "requestId": "abc-123", + "moreDetails": [], + } + ) + error = FabricAPIError(payload) + + assert error.status_code == "ItemNotFound" + assert error.message == "The requested item was not found" + assert error.request_id == "abc-123" + assert error.more_details == [] + + +def test_fabric_api_error_valid_json_without_request_id(): + payload = json.dumps( + { + "errorCode": "Unauthorized", + "message": "Access denied.", + } + ) + error = FabricAPIError(payload) + + assert error.status_code == "Unauthorized" + assert error.request_id is None + # formatted_message should not append a request-id line + assert "Request Id" not in error.formatted_message(verbose=True) + + +def test_fabric_api_error_non_json_body_falls_back_to_raw_text(): + raw = "Internal Server Error" + error = FabricAPIError(raw) + + assert error.message == raw.rstrip(".") + assert error.status_code is None + assert error.request_id is None + assert error.more_details == [] + + +def test_fabric_api_error_non_dict_json_falls_back_to_raw_text(): + for raw in ('"just a string"', "[1, 2, 3]", "42", "true"): + error = FabricAPIError(raw) + assert error.message == raw.rstrip(".") + assert error.status_code is None + assert error.request_id is None + assert error.more_details == [] + + +def test_fabric_api_error_formatted_message_non_json_no_request_id_line(): + error = FabricAPIError("Gateway Timeout") + formatted = error.formatted_message(verbose=True) + assert "Request Id" not in formatted + assert "Gateway Timeout" in formatted + + +def test_fabric_api_error_none_input_falls_back_to_default_message(): + error = FabricAPIError(None) + assert error.status_code is None + assert error.request_id is None + assert error.more_details == []