Browse Source

Fix header lookup (#415)

* Fix header lookup

* Changelog

* Value is also bytes

* Missing else statement

* Incorporate review comments
Brendan Abolivier 2 years ago
parent
commit
26d858f357
2 changed files with 19 additions and 12 deletions
  1. 1 0
      changelog.d/415.bugfix
  2. 18 12
      sydent/sms/openmarket.py

+ 1 - 0
changelog.d/415.bugfix

@@ -0,0 +1 @@
+Fix a long-standing bug with error handling around missing headers when dealing with the OpenMarket API, which could cause the wrong assumption that sending a SMS failed when it didn't.

+ 18 - 12
sydent/sms/openmarket.py

@@ -103,8 +103,8 @@ class OpenMarketSMS:
         headers = dict(resp.headers.getAllRawHeaders())
 
         request_id = None
-        if "X-Request-Id" in headers:
-            request_id = headers["X-Request-Id"][0]
+        if b"X-Request-Id" in headers:
+            request_id = headers[b"X-Request-Id"][0].decode("UTF-8")
 
         # Catch errors from the API. The documentation says a success code should be 202
         # Accepted, but let's be broad here just in case and accept all 2xx response
@@ -132,20 +132,26 @@ class OpenMarketSMS:
                 ),
             )
 
-        if "Location" not in headers:
-            raise Exception("Got response from sending SMS with no location header")
-        # Nominally we should parse the URL, but we can just split on '/' since
-        # we only care about the last part.
-        parts = headers["Location"][0].split("/")
-        if len(parts) < 2:
-            raise Exception(
-                "Got response from sending SMS with malformed location header"
-            )
+        ticket_id = None
+        if b"Location" not in headers:
+            logger.error("Got response from sending SMS with no location header")
+        else:
+            # Nominally we should parse the URL, but we can just split on '/' since
+            # we only care about the last part.
+            value = headers[b"Location"][0].decode("UTF-8")
+            parts = value.split("/")
+            if len(parts) < 2:
+                logger.error(
+                    "Got response from sending SMS with malformed location header: %s",
+                    value,
+                )
+            else:
+                ticket_id = parts[-1]
 
         logger.info(
             "Successfully sent SMS (ticket ID: %s, request ID %s), OpenMarket API"
             " responded with code %d",
-            parts[-1],
+            ticket_id,
             request_id,
             resp.code,
         )