Keep report() from raising on a non-string name and the sender alive on a scheme-less URL

tests/test_trace_client.py

@@ -224,6 +224,28 @@ def test_report_ignores_a_blank_name(self):
         client.close()
         self.assertFalse(self.capture.arrived.wait(0.3))
 
+    def test_report_does_not_raise_for_a_name_that_is_not_a_string(self):
+        client = TraceClient(self.base_url, "MyGame", key="k")
+        client.report(123)  # must not raise
+        client.close()
+        self.assertFalse(self.capture.arrived.wait(0.3), "a report that could not be built is dropped")
+        self.assertTrue(any("could not queue 123" in r.getMessage() for r in self.log),
+                        [r.getMessage() for r in self.log])
+        self.assertTrue(all(r.levelno == logging.DEBUG for r in self.log))
+
+    def test_a_base_url_without_a_scheme_is_logged_not_a_dead_thread(self):
+        crashes = []
+        with mock.patch.object(threading, "excepthook", crashes.append):
+            client = TraceClient("trace.example.org", "MyGame", key="k")
+            client.report("startup")
+            client.report("shutdown")
+            client.close()
+        self.assertEqual([], crashes, "the sender thread must not die with a traceback on stderr")
+        failures = [r for r in self.log if "could not deliver" in r.getMessage()]
+        self.assertEqual(2, len(failures), "each report is logged and the thread keeps going: %s"
+                         % [r.getMessage() for r in self.log])
+        self.assertTrue(all(r.levelno == logging.DEBUG for r in self.log))
+
     def test_constructor_rejects_a_missing_base_url_or_application(self):
         for base_url, application in ((None, "MyGame"), (" ", "MyGame"), ("http://x", None), ("http://x", "")):
             with self.assertRaises(ValueError):

trace_client/trace_client.py

@@ -134,9 +134,11 @@ def report(self, name: str, value: Optional[float] = None,
                tags: Optional[Mapping[str, str]] = None) -> None:
         """Report that ``name`` happened, with an optional numeric value and
         optional string tags. Returns immediately; see the class docstring."""
-        if self._queue is None or not name or not name.strip():
+        if self._queue is None:
             return
         try:
+            if not name or not name.strip():
+                return
             body = _json(self._application, name, value, tags)
             self._queue.put_nowait(body)
         except queue.Full:
@@ -181,14 +183,16 @@ def _drain(self) -> None:
             self._send(body)
 
     def _send(self, body: bytes) -> None:
-        request = urllib.request.Request(
-            self._endpoint, data=body, method="POST",
-            headers={
-                "Content-Type": "application/json; charset=utf-8",
-                "Authorization": "Bearer " + self._key,
-                "User-Agent": "trace-client-python/%s (%s)" % (__version__, self._application),
-            })
         try:
+            # Built inside the try: a base URL without a scheme fails here, and
+            # an uncaught error would kill the sender thread with a traceback.
+            request = urllib.request.Request(
+                self._endpoint, data=body, method="POST",
+                headers={
+                    "Content-Type": "application/json; charset=utf-8",
+                    "Authorization": "Bearer " + self._key,
+                    "User-Agent": "trace-client-python/%s (%s)" % (__version__, self._application),
+                })
             with urllib.request.urlopen(request, timeout=self.TIMEOUT_SECONDS) as response:
                 status = response.status
                 response.read()