bug: handle bad responses from ADM

src/adm.rs

@@ -1,14 +1,14 @@
 use serde::{Deserialize, Serialize};
 use url::Url;
 
-use crate::error::HandlerError;
+use crate::error::{HandlerError, HandlerErrorKind};
 
 #[derive(Debug, Deserialize, Serialize)]
 pub struct AdmTileResponse {
     pub tiles: Vec<AdmTile>,
 }
 
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Debug, Default, Deserialize, Serialize)]
 pub struct AdmTile {
     pub id: u64,
     pub name: String,
@@ -44,16 +44,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

@@ -38,6 +38,9 @@ pub enum HandlerErrorKind {
 
     #[error("Location error: {:?}", _0)]
     Location(String),
+
+    #[error("Bad Adm response: {:?}", _0)]
+    BadAdmResponse(String),
 }
 
 impl HandlerErrorKind {
@@ -55,6 +58,7 @@ impl HandlerErrorKind {
             HandlerErrorKind::General(_) => 500,
             HandlerErrorKind::Internal(_) => 510,
             HandlerErrorKind::Reqwest(_) => 520,
+            HandlerErrorKind::BadAdmResponse(_) => 521,
             HandlerErrorKind::Validation(_) => 600,
             HandlerErrorKind::Location(_) => 530,
         }

src/web/handlers.rs

@@ -10,6 +10,7 @@ use crate::{
     server::{cache, ServerState},
     tags::Tags,
     web::extractors::TilesRequest,
+    web::middleware::sentry as l_sentry,
 };
 
 pub async fn get_image(
@@ -90,24 +91,46 @@ 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,
         fake_ip,
         &stripped_ua,
         &treq.placement,
     )
-    .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);
+                "[]".to_owned()
+            }
+            _ => return Err(e),
         },
-    );
+    };
 
     Ok(HttpResponse::Ok()
         .content_type("application/json")