refactor: use selenium code as possible

appium/webdriver/errorhandler.py

@@ -13,79 +13,17 @@
 # limitations under the License.
 
 import json
-from collections.abc import Sequence
 from typing import Any
 
 import selenium.common.exceptions as sel_exceptions
 from selenium.webdriver.remote import errorhandler
 
 import appium.common.exceptions as appium_exceptions
 
-ERROR_TO_EXC_MAPPING: dict[str, type[sel_exceptions.WebDriverException]] = {
-    'element click intercepted': sel_exceptions.ElementClickInterceptedException,
-    'element not interactable': sel_exceptions.ElementNotInteractableException,
-    'insecure certificate': sel_exceptions.InsecureCertificateException,
-    'invalid argument': sel_exceptions.InvalidArgumentException,
-    'invalid cookie domain': sel_exceptions.InvalidCookieDomainException,
-    'invalid element state': sel_exceptions.InvalidElementStateException,
-    'invalid selector': sel_exceptions.InvalidSelectorException,
-    'invalid session id': sel_exceptions.InvalidSessionIdException,
-    'javascript error': sel_exceptions.JavascriptException,
-    'move target out of bounds': sel_exceptions.MoveTargetOutOfBoundsException,
-    'no such alert': sel_exceptions.NoAlertPresentException,
-    'no such cookie': sel_exceptions.NoSuchCookieException,
-    'no such element': sel_exceptions.NoSuchElementException,
-    'no such frame': sel_exceptions.NoSuchFrameException,
-    'no such window': sel_exceptions.NoSuchWindowException,
-    'no such shadow root': sel_exceptions.NoSuchShadowRootException,
-    'script timeout': sel_exceptions.TimeoutException,
-    'session not created': sel_exceptions.SessionNotCreatedException,
-    'stale element reference': sel_exceptions.StaleElementReferenceException,
-    'detached shadow root': sel_exceptions.NoSuchShadowRootException,
-    'timeout': sel_exceptions.TimeoutException,
-    'unable to set cookie': sel_exceptions.UnableToSetCookieException,
-    'unable to capture screen': sel_exceptions.ScreenshotException,
-    'unexpected alert open': sel_exceptions.UnexpectedAlertPresentException,
-    'unknown command': sel_exceptions.UnknownMethodException,
-    'unknown error': sel_exceptions.WebDriverException,
-    'unknown method': sel_exceptions.UnknownMethodException,
-    'unsupported operation': sel_exceptions.UnknownMethodException,
-    'element not visible': sel_exceptions.ElementNotVisibleException,
-    'element not selectable': sel_exceptions.ElementNotSelectableException,
-    'invalid coordinates': sel_exceptions.InvalidCoordinatesException,
-}
-
-
-def format_stacktrace(original: None | str | Sequence) -> list[str]:
-    if not original:
-        return []
-    if isinstance(original, str):
-        return original.split('\n')
-
-    result: list[str] = []
-    try:
-        for frame in original:
-            if not isinstance(frame, dict):
-                continue
-
-            line = frame.get('lineNumber', '')
-            file = frame.get('fileName', '<anonymous>')
-            if line:
-                file = f'{file}:{line}'
-            meth = frame.get('methodName', '<anonymous>')
-            if 'className' in frame:
-                meth = f'{frame["className"]}.{meth}'
-            result.append(f'    at {meth} ({file})')
-    except TypeError:
-        pass
-    return result
-
 
 class MobileErrorHandler(errorhandler.ErrorHandler):
     def check_response(self, response: dict[str, Any]) -> None:
-        """
-        https://www.w3.org/TR/webdriver/#errors
-        """
+        """Handle W3C errors, retaining Appium-specific exception mappings."""
         payload = response.get('value', '')
         if isinstance(payload, dict):
             payload_dict = payload
@@ -100,27 +38,23 @@ def check_response(self, response: dict[str, Any]) -> None:
         if not isinstance(value, dict):
             return
         error = value.get('error')
-        if not error:
+        if not isinstance(error, str) or not error:
             return
 
-        message = value.get('message', error)
-        stacktrace = value.get('stacktrace', '')
-        # In theory, we should also be checking HTTP status codes.
-        # Java client, for example, prints a warning if the actual `error`
-        # value does not match to the response's HTTP status code.
-        exception_class: type[sel_exceptions.WebDriverException] = ERROR_TO_EXC_MAPPING.get(
-            error, sel_exceptions.WebDriverException
-        )
-        if exception_class is sel_exceptions.WebDriverException and message:
-            if message == 'No such context found.':
+        # Pass the W3C error string as Selenium's internal status, never a
+        # JSONWP numeric status. Selenium handles data.text and stacktrace.
+        try:
+            super().check_response({'status': error, 'value': value})
+        except sel_exceptions.WebDriverException as exc:
+            if type(exc) is not sel_exceptions.WebDriverException:
+                raise
+            exception_class: type[sel_exceptions.WebDriverException]
+            if error in ('unknown command', 'unknown method', 'unsupported operation'):
+                exception_class = sel_exceptions.UnknownMethodException
+            elif exc.msg == 'No such context found.':
                 exception_class = appium_exceptions.NoSuchContextException
-            elif message == 'That command could not be executed in the current context.':
+            elif exc.msg == 'That command could not be executed in the current context.':
                 exception_class = appium_exceptions.InvalidSwitchToTargetException
-
-        if exception_class is sel_exceptions.UnexpectedAlertPresentException:
-            raise sel_exceptions.UnexpectedAlertPresentException(
-                msg=message,
-                stacktrace=format_stacktrace(stacktrace),
-                alert_text=value.get('data'),
-            )
-        raise exception_class(msg=message, stacktrace=format_stacktrace(stacktrace))
+            else:
+                raise
+            raise exception_class(msg=exc.msg, screen=exc.screen, stacktrace=exc.stacktrace) from exc

test/unit/webdriver/errorhandler_test.py

@@ -0,0 +1,95 @@
+#!/usr/bin/env python
+
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import json
+
+import httpretty
+import pytest
+from selenium.common import exceptions
+from selenium.webdriver.common.by import By
+
+from appium.common.exceptions import InvalidSwitchToTargetException, NoSuchContextException
+from appium.webdriver.errorhandler import MobileErrorHandler
+from test.unit.helper.test_helper import android_w3c_driver, appium_command
+
+
+@pytest.mark.parametrize(
+    ('error', 'message', 'expected'),
+    [
+        ('no such element', 'Missing element', exceptions.NoSuchElementException),
+        ('detached shadow root', 'Detached', exceptions.DetachedShadowRootException),
+        ('unknown command', 'Unknown command', exceptions.UnknownMethodException),
+        ('unknown method', 'Unknown method', exceptions.UnknownMethodException),
+        ('unsupported operation', 'Unsupported', exceptions.UnknownMethodException),
+        ('unknown error', 'No such context found.', NoSuchContextException),
+        ('unknown error', 'That command could not be executed in the current context.', InvalidSwitchToTargetException),
+        ('invalid argument', 'No such context found.', exceptions.InvalidArgumentException),
+        ('unknown error', 'Other error', exceptions.WebDriverException),
+    ],
+)
+def test_w3c_exception_mapping(error, message, expected):
+    response = {
+        'status': 500,
+        'value': json.dumps(
+            {
+                'value': {
+                    'error': error,
+                    'message': message,
+                    'stacktrace': 'line1\nline2',
+                }
+            }
+        ),
+    }
+    with pytest.raises(expected) as caught:
+        MobileErrorHandler().check_response(response)
+    assert type(caught.value) is expected
+    assert caught.value.msg.startswith(message)
+    assert caught.value.stacktrace == ['line1', 'line2']
+
+
+@pytest.mark.parametrize('data', [{'text': 'Allow location?'}, {}])
+@httpretty.activate
+def test_driver_unexpected_alert_text(data):
+    driver = android_w3c_driver()
+    httpretty.register_uri(
+        httpretty.POST,
+        appium_command('/session/1234567890/element'),
+        status=500,
+        body=json.dumps(
+            {
+                'value': {
+                    'error': 'unexpected alert open',
+                    'message': 'Blocked',
+                    'stacktrace': '',
+                    'data': data,
+                }
+            }
+        ),
+    )
+    with pytest.raises(exceptions.UnexpectedAlertPresentException) as caught:
+        driver.find_element(By.ID, 'button')
+    assert caught.value.alert_text == data.get('text')
+
+
+@pytest.mark.parametrize(
+    'response',
+    [
+        {'value': None},
+        {'value': {'error': 'timeout', 'message': 'User script result'}},
+        {'status': 7, 'value': {'message': 'Legacy error'}},
+        {'status': 500, 'value': '{"status": 7, "value": {"message": "Legacy error"}}'},
+    ],
+)
+def test_non_w3c_errors_are_not_interpreted(response):
+    MobileErrorHandler().check_response(response)