feat(import): surface S3 upload failures in skippedFiles #619

New Issue

marcel · 2026-05-18T15:20:52+02:00

marcel commented

2026-05-18 15:20:52 +02:00

Context

When importSingleDocument fails to upload a file to S3, it currently returns false and logs an error, but the document is neither counted in processed nor added to skippedFiles. An admin viewing the DONE card after such an import sees only the success count with no indication that files were silently dropped.

This was pre-existing behaviour before #529. Now that skippedFiles exists as a mechanism to surface failures, S3 errors should land there too.

Proposed change

When the S3 putObject call throws, add the filename to skippedFiles with reason S3_UPLOAD_ERROR instead of silently returning false.

// MassImportService.java — inside importSingleDocument, S3 catch block
} catch (Exception e) {
    log.error("S3 Upload Fehler für {}", file.get().getName(), e);
    return ImportResult.FAILED_S3;  // or: return new SkippedFile(filename, "S3_UPLOAD_ERROR")
}

Add i18n key for import_reason_s3_upload_error (de/en/es).

Acceptance criteria

Given an import where one file's S3 upload throws, when the import completes, then skippedFiles contains an entry for that file with reason S3_UPLOAD_ERROR
Given that file, when the admin opens the import card, then the filename appears in the collapsible skipped list with a human-readable label
Regression test covers the S3-failure path analogous to the existing isPdfMagicBytes tests

Surfaced during review of #618. The importSingleDocument boolean return contract (conflating "already exists" vs "upload failed") is tracked separately in #[refactor issue].

## Context When `importSingleDocument` fails to upload a file to S3, it currently returns `false` and logs an error, but the document is neither counted in `processed` nor added to `skippedFiles`. An admin viewing the DONE card after such an import sees only the success count with no indication that files were silently dropped. This was pre-existing behaviour before #529. Now that `skippedFiles` exists as a mechanism to surface failures, S3 errors should land there too. ## Proposed change When the S3 `putObject` call throws, add the filename to `skippedFiles` with reason `S3_UPLOAD_ERROR` instead of silently returning `false`. ```java // MassImportService.java — inside importSingleDocument, S3 catch block } catch (Exception e) { log.error("S3 Upload Fehler für {}", file.get().getName(), e); return ImportResult.FAILED_S3; // or: return new SkippedFile(filename, "S3_UPLOAD_ERROR") } ``` Add i18n key for `import_reason_s3_upload_error` (de/en/es). ## Acceptance criteria - Given an import where one file's S3 upload throws, when the import completes, then `skippedFiles` contains an entry for that file with reason `S3_UPLOAD_ERROR` - Given that file, when the admin opens the import card, then the filename appears in the collapsible skipped list with a human-readable label - Regression test covers the S3-failure path analogous to the existing `isPdfMagicBytes` tests ## Related Surfaced during review of #618. The `importSingleDocument` boolean return contract (conflating "already exists" vs "upload failed") is tracked separately in #[refactor issue].

marcel added the P2-medium feature labels 2026-05-18 15:21:11 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#619