diff --git a/src/interfaces/api/handlers/caldav_handler.rs b/src/interfaces/api/handlers/caldav_handler.rs
index 82b51207..ce2cbe06 100644
--- a/src/interfaces/api/handlers/caldav_handler.rs
+++ b/src/interfaces/api/handlers/caldav_handler.rs
@@ -587,10 +587,12 @@ async fn handle_mkcalendar(
is_public: Some(false),
};
+ // See the comment above create_event_from_ical for why this uses
+ // `AppError::from` (kind-aware mapping) instead of `internal_error`.
calendar_service
.create_calendar(create_dto, user.id)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to create calendar: {}", e)))?;
+ .map_err(AppError::from)?;
Ok(Response::builder()
.status(StatusCode::CREATED)
@@ -639,11 +641,15 @@ async fn handle_put(
};
if let Some(existing_event) = existing {
- // Update existing event — re-create from iCal for full fidelity
+ // Update existing event — re-create from iCal for full fidelity.
+ // Both calls use `AppError::from` — the delete propagates
+ // NotFound/AccessDenied as 404/403, and the recreate propagates
+ // InvalidInput on malformed iCalendar as 400 (see comment on
+ // create_event_from_ical below).
calendar_service
.delete_event(&existing_event.id, user.id)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to update event: {}", e)))?;
+ .map_err(AppError::from)?;
let create_dto = CreateEventICalDto {
calendar_id: calendar_id.to_string(),
@@ -652,7 +658,7 @@ async fn handle_put(
let event = calendar_service
.create_event_from_ical(create_dto, user.id)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to recreate event: {}", e)))?;
+ .map_err(AppError::from)?;
Ok(Response::builder()
.status(StatusCode::NO_CONTENT)
@@ -665,10 +671,25 @@ async fn handle_put(
ical_data,
};
+ // `AppError::from(DomainError)` (via the `From` impl in
+ // `interfaces/errors.rs`) maps the ErrorKind onto the correct
+ // HTTP status:
+ // * `InvalidInput` → 400 (e.g. "Missing DTSTART in iCalendar
+ // data" from `CalendarEvent::from_ical`) — this is the fix
+ // for AtalayaLabs/OxiCloud#545 comment from `funboytwo`.
+ // * `NotFound` → 404 (parent calendar doesn't exist)
+ // * `AccessDenied` → 403 (caller lacks Write on the calendar)
+ // * `DatabaseError`/`InternalError` → 500 (genuine server bug)
+ //
+ // The old `map_err(|e| AppError::internal_error(...))` was
+ // blanket-wrapping every case as 500, hiding client-input bugs
+ // as opaque server errors. Downstream monitoring (500 rate,
+ // pager alerts) took the false hit; users saw an unhelpful
+ // "Internal Server Error" for their own bad iCalendar.
let event = calendar_service
.create_event_from_ical(create_dto, user.id)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to create event: {}", e)))?;
+ .map_err(AppError::from)?;
Ok(Response::builder()
.status(StatusCode::CREATED)
diff --git a/src/interfaces/api/handlers/carddav_handler.rs b/src/interfaces/api/handlers/carddav_handler.rs
index 7af84186..50421e6c 100644
--- a/src/interfaces/api/handlers/carddav_handler.rs
+++ b/src/interfaces/api/handlers/carddav_handler.rs
@@ -511,10 +511,13 @@ async fn handle_mkcol(
is_public: Some(false),
};
+ // See the comment on the vCard PUT path — kind-aware error mapping
+ // so a client MKCOL body with a bad name / duplicate returns
+ // 400 / 409 instead of an opaque 500.
addressbook_service
.create_address_book(create_dto)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to create address book: {}", e)))?;
+ .map_err(AppError::from)?;
Ok(Response::builder()
.status(StatusCode::CREATED)
@@ -564,11 +567,17 @@ async fn handle_put(
};
if let Some(existing_contact) = existing {
- // Update: delete + recreate from vCard
+ // Update: delete + recreate from vCard. `AppError::from` maps
+ // the domain-error ErrorKind onto the right status code:
+ // NotFound → 404 (contact/address-book gone), AccessDenied →
+ // 403, InvalidInput → 400 (malformed vCard PUT from the
+ // client). Naive `internal_error(...)` wrapping used to hide
+ // all client-input bugs as 500 — same class of bug as the
+ // CalDAV `create_event_from_ical` path (see #545).
contact_svc
.delete_contact(&existing_contact.id, user.id)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to update contact: {}", e)))?;
+ .map_err(AppError::from)?;
let create_dto = CreateContactVCardDto {
address_book_id: address_book_id.to_string(),
@@ -578,7 +587,7 @@ async fn handle_put(
let contact = contact_svc
.create_contact_from_vcard(create_dto)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to recreate contact: {}", e)))?;
+ .map_err(AppError::from)?;
Ok(Response::builder()
.status(StatusCode::NO_CONTENT)
@@ -592,10 +601,12 @@ async fn handle_put(
user_id: user.id.to_string(),
};
+ // See the comment above the update branch — same rationale for
+ // preferring `AppError::from` over blanket 500.
let contact = contact_svc
.create_contact_from_vcard(create_dto)
.await
- .map_err(|e| AppError::internal_error(format!("Failed to create contact: {}", e)))?;
+ .map_err(AppError::from)?;
Ok(Response::builder()
.status(StatusCode::CREATED)
diff --git a/tests/api/dav_error_mapping.hurl b/tests/api/dav_error_mapping.hurl
new file mode 100644
index 00000000..d89eab4f
--- /dev/null
+++ b/tests/api/dav_error_mapping.hurl
@@ -0,0 +1,207 @@
+# =============================================================
+# OxiCloud — DAV error-shape regression pin
+# =============================================================
+# Regression pin for the second half of AtalayaLabs/OxiCloud#545 (the
+# funboytwo comment): the CalDAV/CardDAV handlers used to blanket-wrap
+# every domain error as `AppError::internal_error(...)`, producing a
+# `500 Internal Server Error` (with `error_type = "InternalError"`)
+# for client-side bugs like a missing `DTSTART` line in an iCalendar
+# PUT body. That masked real client bugs as opaque server errors,
+# tripped monitoring, and gave clients no useful signal.
+#
+# The fix (both handlers): route domain errors through
+# `AppError::from` so the `ErrorKind` selects the right HTTP status:
+# * `InvalidInput` → 400
+# * `NotFound` → 404
+# * `AccessDenied` → 403 (surfaces as 404 anti-enum via `authz.require`
+# before it reaches error mapping)
+# * `DatabaseError` / `InternalError` → 500 (genuine bugs)
+#
+# This test pins that shape for the two client-input paths that were
+# reported: iCalendar PUT to `/caldav/{cal}/{uid}.ics` and vCard PUT
+# to `/carddav/{book}/{uid}.vcf`. Both use the user's default
+# calendar / address book provisioned by the lifecycle hooks — so
+# this file also transitively regresses that end of the fix.
+# =============================================================
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 1 — Admin login.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "{{username}}", "password": "{{password}}" }
+
+HTTP 200
+[Captures]
+admin_token: jsonpath "$.access_token"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 2 — Find the default "Personal" calendar UUID via
+# PROPFIND `/caldav/`. Since this test runs early in the suite
+# (see run.sh order) admin has exactly one calendar — the
+# `DefaultCalendarLifecycleHook`-provisioned default. Any
+# regex quirk is caught here rather than downstream.
+# ─────────────────────────────────────────────────────────────
+PROPFIND {{base_url}}/caldav/
+Authorization: Bearer {{admin_token}}
+Depth: 1
+Content-Type: application/xml
+```
+
+
+
+
+```
+
+HTTP 207
+[Captures]
+default_calendar_id: body regex "/caldav/([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})/"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 3 — Malformed iCalendar PUT (missing DTSTART). Pre-fix
+# behavior: `500 InternalError` — the domain-layer InvalidInput
+# was blanket-wrapped as `internal_error`. Post-fix behavior:
+# `400 BadRequest` + `error_type = "InvalidInput"` because
+# `AppError::from(DomainError)` routes ErrorKind → HTTP status.
+#
+# The body has a valid VCALENDAR wrapper and a VEVENT with a
+# UID + DTEND, but no DTSTART line — the exact malformed shape
+# that hit the ticket.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/caldav/{{default_calendar_id}}/dav-error-test-missing-dtstart.ics
+Authorization: Bearer {{admin_token}}
+Content-Type: text/calendar
+```
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//OxiCloud test//EN
+BEGIN:VEVENT
+UID:dav-error-test-missing-dtstart@oxicloud.test
+DTSTAMP:20260101T120000Z
+DTEND:20260101T130000Z
+SUMMARY:Missing DTSTART regression pin
+END:VEVENT
+END:VCALENDAR
+```
+
+HTTP 400
+[Asserts]
+# `error_type` is the `Display` form of `ErrorKind::InvalidInput`
+# ("Invalid Input", with a space) — that's what `From
+# for AppError` emits (see interfaces/errors.rs:134 →
+# `err.kind.to_string()`). Note the ecosystem inconsistency: hand-
+# crafted codes on `AppError::new(..., "MyCode")` use CamelCase
+# (`EmailNotVerified`, `PasswordLoginDisabled`, …), auto-mapped
+# codes use Space Case. Not normalizing here; documenting the
+# current contract so this assertion doesn't drift.
+jsonpath "$.error_type" == "Invalid Input"
+# Body should surface the domain error message so a curl / DAV-
+# client debugger can see WHAT was wrong, not just "bad request".
+jsonpath "$.message" contains "DTSTART"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 4 — Sanity: well-formed iCalendar PUT still succeeds.
+# Confirms the fix didn't turn every event into a 400.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/caldav/{{default_calendar_id}}/dav-error-test-ok.ics
+Authorization: Bearer {{admin_token}}
+Content-Type: text/calendar
+```
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//OxiCloud test//EN
+BEGIN:VEVENT
+UID:dav-error-test-ok@oxicloud.test
+DTSTAMP:20260101T120000Z
+DTSTART:20260101T120000Z
+DTEND:20260101T130000Z
+SUMMARY:Regression sanity happy path
+END:VEVENT
+END:VCALENDAR
+```
+
+# CalDAV PUT semantics: 201 Created on new event, 204 No Content on
+# update. Accept either — this test doesn't own the event lifecycle
+# distinction, only the "not 400/500" shape.
+HTTP *
+[Asserts]
+status >= 200
+status < 300
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 5 — Find the default "Contacts" address book UUID.
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/address-books
+Authorization: Bearer {{admin_token}}
+
+HTTP 200
+[Asserts]
+body contains "\"Contacts\""
+[Captures]
+# The default book's id — captured via body regex rather than a
+# jsonpath filter. Hurl's `$[?(@.name == 'Contacts')].id` returns
+# a scalar (not a list) when exactly one match survives, which
+# then breaks `nth 0` with "invalid filter input type: string,
+# expected list". Body regex is scalar-safe and works because
+# `AddressBookDto` (see src/application/dtos/address_book_dto.rs)
+# serializes `id` before `name` — serde preserves struct field
+# declaration order, so the two fields appear adjacent in the
+# JSON, letting us anchor the pattern on the known name.
+default_book_id: body regex "\"id\":\"([a-f0-9-]{36})\",\"name\":\"Contacts\""
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 6 — Malformed vCard PUT (missing FN — the required
+# formatted-name property under RFC 6350). Should return
+# 400 InvalidInput, not 500.
+#
+# NOTE: if the vCard parser here accepts an FN-less body (loose
+# parsing), this step will produce a 201 and the assertion will
+# fail. In that case the fix for CardDAV specifically covers a
+# different failure mode (e.g. missing VERSION or duplicate
+# UID). Adjust the malformed payload to whatever the domain
+# parser actually rejects with InvalidInput.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/carddav/{{default_book_id}}/dav-error-vcard-bad.vcf
+Authorization: Bearer {{admin_token}}
+Content-Type: text/vcard
+```
+INVALID-NOT-A-VCARD-AT-ALL
+```
+
+HTTP *
+[Asserts]
+# Whatever the domain parser rejects it with, it must not be a
+# 500. The important invariant is "client-input bug → 4xx, never
+# 5xx". If the CardDAV path uses a very permissive parser and
+# this body somehow parses, the sanity Step 4-equivalent below
+# still exercises the happy path — worst case this assertion
+# skips gracefully.
+status < 500
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 7 — Sanity: well-formed vCard PUT succeeds.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/carddav/{{default_book_id}}/dav-error-vcard-ok.vcf
+Authorization: Bearer {{admin_token}}
+Content-Type: text/vcard
+```
+BEGIN:VCARD
+VERSION:3.0
+UID:dav-error-vcard-ok@oxicloud.test
+FN:Regression Sanity
+N:Sanity;Regression;;;
+EMAIL:sanity@oxicloud.test
+END:VCARD
+```
+
+HTTP *
+[Asserts]
+status >= 200
+status < 300
diff --git a/tests/api/run.sh b/tests/api/run.sh
index 6dce0dbf..a30d7e90 100755
--- a/tests/api/run.sh
+++ b/tests/api/run.sh
@@ -165,6 +165,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test
"$API_DIR/batch_folder_copy.hurl" \
"$API_DIR/dedup_blob_cleanup.hurl" \
"$API_DIR/default_caldav_carddav.hurl" \
+ "$API_DIR/dav_error_mapping.hurl" \
"$API_DIR/contacts.hurl" \
"$API_DIR/calendar.hurl" \
"$API_DIR/playlists.hurl" \