Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No error logging for unsupported file type #4234

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

No error logging for unsupported file type #4234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b30b74
refactor updateUploadStatusTable to throw uploadfailures, log as info…
dblatcher Jan 4, 2024
3e66e9c
factor out the attemptUpdateStatusAndLogErrorIfFails operation
dblatcher Jan 5, 2024
b20d72f
add metric for uploads with invalid mimetypes
dblatcher Jan 5, 2024
2882451
remove println
dblatcher Jan 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 35 additions & 36 deletions image-loader/app/controllers/ImageLoaderController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions image-loader/app/lib/ImageLoaderMetrics.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}