fix(server): handle disk-full uploads gracefully#3133
Merged
Conversation
OSError ENOSPC during an upload (Django's multipart temp spool or the copy into assetdir) surfaced as an unhandled 500 + Sentry error (ANTHIAS-3K), telling the operator nothing. - HTML upload: catch ENOSPC at both write points, show an actionable disk-full toast, clean up the partial file - file_asset API: same guards, responding 507 Insufficient Storage - Shared DISK_FULL_ERROR message + is_disk_full helper so the wording can't drift between surfaces Fixes #3127 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves robustness of asset uploads by gracefully handling disk-full (ENOSPC) conditions in both the HTML upload endpoint and the v1 file_asset API, replacing unhandled 500s with operator-actionable feedback.
Changes:
- Catch
ENOSPCduring the lazy multipart parse (request.FILES/request.data) and return a user-facing toast (HTML) or507 Insufficient Storage(API). - Catch
ENOSPCduring the subsequent write into the asset directory / temp upload file and clean up partial files before responding. - Add shared
DISK_FULL_ERRORmessage +is_disk_full()helper, and add tests covering both surfaces.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_template_views.py | Adds a regression test ensuring HTML upload shows a disk-full toast instead of 500. |
| src/anthias_server/app/views.py | Handles ENOSPC during multipart parse and file copy; cleans up partial asset files. |
| src/anthias_server/api/views/mixins.py | Handles ENOSPC during multipart parse and temp-file write; returns 507 and cleans up partial .tmp. |
| src/anthias_server/api/tests/test_v1_endpoints.py | Adds a regression test ensuring API returns 507 with the shared disk-full message. |
| src/anthias_common/utils.py | Introduces shared disk-full message constant and is_disk_full() helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot review: the Content-Range branch opened the .tmp in append mode, where seek() is ignored and every write pins to EOF — an out-of-order chunk would corrupt the file. Open r+b (wb for the first chunk) so the offset is honoured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot review: the reported ANTHIAS-3K stack is ENOSPC while Django spools the request body during multipart parsing (surfaced at request.FILES / request.data access), which the existing tests didn't exercise — they only patched the later copy-into-assetdir write. Force MultiPartParser.parse to raise and assert the toast/507. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot review: the earlier tests fail at open(), so the cleanup branch (remove the partially-written file when f.write() raises ENOSPC after the file exists) was never exercised. Add tests that let open() succeed, raise ENOSPC on write, and assert remove() runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot review: the resumable-upload branch parsed Content-Range with split()/int(), so a malformed client header raised ValueError/ IndexError and surfaced as a 500. Parse it with a strict regex and return a 400 ValidationError instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot review: the resumable upload path validated header syntax but ignored the numeric semantics. The .tmp path is deterministic per filename, so a shorter new upload could inherit trailing bytes from a longer previous attempt and rename() a corrupted asset into place. - Reject end<start, end>=total, and chunk length != end-start+1 (400) - On the final chunk, truncate to the declared total so stale trailing bytes can't survive — order-independent, unlike a start==0 truncate which would clobber out-of-order chunks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot review: a * (unknown) total left total_bytes unset, so the final-chunk truncation that drops stale trailing bytes never ran — reopening the corruption window it exists to close. Our uploader always knows the file size, so require a numeric total and 400 on *. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This was referenced Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Description
Uploading to a device with a full disk died with
OSError: [Errno 28] No space left on deviceinside Django's multipart parser — an unhandled 500 and a Sentry error, with the operator none the wiser.The ENOSPC surfaces at two points, both now guarded on both upload surfaces (HTML
assets/upload/and thefile_assetAPI):The HTML path returns the normal table partial with an actionable toast ("The device disk is full. Free up space — for example by deleting unused assets — and try again."); the API returns 507 Insufficient Storage with the same shared message. Any non-ENOSPC
OSErrorstill propagates.This PR also fixes a pre-existing bug the review surfaced: the resumable (
Content-Range) branch opened the.tmpin append mode, whereseek()is ignored and every write pins to EOF — an out-of-order chunk would corrupt the file. It now opensr+b(wbfor the first chunk) so the offset is honoured.Validation on real hardware
Exercised in the real server container / over real HTTP on the x86 testbed:
bytes 4-7) before the head (bytes 0-3) through the livefile_assetendpoint; the reassembled file read backAAAABBBB(append mode would have corrupted it). This confirms ther+bfix end-to-end.is_disk_full(OSError(ENOSPC))→True,EACCES→False, in the real container; the shared operator message rendered correctly.Physically filling the device disk was deliberately avoided (a full-disk state risks wedging a testbed); the ENOSPC response paths themselves are covered by unit tests at both the parse stage and the mid-write cleanup stage. Server-side and arch-independent.
Checklist
🤖 Generated with Claude Code