Skip to content

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

Open
LevinCeglie wants to merge 3 commits into
devfrom
feat/improve_file_state_diagnostic
Open

Feat: Improve Error Message on Upload for Corrupted Files#2335
LevinCeglie wants to merge 3 commits into
devfrom
feat/improve_file_state_diagnostic

Conversation

@LevinCeglie

@LevinCeglie LevinCeglie commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

requires DB migration

@LevinCeglie LevinCeglie linked an issue Jul 8, 2026 that may be closed by this pull request
@LevinCeglie
LevinCeglie marked this pull request as ready for review July 8, 2026 14:21
@LevinCeglie
LevinCeglie requested a review from wp99cp July 8, 2026 14:21
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds user-facing diagnostics for failed file uploads. The main changes are:

  • A new nullable stateComment field on files.
  • API, CLI, and frontend display support for the new field.
  • More detailed magic-number validation errors.
  • Queue-consumer updates that set or clear comments during file processing.

Confidence Score: 4/5

The changed flow is close, but raw queue-consumer diagnostics are now visible through file metadata.

  • The new field is wired through persistence, DTO serialization, CLI parsing, and frontend display.
  • The validator return-shape change appears consistently updated at its known caller.
  • The public DTO can now return unsanitized exception text from file processing failures.

packages/api-dto/src/types/file/file.dto.ts and the queue-consumer error writers that assign stateComment.

Security Review

Raw processing errors are now exposed through file metadata. These messages should be sanitized before they are returned to clients.

Important Files Changed

Filename Overview
packages/api-dto/src/types/file/file.dto.ts Adds the exposed stateComment field; propagation works, but raw diagnostics can now reach clients.
packages/backend-common/src/entities/file/file.entity.ts Adds a nullable text column mapping that matches the migration and optional DTO field.
backend/migration/migrations/1783504170192-AddStateCommentToFileEntity.ts Adds and removes the nullable stateComment column on file_entity.
queueConsumer/src/file-processor/helper/magic-number.validator.ts Changes validation to return both validity and a diagnostic message across all checked branches.
queueConsumer/src/file-processor/file-ingestion.service.ts Consumes the new validation result and stores detailed corruption comments.
queueConsumer/src/file-processor/handlers/abstract-metadata.service.ts Clears comments on successful metadata extraction and stores failure text on conversion errors.
queueConsumer/src/file-processor/handlers/bag.hander.ts Adds comments for bag conversion and extraction failures and clears them after successful conversion.
queueConsumer/src/file-processor/handlers/db3.handler.ts Stores DB3 extraction failures in stateComment when the file is marked corrupted.
queueConsumer/src/file-processor/handlers/mcap.handler.ts Stores MCAP extraction failures in stateComment when the file is marked corrupted.
frontend/src/services/generic.ts Extends file-state tooltips to append the optional state comment.
cli/kleinkram/api/deser.py Parses the optional API stateComment field into the CLI file model.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/api-dto/src/types/file/file.dto.ts:79-86
**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.

Reviews (1): Last reviewed commit: "feat(cli/sdk): display file stateComment..." | Re-trigger Greptile

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

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!

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

LevinCeglie added a commit that referenced this pull request Jul 17, 2026
LevinCeglie added a commit that referenced this pull request Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat: Improve Error Message on Upload for Corrupted Files

1 participant