diff --git a/image-loader/app/controllers/ImageLoaderController.scala b/image-loader/app/controllers/ImageLoaderController.scala index 8ec139691e..4ee1e4dd3f 100644 --- a/image-loader/app/controllers/ImageLoaderController.scala +++ b/image-loader/app/controllers/ImageLoaderController.scala @@ -157,10 +157,19 @@ class ImageLoaderController(auth: Authentication, metrics.successfulIngestsFromQueue.incrementBothWithAndWithoutDimensions(metricDimensions).run logger.info(logMarker, s"Successfully processed image ${digestedFile.file.getName}") store.deleteObjectFromIngestBucket(s3IngestObject.key) - } recover { case t: Throwable => - metrics.failedIngestsFromQueue.incrementBothWithAndWithoutDimensions(metricDimensions).run - logger.error(logMarker, s"Failed to process file. Moving to fail bucket.", t) - store.moveObjectToFailedBucket(s3IngestObject.key) + } recover { + case unsupportedMimeTypeException: UnsupportedMimeTypeException => + metrics.failedIngestsFromQueue.incrementBothWithAndWithoutDimensions(metricDimensions).run + metrics.invalidMimeTypeUploaded.increment(List( + new Dimension().withName("MimeType").withValue(unsupportedMimeTypeException.mimeType), + new Dimension().withName("UploadedBy").withValue(s3IngestObject.uploadedBy) + )) + logger.info(logMarker, s"File has unsupported mime type(${unsupportedMimeTypeException.mimeType}). Moving to fail bucket.") + store.moveObjectToFailedBucket(s3IngestObject.key) + case t: Throwable => + metrics.failedIngestsFromQueue.incrementBothWithAndWithoutDimensions(metricDimensions).run + logger.error(logMarker, s"Failed to process file. Moving to fail bucket.", t) + store.moveObjectToFailedBucket(s3IngestObject.key) } } } @@ -439,53 +448,43 @@ class ImageLoaderController(auth: Authentication, } } + private def attemptUpdateStatusAndLogErrorIfFails(imageId: String, status: UploadStatus)(implicit logMarker:LogMarker): Future[Unit] = { + uploadStatusTable.updateStatus(imageId, status) + .map { + case Left (_: ConditionNotMet) => { + logger.error(logMarker, s"ConditionNotMet error occurred while updating image upload status, image-id:${imageId}") + } + case Left(_: ScanamoError) => { + logger.error(logMarker, "an error occurred while updating image upload status, image-id:${imageId}") + } + case Right(_) => () + } + .recover { + case error => + logger.error(logMarker, s"an error occurred while updating image upload status, image-id:${imageId}", error) + } + } + private def updateUploadStatusTable ( uploadAttempt: Future[UploadStatusUri], digestedFile: DigestedFile )(implicit logMarker:LogMarker): Future[Unit] = { - def reportFailure (error:Throwable): Unit = { - val errorMessage = s"an error occurred while updating image upload status, error:$error" - logger.error(logMarker, errorMessage, error) - Future.failed(new Exception(errorMessage)) - } - def reportScanamoError (error:ScanamoError): Unit = { - val errorMessage = error match { - case ConditionNotMet(_) => s"ConditionNotMet error occurred while updating image upload status, image-id:${digestedFile.digest}, error:$error" - case _ => s"an error occurred while updating image upload status, image-id:${digestedFile.digest}, error:$error" - } - logger.error(logMarker, errorMessage) - Future.failed(new Exception(errorMessage)) - } - - uploadAttempt + uploadAttempt .recover { case uploadFailure => - uploadStatusTable.updateStatus( //FIXME use set status to avoid potential ConditionNotMet (when status table rows have expired/TTL) + attemptUpdateStatusAndLogErrorIfFails( //FIXME use set status to avoid potential ConditionNotMet (when status table rows have expired/TTL) digestedFile.digest, UploadStatus(StatusType.Failed, Some(s"${uploadFailure.getClass.getName}: ${uploadFailure.getMessage}")) ) - .recover { - case error => reportFailure(error) - } - .map { - case Left(error:ScanamoError) => reportScanamoError(error) - case Right => uploadFailure - } + throw uploadFailure } - .map { + .flatMap { case uploadStatusUri:UploadStatusUri => - uploadStatusTable.updateStatus( //FIXME use set status to avoid potential ConditionNotMet (when status table rows have expired/TTL) + attemptUpdateStatusAndLogErrorIfFails( //FIXME use set status to avoid potential ConditionNotMet (when status table rows have expired/TTL) digestedFile.digest, UploadStatus(StatusType.Completed, None) ) - .recover { - case error => reportFailure(error) - } - .map { - case Left(error:ScanamoError) => reportScanamoError(error) - case Right => uploadStatusUri - } } } diff --git a/image-loader/app/lib/ImageLoaderMetrics.scala b/image-loader/app/lib/ImageLoaderMetrics.scala index 7e27b89e9e..e48c988aa8 100644 --- a/image-loader/app/lib/ImageLoaderMetrics.scala +++ b/image-loader/app/lib/ImageLoaderMetrics.scala @@ -9,4 +9,6 @@ class ImageLoaderMetrics(config: ImageLoaderConfig) extends CloudWatchMetrics (n val failedIngestsFromQueue = new CountMetric("FailedIngestsFromQueue") val abandonedMessagesFromQueue = new CountMetric("AbandonedMessagesFromQueue") + + val invalidMimeTypeUploaded = new CountMetric("invalidMimeTypeUploaded") }