From 619e626d707418ddaf3c307825804fa6e0311391 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 3 Jan 2024 18:22:38 -0600 Subject: [PATCH] Handle unrecognized errors in guest create `globus gcs collection create guest` previously assumed that the shape of an error from GCS met its expectations that `detail` would be dict data. However, GCS docs do not guarantee this -- `detail` can be of any type. Therefore, the inspection should explicitly check for this and fail if the field is present with an unexpected type. Failure to do so resulted in a `get()` call on a non-dict object. Resolves #928 --- ...2435_sirosen_handle_non_dict_gcs_detail.md | 4 ++ .../commands/collection/create/guest.py | 6 ++- .../test_collection_create_guest.py | 41 +++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 changelog.d/20240103_182435_sirosen_handle_non_dict_gcs_detail.md diff --git a/changelog.d/20240103_182435_sirosen_handle_non_dict_gcs_detail.md b/changelog.d/20240103_182435_sirosen_handle_non_dict_gcs_detail.md new file mode 100644 index 000000000..75b9aba43 --- /dev/null +++ b/changelog.d/20240103_182435_sirosen_handle_non_dict_gcs_detail.md @@ -0,0 +1,4 @@ +### Bugfixes + +* Fix the error handling when `globus gcs collection create guest` encounters a + non-session error. diff --git a/src/globus_cli/commands/collection/create/guest.py b/src/globus_cli/commands/collection/create/guest.py index d270c8411..e89b864f4 100644 --- a/src/globus_cli/commands/collection/create/guest.py +++ b/src/globus_cli/commands/collection/create/guest.py @@ -197,7 +197,11 @@ def _is_session_timeout_error(e: globus_sdk.GCSAPIError) -> bool: Detect session timeouts related to HA collections. This is a hacky workaround until we have better GARE support across the CLI. """ - detail_type = getattr(e, "detail", {}).get("DATA_TYPE") + detail = getattr(e, "detail", {}) + if not isinstance(detail, dict): + return False + + detail_type = detail.get("DATA_TYPE") return ( e.http_status == 403 and isinstance(detail_type, str) diff --git a/tests/functional/collection/test_collection_create_guest.py b/tests/functional/collection/test_collection_create_guest.py index d2957554a..e57392f84 100644 --- a/tests/functional/collection/test_collection_create_guest.py +++ b/tests/functional/collection/test_collection_create_guest.py @@ -228,3 +228,44 @@ def test_guest_collection_create__when_session_times_out_against_ha_mapped_colle assert "Session timeout detected; Re-authentication required." in result.stderr assert f"globus login --gcs {endpoint_id} --force" in result.stderr + + +def test_guest_collection_create__when_gcs_emits_unrecognized_error( + run_line, + mock_user_data, + add_gcs_login, +): + meta = load_response_set("cli.collection_operations").metadata + mapped_collection_id = meta["mapped_collection_id"] + display_name = meta["guest_display_name"] + gcs_hostname = meta["gcs_hostname"] + endpoint_id = meta["endpoint_id"] + add_gcs_login(endpoint_id) + + create_guest_collection_route = f"https://{gcs_hostname}/api/collections" + responses.replace( + "POST", + create_guest_collection_route, + status=403, + json={ + "DATA_TYPE": "result#1.0.0", + "code": "permission_denied", + "detail": "oh noez!", + "has_next_page": False, + "http_response_code": 403, + "message": "Oh noez! You must authenticate much better!", + }, + ) + + get_endpoint_route = ( + f"{get_service_url('transfer')}v0.10/endpoint/{mapped_collection_id}" + ) + get_endpoint_resp = requests.get(get_endpoint_route).json() + get_endpoint_resp["high_assurance"] = True + responses.replace("GET", get_endpoint_route, json=get_endpoint_resp) + + params = f"{mapped_collection_id} /home/ '{display_name}'" + result = run_line(f"globus collection create guest {params}", assert_exit_code=1) + + assert "Session timeout detected" not in result.stderr + assert "Oh noez! You must authenticate much better!" in result.stderr