From e2f3b7d86b218852d5937bdddd96953ffffe34d4 Mon Sep 17 00:00:00 2001 From: Corey Goldberg <1113081+cgoldberg@users.noreply.github.com> Date: Tue, 10 Jun 2025 16:22:21 -0400 Subject: [PATCH 1/4] [py] Fix error handler for non-json responses --- py/selenium/webdriver/remote/errorhandler.py | 35 ++++++++++--------- .../webdriver/remote/error_handler_tests.py | 28 +++++++++++---- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/py/selenium/webdriver/remote/errorhandler.py b/py/selenium/webdriver/remote/errorhandler.py index fbfc7696d3986..53917a78a0d44 100644 --- a/py/selenium/webdriver/remote/errorhandler.py +++ b/py/selenium/webdriver/remote/errorhandler.py @@ -161,23 +161,24 @@ def check_response(self, response: dict[str, Any]) -> None: if isinstance(status, int): value_json = response.get("value", None) if value_json and isinstance(value_json, str): - import json - - try: - value = json.loads(value_json) - if len(value) == 1: - value = value["value"] - status = value.get("error", None) - if not status: - status = value.get("status", ErrorCode.UNKNOWN_ERROR) - message = value.get("value") or value.get("message") - if not isinstance(message, str): - value = message - message = message.get("message") - else: - message = value.get("message", None) - except ValueError: - pass + if not value_json.isdigit(): + import json + + try: + value = json.loads(value_json) + if len(value) == 1: + value = value["value"] + status = value.get("error", None) + if not status: + status = value.get("status", ErrorCode.UNKNOWN_ERROR) + message = value.get("value") or value.get("message") + if not isinstance(message, str): + value = message + message = message.get("message") + else: + message = value.get("message", None) + except ValueError: + pass exception_class: type[WebDriverException] e = ErrorCode() diff --git a/py/test/unit/selenium/webdriver/remote/error_handler_tests.py b/py/test/unit/selenium/webdriver/remote/error_handler_tests.py index 3d5afdad2bc7c..f8e35e082e440 100644 --- a/py/test/unit/selenium/webdriver/remote/error_handler_tests.py +++ b/py/test/unit/selenium/webdriver/remote/error_handler_tests.py @@ -15,6 +15,8 @@ # specific language governing permissions and limitations # under the License. +import json + import pytest from selenium.common import exceptions @@ -249,20 +251,15 @@ def test_raises_exception_for_detached_shadow_root(handler, code): @pytest.mark.parametrize("key", ["stackTrace", "stacktrace"]) def test_relays_exception_stacktrace(handler, key): - import json - stacktrace = {"lineNumber": 100, "fileName": "egg", "methodName": "ham", "className": "Spam"} value = {key: [stacktrace], "message": "very bad", "error": ErrorCode.UNKNOWN_METHOD[0]} response = {"status": 400, "value": json.dumps({"value": value})} with pytest.raises(exceptions.UnknownMethodException) as e: handler.check_response(response) - assert "Spam.ham" in e.value.stacktrace[0] def test_handle_errors_better(handler): - import json - response = { "status": 500, "value": json.dumps( @@ -281,5 +278,24 @@ def test_handle_errors_better(handler): } with pytest.raises(exceptions.WebDriverException) as e: handler.check_response(response) - assert "Could not start a new session." in e.value.msg + + +def test_handle_numeric_value_error(handler): + response = { + "status": 999, + "value": "0", + } + with pytest.raises(exceptions.WebDriverException) as e: + handler.check_response(response) + assert e.value.msg == "0" + + +def test_handle_non_json_error(handler): + response = { + "status": 407, + "value": "Proxy Authentication Required", + } + with pytest.raises(exceptions.WebDriverException) as e: + handler.check_response(response) + assert e.value.msg == "Proxy Authentication Required" From a77fa6650dd0410d6cd676023858c5e27ca92ce4 Mon Sep 17 00:00:00 2001 From: Corey Goldberg <1113081+cgoldberg@users.noreply.github.com> Date: Tue, 10 Jun 2025 16:37:42 -0400 Subject: [PATCH 2/4] [py] Fix formatting --- py/selenium/webdriver/remote/errorhandler.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/py/selenium/webdriver/remote/errorhandler.py b/py/selenium/webdriver/remote/errorhandler.py index 53917a78a0d44..ce5d900f461f4 100644 --- a/py/selenium/webdriver/remote/errorhandler.py +++ b/py/selenium/webdriver/remote/errorhandler.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +import json from typing import Any from selenium.common.exceptions import ( @@ -162,8 +163,6 @@ def check_response(self, response: dict[str, Any]) -> None: value_json = response.get("value", None) if value_json and isinstance(value_json, str): if not value_json.isdigit(): - import json - try: value = json.loads(value_json) if len(value) == 1: From ab9591a89416b3114586ec8b067c059608210700 Mon Sep 17 00:00:00 2001 From: Corey Goldberg <1113081+cgoldberg@users.noreply.github.com> Date: Tue, 10 Jun 2025 20:05:31 -0400 Subject: [PATCH 3/4] [py] Fix indentation --- py/test/unit/selenium/webdriver/remote/error_handler_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/test/unit/selenium/webdriver/remote/error_handler_tests.py b/py/test/unit/selenium/webdriver/remote/error_handler_tests.py index f8e35e082e440..3d7614e225546 100644 --- a/py/test/unit/selenium/webdriver/remote/error_handler_tests.py +++ b/py/test/unit/selenium/webdriver/remote/error_handler_tests.py @@ -298,4 +298,4 @@ def test_handle_non_json_error(handler): } with pytest.raises(exceptions.WebDriverException) as e: handler.check_response(response) - assert e.value.msg == "Proxy Authentication Required" + assert e.value.msg == "Proxy Authentication Required" From bbea0f273d1f49b8e679dd2333399e36bdd5bc72 Mon Sep 17 00:00:00 2001 From: Corey Goldberg <1113081+cgoldberg@users.noreply.github.com> Date: Tue, 10 Jun 2025 20:44:58 -0400 Subject: [PATCH 4/4] [py] Check dict instance --- py/selenium/webdriver/remote/errorhandler.py | 10 +++++----- .../webdriver/remote/error_handler_tests.py | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/py/selenium/webdriver/remote/errorhandler.py b/py/selenium/webdriver/remote/errorhandler.py index ce5d900f461f4..9a100e20543f5 100644 --- a/py/selenium/webdriver/remote/errorhandler.py +++ b/py/selenium/webdriver/remote/errorhandler.py @@ -162,9 +162,9 @@ def check_response(self, response: dict[str, Any]) -> None: if isinstance(status, int): value_json = response.get("value", None) if value_json and isinstance(value_json, str): - if not value_json.isdigit(): - try: - value = json.loads(value_json) + try: + value = json.loads(value_json) + if isinstance(value, dict): if len(value) == 1: value = value["value"] status = value.get("error", None) @@ -176,8 +176,8 @@ def check_response(self, response: dict[str, Any]) -> None: message = message.get("message") else: message = value.get("message", None) - except ValueError: - pass + except ValueError: + pass exception_class: type[WebDriverException] e = ErrorCode() diff --git a/py/test/unit/selenium/webdriver/remote/error_handler_tests.py b/py/test/unit/selenium/webdriver/remote/error_handler_tests.py index 3d7614e225546..3b07a97394ceb 100644 --- a/py/test/unit/selenium/webdriver/remote/error_handler_tests.py +++ b/py/test/unit/selenium/webdriver/remote/error_handler_tests.py @@ -259,7 +259,7 @@ def test_relays_exception_stacktrace(handler, key): assert "Spam.ham" in e.value.stacktrace[0] -def test_handle_errors_better(handler): +def test_handle_json_error_with_message(handler): response = { "status": 500, "value": json.dumps( @@ -281,21 +281,21 @@ def test_handle_errors_better(handler): assert "Could not start a new session." in e.value.msg -def test_handle_numeric_value_error(handler): +def test_handle_string_error(handler): response = { - "status": 999, - "value": "0", + "status": 407, + "value": "Proxy Authentication Required", } with pytest.raises(exceptions.WebDriverException) as e: handler.check_response(response) - assert e.value.msg == "0" + assert e.value.msg == "Proxy Authentication Required" -def test_handle_non_json_error(handler): +def test_handle_numeric_error(handler): response = { - "status": 407, - "value": "Proxy Authentication Required", + "status": 999, + "value": "0", } with pytest.raises(exceptions.WebDriverException) as e: handler.check_response(response) - assert e.value.msg == "Proxy Authentication Required" + assert e.value.msg == "0"