bug: report error from image library

integration-tests/volumes/client/scenarios.yml

@@ -150,8 +150,9 @@ scenarios:
           headers:
             # Contile maps the User-Agent Header value to os-family and form-factor parameters
             # The following value will result in os-family: ios and form-factor: tablet
+            # NOTE: Firefox for iPad returns the default desktop UA string.
             - name: User-Agent
-              value: 'Mozilla/5.0 (iPad; CPU iPhone OS 11_5_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/35.0 Mobile/15E148 Safari/605.1.15'
+              value: 'iPad; CPU iPhone OS 11_5_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/35.0 Mobile/15E148 Safari/605.1.15'
         response:
           status_code: 204 # No Content
           content: ''

src/adm/filter.rs

@@ -140,7 +140,7 @@ impl AdmFilter {
         // TODO: if not error.is_reportable, just add to metrics.
         let mut merged_tags = error.tags.clone();
         merged_tags.extend(tags.clone());
-        l_sentry::report(&merged_tags, sentry::event_from_error(error));
+        l_sentry::report(sentry::event_from_error(error), &merged_tags);
     }
 
     /// check to see if the bucket has been modified since the last time we updated.

src/adm/tiles.rs

@@ -18,6 +18,7 @@ use crate::{
     server::ServerState,
     settings::Settings,
     tags::Tags,
+    web::middleware::sentry::report,
     web::DeviceInfo,
 };
 
@@ -211,6 +212,7 @@ pub async fn get_tiles(
                     // We still want to track this as a server error later.
                     //
                     // TODO: Remove this after the shared cache is implemented.
+                    dbg!(&e);
                     let mut err: HandlerError = if e.is_timeout()
                         && Instant::now()
                             .checked_duration_since(state.start_up)
@@ -257,23 +259,33 @@ pub async fn get_tiles(
         .take(settings.adm_max_tiles as usize)
         .collect();
 
-    if filtered.is_empty() {
-        warn!("adm::get_tiles no valid tiles {}", adm_url);
-        metrics.incr_with_tags("filter.adm.all_filtered", Some(tags));
-    }
     let mut tiles: Vec<Tile> = Vec::new();
     for mut tile in filtered {
         if let Some(storage) = image_store {
             // we should have already proven the image_url in `filter_and_process`
             // we need to validate the image, store the image for eventual CDN retrieval,
             // and get the metrics of the image.
-            let result = storage.store(&tile.image_url.parse().unwrap()).await?;
-            tile.image_url = result.url.to_string();
-            // Since height should equal width, using either value here works.
-            tile.image_size = Some(result.image_metrics.width);
+            match storage.store(&tile.image_url.parse().unwrap()).await {
+                Ok(result) => {
+                    tile.image_url = result.url.to_string();
+                    // Since height should equal width, using either value here works.
+                    tile.image_size = Some(result.image_metrics.width);
+                }
+                Err(e) => {
+                    // quietly report the error, and drop the tile.
+                    report(sentry::event_from_error(&e), tags);
+                    continue;
+                }
+            }
         }
         tiles.push(tile);
     }
+
+    if tiles.is_empty() {
+        warn!("adm::get_tiles no valid tiles {}", adm_url);
+        metrics.incr_with_tags("filter.adm.all_filtered", Some(tags));
+    }
+
     Ok(TileResponse { tiles })
 }
 

src/server/img_storage.rs

@@ -187,12 +187,23 @@ impl StoreImage {
         }))
     }
 
-    pub fn meta(&self, image: &Bytes, fmt: ImageFormat) -> HandlerResult<ImageMetrics> {
+    pub fn meta(
+        &self,
+        uri: &uri::Uri,
+        image: &Bytes,
+        fmt: ImageFormat,
+    ) -> HandlerResult<ImageMetrics> {
         let mut reader = ImageReader::new(Cursor::new(image));
         reader.set_format(fmt);
-        let img = reader
-            .decode()
-            .map_err(|_| HandlerErrorKind::BadImage("Image unreadable"))?;
+        let img = reader.decode().map_err(|e| {
+            let mut tags = Tags::default();
+            tags.add_extra("error", &e.to_string());
+            tags.add_extra("url", &uri.to_string());
+            tags.add_extra("format", fmt.extensions_str().first().unwrap_or(&"Unknown"));
+            let mut err: HandlerError = HandlerErrorKind::BadImage("Image unreadable").into();
+            err.tags = tags;
+            err
+        })?;
         let rgb_img = img.to_rgb16();
         Ok(ImageMetrics {
             height: rgb_img.height(),
@@ -271,13 +282,14 @@ impl StoreImage {
                 _ => {
                     let mut tags = Tags::default();
                     tags.add_extra("url", &uri.to_string());
+                    tags.add_extra("format", content_type);
                     let mut err: HandlerError =
                         HandlerErrorKind::BadImage("Invalid image format").into();
                     err.tags = tags;
                     return Err(err);
                 }
             };
-            self.meta(image, fmt)?
+            self.meta(uri, image, fmt)?
         };
         if self.settings.metrics.symmetric && image_metrics.width != image_metrics.height {
             let mut tags = Tags::default();

src/web/handlers.rs

@@ -179,7 +179,7 @@ pub async fn get_tiles(
                     let mut tags = Tags::from_head(request.head(), settings);
                     tags.add_extra("err", es);
                     tags.add_tag("level", "warning");
-                    l_sentry::report(&tags, sentry::event_from_error(&e));
+                    l_sentry::report(sentry::event_from_error(&e), &tags);
                     warn!("ADM Server error: {:?}", e);
                     Ok(HttpResponse::NoContent().finish())
                 }

src/web/middleware/sentry.rs

@@ -62,7 +62,7 @@ pub struct SentryWrapperMiddleware<S> {
 /// Report an error with [crate::tags::Tags] and [Event] directly to sentry
 ///
 /// And [Event] can be derived using `event_from_error(HandlerError)`
-pub fn report(tags: &Tags, mut event: Event<'static>) {
+pub fn report(mut event: Event<'static>, tags: &Tags) {
     let tags = tags.clone();
     event.tags = tags.clone().tag_tree();
     event.extra = tags.extra_tree();
@@ -118,7 +118,7 @@ where
                     {
                         for event in events {
                             trace!("Sentry: found an error stored in request: {:?}", &event);
-                            report(&tags, event);
+                            report(event, &tags);
                         }
                     }
                     if let Some(events) = sresp
@@ -128,15 +128,15 @@ where
                     {
                         for event in events {
                             trace!("Sentry: Found an error stored in response: {:?}", &event);
-                            report(&tags, event);
+                            report(event, &tags);
                         }
                     }
                 }
                 Some(e) => {
                     if let Some(herr) = e.as_error::<HandlerError>() {
                         if herr.kind().is_sentry_event() {
                             tags.extend(herr.tags.clone());
-                            report(&tags, event_from_error(herr));
+                            report(event_from_error(herr), &tags);
                         } else if let Some(label) = herr.kind().metric_label() {
                             metrics.incr_with_tags(label).send()
                         }