Skip to content

Feat: Improve Error Message on Upload for Corrupted Files #2335

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

Open
LevinCeglie wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Feat: Improve Error Message on Upload for Corrupted Files #2335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03f7c7e
feat(queueCon): add stateComment to save detailed file validation and…
LevinCeglie Jul 8, 2026
8f38fed
feat(frontend): display stateComment for corrupted files
LevinCeglie Jul 8, 2026
0962787
feat(cli/sdk): display file stateComment in 'klein file info <uuid>'
LevinCeglie Jul 8, 2026
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
17 changes: 17 additions & 0 deletions backend/migration/migrations/1783504170192-AddStateCommentToFileEntity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class AddStateCommentToFileEntity1783504170192 implements MigrationInterface {
name = 'AddStateCommentToFileEntity1783504170192';

public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "file_entity" ADD "stateComment" text`,
);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "file_entity" DROP COLUMN "stateComment"`,
);
}
}
3 changes: 3 additions & 0 deletions cli/kleinkram/api/deser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class FileObjectKeys(str, Enum):
HASH = "hash"
TYPE = "type"
CATEGORIES = "categories"
STATE_COMMENT = "stateComment"


class MissionObjectKeys(str, Enum):
Expand Down Expand Up @@ -226,6 +227,7 @@ def _parse_file(file: FileObject) -> File:
created_at = _parse_datetime(file[FileObjectKeys.CREATED_AT])
updated_at = _parse_datetime(file[FileObjectKeys.UPDATED_AT])
state = _parse_file_state(file[FileObjectKeys.STATE])
state_comment = file.get(FileObjectKeys.STATE_COMMENT)
categories_raw = file.get(FileObjectKeys.CATEGORIES) or []
categories = [c["name"] if isinstance(c, dict) and "name" in c else str(c) for c in categories_raw]

Expand All @@ -241,6 +243,7 @@ def _parse_file(file: FileObject) -> File:
date=fdate,
categories=categories,
state=state,
state_comment=state_comment,
created_at=created_at,
updated_at=updated_at,
mission_id=mission_id,
Expand Down
7 changes: 5 additions & 2 deletions cli/kleinkram/api/file_transfer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,11 +491,14 @@ def _download_state_message(state: DownloadState, path: Path, file: File) -> Opt
False,
),
DownloadState.SKIPPED_INVALID_REMOTE_STATE: (
f"skipped {path}, remote file has invalid state ({file.state.value})",
f"skipped {path}, remote file has invalid state ({file.state.value})"
+ (f": {file.state_comment}" if file.state_comment else ""),
False,
),
DownloadState.SKIPPED_CORRUPTED: (
f"skipped {path}, remote file is CORRUPTED (use --allow-corrupt to override)",
f"skipped {path}, remote file is CORRUPTED"
+ (f": {file.state_comment}" if file.state_comment else "")
+ " (use --allow-corrupt to override)",
False,
),
DownloadState.SKIPPED_CORRUPTED_LOCAL_OK: (
Expand Down
1 change: 1 addition & 0 deletions cli/kleinkram/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class File:
categories: List[str] = field(default_factory=list)
topics: List[str] = field(default_factory=list)
state: FileState = FileState.OK
state_comment: Optional[str] = None


class ExecutionStatus(str, Enum):
Expand Down
2 changes: 2 additions & 0 deletions cli/kleinkram/printing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ def file_info_table(file: File) -> Table:
table.add_row("updated", str(file.updated_at))
table.add_row("size", format_bytes(file.size))
table.add_row("state", file_state_to_text(file.state))
if file.state_comment:
table.add_row("state comment", Text(file.state_comment, style="dim"))
table.add_row("categories", ", ".join(file.categories))
table.add_row("topics", ", ".join(file.topics))
table.add_row("hash", file.hash)
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/explorer-page/explorer-page-files-table.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
:color="getColorFileState(props.row.state)"
size="20px"
>
<q-tooltip>{{ getTooltip(props.row.state) }}</q-tooltip>
<q-tooltip>{{
getTooltip(props.row.state, props.row.stateComment)
}}</q-tooltip>
</q-icon>
</q-td>
</template>
Expand Down
13 changes: 13 additions & 0 deletions frontend/src/components/inspect-file/file-error-state.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@
<span class="text-weight-bold">.{{ fileExtension }}</span>
</div>
</div>

<div
v-if="file.stateComment"
class="text-caption q-mt-md text-red"
style="
max-width: 600px;
word-break: break-word;
margin-left: auto;
margin-right: auto;
"
>
{{ file.stateComment }}
</div>
</div>
</template>

Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/inspect-file/file-header.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@
"
size="sm"
>
<q-tooltip>{{ getTooltip(file?.state) }}</q-tooltip>
<q-tooltip>{{
getTooltip(file?.state, file?.stateComment)
}}</q-tooltip>
</q-icon>
</div>
<div class="col-12 col-md-1">
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/pages/data-table-page.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
:color="getColorFileState(props.row.state)"
size="20px"
>
<q-tooltip>{{ getTooltip(props.row.state) }}</q-tooltip>
<q-tooltip>{{
getTooltip(props.row.state, props.row.stateComment)
}}</q-tooltip>
</q-icon>
</q-td>
</template>
Expand Down
35 changes: 27 additions & 8 deletions frontend/src/services/generic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,34 +291,53 @@ export function getActionColor(state: ActionState) {
}
}

export function getTooltip(state: FileState | undefined) {
export function getTooltip(
state: FileState | undefined,
stateComment?: string | null,
) {
if (state === undefined) {
return 'Unknown';
}

let baseText = '';
switch (state) {
case FileState.OK: {
return 'File is OK';
baseText = 'File is OK';
break;
}
case FileState.ERROR: {
return 'File has an error';
baseText = 'File has an error';
break;
}
case FileState.UPLOADING: {
return 'File is uploading';
baseText = 'File is uploading';
break;
}
case FileState.CORRUPTED: {
return 'File is corrupted';
baseText = 'File is corrupted';
break;
}
case FileState.LOST: {
return 'File cannot be found in storage';
baseText = 'File cannot be found in storage';
break;
}
case FileState.FOUND: {
return 'File was recovered';
baseText = 'File was recovered';
break;
}
case FileState.CONVERSION_ERROR: {
return 'File conversion failed';
baseText = 'File conversion failed';
break;
}
default: {
baseText = 'Unknown';
}
}

if (stateComment) {
return `${baseText}: ${stateComment}`;
}
return baseText;
}

export function getIcon(state: FileState) {
Expand Down
9 changes: 9 additions & 0 deletions packages/api-dto/src/types/file/file.dto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ export class FileDto {
@Expose()
state!: FileState;

@ApiProperty({
description: 'Diagnostic message when state is non-OK',
required: false,
})
@IsString()
@IsOptional()
@Expose()
stateComment?: string | null;
Comment on lines +79 to +86

@greptile-apps greptile-apps Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Raw Diagnostics Become Public

This exposes stateComment through normal file DTO responses while the new queue-consumer writers persist String(error) from conversion and extraction failures. A parser, storage, or filesystem error can include internal paths, object keys, or stack-like text, and that raw diagnostic is then shown in the UI and CLI to anyone who can read the file metadata.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/api-dto/src/types/file/file.dto.ts
Line: 79-86

Comment:
**Raw Diagnostics Become Public**

This exposes `stateComment` through normal file DTO responses while the new queue-consumer writers persist `String(error)` from conversion and extraction failures. A parser, storage, or filesystem error can include internal paths, object keys, or stack-like text, and that raw diagnostic is then shown in the UI and CLI to anyone who can read the file metadata.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@LevinCeglie LevinCeglie Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not sure about our policy, @wp99cp


@ApiProperty({
description: 'The creator of the file',
type: () => UserDto,
Expand Down
3 changes: 3 additions & 0 deletions packages/backend-common/src/entities/file/file.entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ export class FileEntity extends BaseEntity {
@Column({ type: 'enum', enum: FileState, default: FileState.OK })
state!: FileState;

@Column({ type: 'text', nullable: true })
stateComment?: string | null;

@Column({ nullable: true })
hash?: string;

Expand Down
14 changes: 9 additions & 5 deletions queueConsumer/src/file-processor/file-ingestion.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,20 @@ export class FileIngestionService {
fileData.filePath,
);

const isValid = await MagicNumberValidator.validate(
fileData.filePath,
primaryFile.type,
);
const validationResult =
await MagicNumberValidator.validate(
fileData.filePath,
primaryFile.type,
);

if (!isValid) {
if (!validationResult.valid) {
logger.warn(
`Magic number validation failed for ${primaryFile.filename} (${primaryFile.type})`,
);
primaryFile.state = FileState.CORRUPTED;
primaryFile.stateComment =
validationResult.error ??
'Magic number validation failed';
await this.fileRepo.save(primaryFile);
await this.updateQueueState(
queueItem,
Expand Down
1 change: 1 addition & 0 deletions queueConsumer/src/file-processor/file-queue-processor.provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export class FileQueueProcessorProvider {
file.hash = hash;
// Ensure state is OK if it was somehow different
file.state = FileState.OK;
file.stateComment = null;
await this.fileRepo.save(file);
logger.debug(`Updated hash for ${fileUuid}`);
} catch (error) {
Expand Down
2 changes: 2 additions & 0 deletions queueConsumer/src/file-processor/handlers/abstract-metadata.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export abstract class AbstractMetadataService {
targetEntity.date = fileDate;
}
targetEntity.state = FileState.OK;
targetEntity.stateComment = null;
targetEntity.size = fileSize;
await this.fileRepo.save(targetEntity);

Expand Down Expand Up @@ -97,6 +98,7 @@ export abstract class AbstractMetadataService {
`Metadata extraction finalize failed for ${targetEntity.filename}: ${String(error)}`,
);
targetEntity.state = FileState.CONVERSION_ERROR;
targetEntity.stateComment = String(error);
await this.fileRepo.save(targetEntity);
throw error;
}
Expand Down
4 changes: 4 additions & 0 deletions queueConsumer/src/file-processor/handlers/bag.hander.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export class RosBagHandler implements FileHandler {
} catch (error: unknown) {
logger.error(`RosBag Conversion failed: ${String(error)}`);
primaryFile.state = FileState.CONVERSION_ERROR;
primaryFile.stateComment = String(error);
await this.fileRepo.save(primaryFile);
throw error;
}
Expand All @@ -89,6 +90,7 @@ export class RosBagHandler implements FileHandler {
} catch (error: unknown) {
logger.error(`RosBag Extraction failed: ${String(error)}`);
primaryFile.state = FileState.CORRUPTED;
primaryFile.stateComment = String(error);
await this.fileRepo.save(primaryFile);
throw error;
}
Expand Down Expand Up @@ -162,6 +164,7 @@ export class RosBagHandler implements FileHandler {
// Update Primary Bag File (inherit date from conversion result)
primaryFile.date = savedMcapEntity.date;
primaryFile.state = FileState.OK;
primaryFile.stateComment = null;
await this.fileRepo.save(primaryFile);

// Cleanup local converted file
Expand Down Expand Up @@ -195,6 +198,7 @@ export class RosBagHandler implements FileHandler {
);
} catch (error: unknown) {
savedMcapEntity.state = FileState.CONVERSION_ERROR;
savedMcapEntity.stateComment = String(error);
await this.fileRepo.save(savedMcapEntity);

// Ensure cleanup on failure
Expand Down
1 change: 1 addition & 0 deletions queueConsumer/src/file-processor/handlers/db3.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class Db3Handler implements FileHandler {
);
} catch (error) {
primaryFile.state = FileState.CORRUPTED;
primaryFile.stateComment = String(error);
await this.fileRepo.save(primaryFile);
throw error;
}
Expand Down
1 change: 1 addition & 0 deletions queueConsumer/src/file-processor/handlers/mcap.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class McapHandler implements FileHandler {
);
} catch (error) {
primaryFile.state = FileState.CORRUPTED;
primaryFile.stateComment = String(error);
await this.fileRepo.save(primaryFile);
throw error;
}
Expand Down
Loading
Loading