bug: handle bad responses from ADM (#57)

* bug: handle bad responses from ADM

ADM can sometimes return invalid content (e.g. a 200 response with no
"tiles".

Report this to sentry, and log, but we return an "empty" value to the
client.

Closes #54

src/adm.rs

@@ -288,16 +288,22 @@ pub async fn get_tiles(
     let adm_url = adm_url.as_str();
 
     trace!("get_tiles GET {}", adm_url);
-    // XXX: handle empty responses -- AdM sends empty json in that case
-    // 'Error("missing field `tiles`"'
     let mut response: AdmTileResponse = reqwest_client
         .get(adm_url)
         .header(reqwest::header::USER_AGENT, stripped_ua)
         .send()
-        .await?
+        .await
+        .map_err(|e| {
+            // ADM servers are down, or improperly configured
+            HandlerErrorKind::BadAdmResponse(format!("ADM Server Error: {:?}", e))
+        })?
         .error_for_status()?
         .json()
-        .await?;
+        .await
+        .map_err(|e| {
+            // ADM servers are not returning correct information
+            HandlerErrorKind::BadAdmResponse(format!("ADM provided invalid response: {:?}", e))
+        })?;
     response.tiles = response
         .tiles
         .into_iter()

src/error.rs

@@ -50,6 +50,9 @@ pub enum HandlerErrorKind {
 
     #[error("Location error: {:?}", _0)]
     Location(String),
+
+    #[error("Bad Adm response: {:?}", _0)]
+    BadAdmResponse(String),
 }
 
 impl HandlerErrorKind {
@@ -67,6 +70,7 @@ impl HandlerErrorKind {
             HandlerErrorKind::General(_) => 500,
             HandlerErrorKind::Internal(_) => 510,
             HandlerErrorKind::Reqwest(_) => 520,
+            HandlerErrorKind::BadAdmResponse(_) => 521,
             HandlerErrorKind::Validation(_) => 600,
             HandlerErrorKind::InvalidHost(_, _) => 601,
             HandlerErrorKind::UnexpectedHost(_, _) => 602,

src/web/handlers.rs

@@ -1,5 +1,6 @@
 //! API Handlers
 use actix_web::{web, HttpRequest, HttpResponse};
+use serde_json::json;
 
 use super::user_agent;
 use crate::{
@@ -9,6 +10,7 @@ use crate::{
     server::{cache, location::LocationResult, ServerState},
     tags::Tags,
     web::extractors::TilesRequest,
+    web::middleware::sentry as l_sentry,
 };
 
 pub async fn get_tiles(
@@ -75,7 +77,7 @@ pub async fn get_tiles(
             .body(&tiles.json));
     }
 
-    let response = adm::get_tiles(
+    let tiles = match adm::get_tiles(
         &state.reqwest_client,
         &state.adm_endpoint_url,
         &location,
@@ -84,17 +86,39 @@ pub async fn get_tiles(
         &state,
         &mut tags,
     )
-    .await?;
-    let tiles = serde_json::to_string(&response)
-        .map_err(|e| HandlerError::internal(&format!("Response failed to serialize: {}", e)))?;
-    trace!("get_tiles: cache miss: {:?}", audience_key);
-    metrics.incr("tiles_cache.miss");
-    state.tiles_cache.write().await.insert(
-        audience_key,
-        cache::Tiles {
-            json: tiles.clone(),
+    .await
+    {
+        Ok(response) => {
+            // adM sometimes returns an invalid response. We don't want to cache that.
+            let tiles = serde_json::to_string(&response).map_err(|e| {
+                HandlerError::internal(&format!("Response failed to serialize: {}", e))
+            })?;
+            trace!("get_tiles: cache miss: {:?}", audience_key);
+            metrics.incr("tiles_cache.miss");
+            state.tiles_cache.write().await.insert(
+                audience_key,
+                cache::Tiles {
+                    json: tiles.clone(),
+                },
+            );
+            tiles
+        }
+        Err(e) => match e.kind() {
+            HandlerErrorKind::BadAdmResponse(es) => {
+                warn!("Bad response from ADM: {:?}", e);
+                // Report directly to sentry
+                // (This is starting to become a pattern. 🤔)
+                let mut tags = Tags::from(request.head());
+                tags.add_extra("err", es);
+                tags.add_tag("level", "warning");
+                l_sentry::report(&tags, sentry::event_from_error(&e));
+                //TODO: probably should do: json!(vec![adm::AdmTile::default()]).to_string()
+                warn!("ADM Server error: {:?}", e);
+                json!({"tiles":[]}).to_string()
+            }
+            _ => return Err(e),
         },
-    );
+    };
 
     Ok(HttpResponse::Ok()
         .content_type("application/json")