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

[CCAP-670] Enhance file conversion service to retry up to 10 times if LibreOffice fails #658

Merged
merged 1 commit into from
Feb 12, 2025
+8 −8
Merged

[CCAP-670] Enhance file conversion service to retry up to 10 times if LibreOffice fails #658

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions src/main/java/formflow/library/file/FileConversionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public Set<MultipartFile> convertFileToPDF(MultipartFile file) {
}
case OFFICE_DOCUMENT -> {
log.info("Converting document of type {} to PDF", fileMimeType);
yield convertOfficeDocumentToPDF(file);
yield convertOfficeDocumentToPDF(file, 0);
}
default -> {
log.info("Skipping converting file of type {} to PDF", fileMimeType);
Expand Down Expand Up @@ -152,6 +152,7 @@ private Set<MultipartFile> convertImageToPDF(MultipartFile file) {
log.info("Compressed file size {}", convertedPDF.getSize());

if (!isTooLarge(convertedPDF)) {
log.info("Successfully converted image to PDF with file size: {}", convertedPDF.getSize());
break;
}
}
Expand Down Expand Up @@ -232,7 +233,7 @@ private byte[] compressAndScaleImage(byte[] image, float quality) throws IOExcep
return byteArrayOutputStream.toByteArray();
}

private Set<MultipartFile> convertOfficeDocumentToPDF(MultipartFile file) {
private Set<MultipartFile> convertOfficeDocumentToPDF(MultipartFile file, int numberOfRetries) {
File inputFile;
File pdfFile;
File compressedPDFFile;
Expand Down Expand Up @@ -274,13 +275,12 @@ private Set<MultipartFile> convertOfficeDocumentToPDF(MultipartFile file) {
// Read the converted PDF from disk
pdfFile = new File(outputPdfPath);
if (!pdfFile.exists()) {
if (exitCode != 0) {
throw new RuntimeException("PDF conversion failed. Unable to find PDF file " + pdfFile.getAbsolutePath());
if (numberOfRetries < 10) {
numberOfRetries++;
log.warn("Unable to find PDF file {} with exit code: {}, will retry with attempt {}", pdfFile.getAbsolutePath(), exitCode, numberOfRetries);
return convertOfficeDocumentToPDF(file, numberOfRetries);
} else {
// There is a race condition where soffice will say everything is great, but it's not great and there's no
// converted file. We can retry in this situation.
log.warn("Unable to find PDF file {}, will retry.", pdfFile.getAbsolutePath());
return convertOfficeDocumentToPDF(file);
throw new RuntimeException("PDF conversion failed. Unable to find PDF file " + pdfFile.getAbsolutePath() + " after " + numberOfRetries + " retries. Final exit code: " + exitCode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when this exception is thrown after 10 retries?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gets all the way up to the FileController:

form-flow/src/main/java/formflow/library/FileController.java

Line 260 in bd983f2

log.error("Error converting file {} to PDF", userFileId, e);

And then nothing happens except logging that nothing was converted: https://github.com/codeforamerica/form-flow/blob/bd983f2a93ceacbb5fc1df91c05c3f7d11afe4a5/src/main/java/formflow/library/FileController.java#L280C19-L280C42

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it bubbling up to the FileController makes sense, but maybe we should make a slack alert in Datadog if this happens so we can be alerted, as my understanding is if this happens, then it's possible we don't end up sending a supporting document with an application?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, there's already a datadog alert for all the warns/errors/interesting-infos and they are going to #snlab-il-doc-conversion-alerts

I'll pare those down into more interesting/useful alerts and send them to the normal alerts channel once we're done testing and we don't care about every single logging statement!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sweet! Thanks for being thorough!

}
}

Expand Down