Handle invalid client ID in ApitallyClient (#161)

* Handle invalid client ID in ApitallyClient

* Fix

* Add test

* Fix

* Fix

Author: itssimon

.github/workflows/tests.yaml

@@ -64,7 +64,7 @@ jobs:
           - django-ninja==0.18.0 django
           - litestar
           - litestar==2.0.1
-          - blacksheep
+          - blacksheep==2.4.1 # current version v2.4.2 is broken
           - blacksheep==2.1.0
         exclude:
           - python: "3.12"

apitally/client/client_asyncio.py

@@ -50,6 +50,8 @@ def get_http_client(self) -> httpx.AsyncClient:
             return httpx.AsyncClient(base_url=self.hub_url, timeout=REQUEST_TIMEOUT, proxies=self.proxy)
 
     def start_sync_loop(self) -> None:
+        if not self.enabled:
+            return  # pragma: no cover
         self._stop_sync_loop = False
         self._sync_loop_task = asyncio.create_task(self._run_sync_loop())
 

apitally/client/client_base.py

@@ -43,16 +43,23 @@ def __new__(cls: Type[TApitallyClient], *args, **kwargs) -> TApitallyClient:
     def __init__(self, client_id: str, env: str, request_logging_config: Optional[RequestLoggingConfig] = None) -> None:
         if hasattr(self, "client_id"):
             raise RuntimeError("Apitally client is already initialized")  # pragma: no cover
+
+        client_id_valid = True
+        env_valid = True
+
         try:
             UUID(client_id)
         except ValueError:
-            raise ValueError(f"invalid client_id '{client_id}' (expecting hexadecimal UUID format)")
+            logger.error(f"invalid client_id '{client_id}' (expecting hexadecimal UUID format)")
+            client_id_valid = False
+
         if re.match(r"^[\w-]{1,32}$", env) is None:
-            raise ValueError(f"invalid env '{env}' (expecting 1-32 alphanumeric lowercase characters and hyphens only)")
+            logger.error(f"invalid env '{env}' (expecting 1-32 alphanumeric lowercase characters and hyphens only)")
+            env_valid = False
 
         self.client_id = client_id
         self.env = env
-        self.enabled = True
+        self.enabled = client_id_valid and env_valid
         self.instance_uuid = str(uuid4())
         self.request_counter = RequestCounter()
         self.validation_error_counter = ValidationErrorCounter()

apitally/client/client_threading.py

@@ -61,6 +61,8 @@ def __init__(
         self._sync_data_queue: Queue[Dict[str, Any]] = Queue()
 
     def start_sync_loop(self) -> None:
+        if not self.enabled:
+            return  # pragma: no cover
         self._stop_sync_loop.clear()
         if self._thread is None or not self._thread.is_alive():
             self._thread = Thread(target=self._run_sync_loop, daemon=True)

tests/test_client_base.py

@@ -0,0 +1,16 @@
+def test_client_id_validation():
+    from apitally.client.client_base import ApitallyClientBase
+
+    ApitallyClientBase._instance = None
+    client = ApitallyClientBase(client_id="xxx", env="test")
+    assert client.enabled is False
+
+    ApitallyClientBase._instance = None
+    client = ApitallyClientBase(client_id="9003a5c6-0725-4502-8e57-963a21ba97b6", env="")
+    assert client.enabled is False
+
+    ApitallyClientBase._instance = None
+    client = ApitallyClientBase(client_id="9003a5c6-0725-4502-8e57-963a21ba97b6", env="test")
+    assert client.enabled is True
+
+    ApitallyClientBase._instance = None