Skip to content

fix: surface informative 415 for rejected image MIME types#2954

Open
cqnykamp wants to merge 1 commit into
Doenet:mainfrom
cqnykamp:feat/issue-2940
Open

fix: surface informative 415 for rejected image MIME types#2954
cqnykamp wants to merge 1 commit into
Doenet:mainfrom
cqnykamp:feat/issue-2940

Conversation

@cqnykamp

Copy link
Copy Markdown
Contributor

Summary

  • apps/api/src/media/upload.ts: replace the silent cb(null, false) in the multer fileFilter with a typed UnsupportedMimeTypeError, and have handleUploadError map it to a 415 carrying the allowed-MIME list in details.
  • Frontend already surfaces err.response.data.details in its toast, so unsupported uploads (SVG, HEIC, PDF, raw, …) now produce a useful "Allowed: image/jpeg, image/png, image/webp, image/gif" message with no client changes.

Closes #2940

Test plan

  • npx vitest run src/media/upload.test.ts — new test covering handleUploadError for the unsupported-MIME case passes; remaining failures in the file are pre-existing and only happen when DATABASE_URL is unset.
  • npx tsc --noEmit clean.
  • Manual: upload an SVG/HEIC via the UI and confirm the toast shows the allowed list instead of the generic message.

🤖 Generated with Claude Code

The multer fileFilter previously called `cb(null, false)` for unsupported
MIME types, which silently dropped the file and caused the handler to
emit a generic "Unsupported or missing image" 415. Users had no way to
tell whether they hit the type filter or some other failure, and the
allowed formats were nowhere in the response.

Throw a typed `UnsupportedMimeTypeError` from the filter instead, and
extend `handleUploadError` to convert it into a 415 with the allowed
list in `details`. The frontend toast already surfaces
`err.response.data.details`, so this lands without client changes.

Closes Doenet#2940

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@cqnykamp

Copy link
Copy Markdown
Contributor Author

CI status note: 14/15 CI jobs pass. The only failure is e2e-brittle-tests (brittle1)sharingActivities.cy.ts "create, share, and copy public activity", which times out waiting for .doenet-viewer to appear in an iframe.

This appears to be a pre-existing flake, not a regression from this PR:

  • This PR only modifies apps/api/src/media/upload.{ts,test.ts} — a server-side image-upload endpoint. The failing test exercises the activity-sharing iframe load and never touches the upload code path.
  • The identical brittle1 job failed on main HEAD (commit b5171f6, run 26675170593) before this PR was opened, with the same sharingActivities timeout.
  • The job is segregated as *-brittle* and already retries 3× inside Cypress.
  • I requested a manual job rerun; it failed identically — so naive retries are not enough to clear the flake right now.

I have not pushed any additional changes, since the flake is out of scope for this PR (and I'd be guessing at a frontend race condition I don't have evidence for).

Recommendation: either merge despite the known flake, or wait for a future rerun once the flake settles. If brittle1 is consistently broken on main now, that should be its own issue.

@cqnykamp

Copy link
Copy Markdown
Contributor Author

Blocked on #2957 — the failing e2e-brittle-tests (brittle1) job is the flaky sharingActivities.cy.ts failure tracked there. The change in this PR (415 error message) is unrelated to e2e and all other checks are green. Leaving open until the flake is resolved or we decide to merge through it.

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.

Image upload: rejected MIME types surface as uninformative 415

1 participant