fix(files): missing folder_id is 400, not 500

Uploading without folder_id answered `500 Internal Error: folder_id is
required to determine file owner`. A missing required field is the
caller's error; as an internal_error it produced `error_type: Internal
Error`, which the SPA cannot distinguish from the server breaking — so a
malformed request looked like an outage.

Both sites become validation_error (ErrorKind::InvalidInput → 400), with
messages that say WHY the field is needed rather than restating that it
is: the destination folder determines the file's owner and drive.

The OpenAPI request body described it as "optional folder_id field",
which is how it came to be omitted — hit while writing
thumbnail_etag_content_keyed.hurl, where the upload was written from the
documented contract and 500'd. Now stated as required.

Regression test asserts the status AND that error_type is not "Internal
Error", since the contract the SPA switches on is error_type rather than
the message.
This commit is contained in:
Edouard Vanbelle
2026-08-26 23:07:58 +02:00
parent 4997eb4fb0
commit d7de1c41e7
3 changed files with 37 additions and 7 deletions
@@ -130,9 +130,11 @@ impl FileBlobWriteRepository {
DomainError::internal_error("FileBlobWrite", format!("parent lookup: {e}"))
})?
.ok_or_else(|| DomainError::not_found("Folder", fid)),
None => Err(DomainError::internal_error(
"FileBlobWrite",
"folder_id is required to determine the target drive",
// Same reasoning as the owner lookup below: caller error, not
// server error.
None => Err(DomainError::validation_error(
"folder_id is required: the destination folder determines the \
target drive",
)),
}
}
@@ -289,9 +291,15 @@ impl FileBlobWriteRepository {
rollback_err
);
}
return Err(DomainError::internal_error(
"FileBlobWrite",
"folder_id is required to determine file owner",
// A missing required field is the caller's error, not the
// server's. As `internal_error` this surfaced as 500 /
// `error_type: Internal Error`, which the SPA cannot tell apart
// from the server breaking — so a malformed upload looked like an
// outage. The OpenAPI body description called the field optional,
// which is how it came to be omitted in the first place.
return Err(DomainError::validation_error(
"folder_id is required: the destination folder determines the \
file's owner and drive",
));
};
+1 -1
View File
@@ -1552,7 +1552,7 @@ pub async fn list_files_query(
#[utoipa::path(
post,
path = "/api/files/upload",
request_body(content_type = "multipart/form-data", description = "File data + optional folder_id field"),
request_body(content_type = "multipart/form-data", description = "File data + folder_id (required: it determines the file's owner and drive)"),
responses(
(status = 201, description = "File uploaded", body = FileDto),
(status = 400, description = "Invalid request"),
+22
View File
@@ -357,3 +357,25 @@ Authorization: Bearer {{token}}
HTTP 200
[Asserts]
header "Content-Type" startsWith "image/"
# ─────────────────────────────────────────────────────────────
# Step 23 – Upload without folder_id is a CLIENT error
#
# The destination folder determines the file's owner and drive, so the
# field is required. It used to answer 500 / `error_type: Internal
# Error`, which the SPA cannot distinguish from the server breaking — a
# malformed request looked like an outage. The OpenAPI body description
# called the field optional, which is how it came to be omitted.
#
# Asserts the status AND the error_type, because the contract the SPA
# switches on is `error_type`, not the message.
# ─────────────────────────────────────────────────────────────
POST {{base_url}}/api/files/upload
Authorization: Bearer {{token}}
[MultipartFormData]
file: file,fixtures/hello.txt; text/plain
HTTP 400
[Asserts]
jsonpath "$.error_type" != "Internal Error"