diff --git a/DESIGN.md b/DESIGN.md index f559b889..fd7995d7 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1404,9 +1404,12 @@ Besprechung sagte sie dem Gast ab. Jetzt: folgenden“ wird nicht angeboten: Das schriebe die Regel um, die nur der Organisator ändern darf. - Eine Ablehnung reist als Marke (`cal_core::WriteRefusal`: - `reply-only-invitation:`, `server-refused:`, `identity-unknown:`), und - beide Oberflächen sagen daraus einen Satz in der Sprache des Nutzers - (`shared/eventWriteError.ts`). `CACHE_GENERATION` 4, damit eine + `reply-only-invitation:`, `server-refused:`, `identity-unknown:`, + `occurrence-not-writable:`, `unsafe-to-write:`), und beide Oberflächen + sagen daraus einen Satz in der Sprache des Nutzers + (`shared/eventWriteError.ts`). Eine Marke heißt auch: Es wurde sicher + nichts geschrieben (`writeNeverLanded`); beim Ändern eines Termins melden + alle Adapter eine Ablehnung des Servers so (147). `CACHE_GENERATION` 4, damit eine gespeicherte Kalenderliste ohne das neue Merkmal neu gelesen wird. **Free/Busy-Abfrage (implementiert).** Im Termin-Dialog prüft „Verfügbarkeit diff --git a/TODO.md b/TODO.md index 53385b1b..9cae6be9 100644 --- a/TODO.md +++ b/TODO.md @@ -2409,22 +2409,41 @@ Siehe DESIGN §4.2. „weg oder nie hier“, die Suche über die Kalender geht weiter und zählt es nur als gelöscht, wenn kein anderer Kalender den Termin hat (`DeleteWalk`). Nach einem gescheiterten Verbindungsaufbau gilt nichts - davon: da ging nichts hinaus. + davon: da ging nichts hinaus. Am Handy kommen nur `forbidden`, + `conflict` und `network` mit Code an (B9); eine Ablehnung mit Marke + (`server-refused`, `unsafe-to-write`) reist aber als `forbidden` und + wird erkannt, jede andere gilt dort vorsichtig als unklar. Handy ohne + Testläufer — ↻ im Test. - 🚩 CalDAV `create_task_list`: MKCALENDAR läuft über `send_retrying`. Kam der erste Versuch an und brach dann die Verbindung ab, antwortet die Wiederholung 405 (die Liste gibt es schon): gemeldet als Fehler, ein erneuter Versuch legt eine zweite Liste an. Vor #97 schon so. - - 🚩 **Sichere Ablehnungen als Ablehnung kennzeichnen** (147, eigener PR - direkt nach #97): EWS-Fehlerantworten, HTTP 400/429 bei Google und - Microsoft, CalDAV-Prüfungen „nothing was saved“ kommen als `protocol` an; - beim Teilen bleiben dann beide Serien mit Warnung stehen, obwohl sicher - nichts gekürzt wurde. Die Adapter sollen sie mit `server-refused` - kennzeichnen. Dazu Graph: nach erfolgreichem `/cancel` liefert das - folgende DELETE womöglich 404 (die Absage verschiebt das Ereignis), das - Löschen gilt dann als gescheitert. Am Handy kommen - nur `forbidden`, `conflict` und `network` mit Code an (B9), jede andere - Ablehnung gilt dort also vorsichtig als unklar. Handy ohne Testläufer — - ↻ im Test. + - ✅ **Sichere Ablehnungen als Ablehnung gekennzeichnet** (147). EWS- + Fehlerantworten, HTTP 400/429 bei Google und Microsoft und CalDAVs eigene + Prüfungen „nothing was saved“ kamen als `protocol` an; beim Teilen + blieben dann beide Serien mit Warnung stehen, obwohl sicher nichts + gekürzt wurde. Jetzt meldet jeder Adapter beim Ändern eines Termins + (`update_event`) eine Ablehnung als solche: ein HTTP-Status, mit dem der + Server das Schreiben ganz abgelehnt hat + (`WriteRefusal::refused_status`: 400, 413, 415, 422, 429, 507), samt dem + Grund, den Google und Microsoft nennen, und bei EWS die bekannten Prüf- + und Drosselungsfehler (`REFUSED_UPDATE_CODES`, auch als SOAP-Fault mit + HTTP 500), als `server-refused`; jeder andere EWS-Code bleibt unklar, + weil eine Besprechung erst gespeichert und dann verschickt wird; die + Konfliktcodes auch, denn Aperio schreibt mit `AlwaysOverwrite`. CalDAVs + eigene Prüfungen kommen als neue Marke `unsafe-to-write` („Aperio kann + diesen Termin nicht sicher ändern“). `server-refused` und + `unsafe-to-write` reisen als `forbidden` und kommen so auch am Handy an. + Antwortet der Server auf eine CalDAV-Wiederholung mit einem Fehler, + bleibt es unklar (`network`, die Antwort steht im Protokoll): der erste + Versuch kann angekommen sein. Scheitert bei Google oder Microsoft die + Erneuerung des Tokens (400/401 am Token-Endpunkt), ist es ein + Anmeldefehler (`auth`), keine Ablehnung des Kalenderservers; am Handy + kommt er ohne Code an und bleibt dort unklar. Graph: ein DELETE, das nach erfolgreichem `/cancel` + nichts mehr findet, gilt als erledigt (die Absage verschiebt das + Ereignis nach „Gelöschte Elemente“, mit neuer Id). Nicht hier: Anlegen + und Löschen melden solche Ablehnungen weiter als `protocol` (für das + Teilen zählt nur das Kürzen). - 🚩 **Weckerberechnung bei einem Ende vor dem Beginn** (124: jetzt nicht). `expand_on` nimmt bei einer Regel, die vor ihrem Beginn endet, weiter den Serienbeginn. Aperio schreibt solche Regeln nicht mehr, aber ein Enddatum, diff --git a/crates/adapter-caldav/src/events.rs b/crates/adapter-caldav/src/events.rs index c9f9dc3d..523c9e30 100644 --- a/crates/adapter-caldav/src/events.rs +++ b/crates/adapter-caldav/src/events.rs @@ -331,20 +331,32 @@ async fn update_master( .map_err(|e| CaldavError::Config(format!("event.calendar_id is not a URL: {e}")))?; let resource = resource_url_for_event(&cal_url, &event.id)?; let (body, server_etag) = get_resource(client, &resource, credentials).await?; - let refused = - |why: &str| CaldavError::Protocol(format!("{}: {why}; nothing was saved", event.id)); + // Nothing is written, and the caller must know it for certain: splitting a + // series takes its new part back only after a refusal like this one + // (decision 144), so it travels as one, not as a protocol error. + let refused = |token: &str, why: &str| { + tracing::warn!(event_id = %event.id, why, "refusing to write the event; nothing was saved"); + CaldavError::Forbidden(WriteRefusal::UnsafeToWrite.message(token)) + }; let own = ctx.identity.clone().unwrap_or_default(); - let blocks = vevent_blocks(&body, cal_url.as_str(), &own) - .ok_or_else(|| refused("its resource cannot be read block by block"))?; + let blocks = vevent_blocks(&body, cal_url.as_str(), &own).ok_or_else(|| { + refused( + "unreadable-blocks", + "its resource cannot be read block by block", + ) + })?; let (_, uid) = decode_event_id(&event.id); let master = blocks .iter() .find(|b| { override_recurrence_id(&b.event.id).is_none() && decode_event_id(&b.event.id).1 == uid }) - .ok_or_else(|| refused("its resource holds no such event"))?; + .ok_or_else(|| refused("no-such-event", "its resource holds no such event"))?; if !one_organizer(&body, &blocks) { - return Err(refused("its components name different organizers")); + return Err(refused( + "mixed-organizers", + "its components name different organizers", + )); } if ctx.schedules && attendee_copy(&body[master.range.clone()], ctx)? { return write_attendee_copy( @@ -381,7 +393,12 @@ async fn update_master( |rid| cutoff.is_none_or(|until| rid <= until), |block| apply_to_block(block, &plan.change), ) - .ok_or_else(|| refused("its resource cannot be read block by block"))?; + .ok_or_else(|| { + refused( + "unreadable-blocks", + "its resource cannot be read block by block", + ) + })?; let if_match = event.etag.as_deref().or(server_etag.as_deref()); let new_etag = put_resource(client, &resource, new_body, if_match, credentials).await?; @@ -793,18 +810,27 @@ async fn put_resource( .body(body) .send_retrying_marked() .await?; - // A 412 on the replay of a guarded write: the connection died after the - // first PUT went out, and that one may have landed — its new ETag is what - // refuses the replay. Read as a refusal, a caller would undo around a write - // that went through: splitting a series deleted its new part while the old - // part was already cut short (decision 144). So it is what it is, unsure. - if first_may_have_landed - && if_match.is_some() - && response.status() == StatusCode::PRECONDITION_FAILED - { + // Any refusal of a replay whose first attempt may have landed: the + // connection died after the first PUT went out, and the answer may be + // about that one — a 412 because its new ETag refuses the replay, a 429 or + // a 507 because it was taken. Read as a refusal, a caller would undo + // around a write that went through: splitting a series deleted its new + // part while the old part was already cut short (decision 144). So it is + // what it is, unsure. + if first_may_have_landed && !response.status().is_success() { + // The answer itself is kept for the log: it may be the first + // attempt's, or a refusal the replay met on its own. + let status = response.status().as_u16(); + let body = response.text().await.unwrap_or_default(); + tracing::warn!( + %resource, + status, + body = %body.chars().take(200).collect::(), + "a replayed PUT was answered with a failure; the first attempt may have been saved", + ); return Err(CaldavError::Network(format!( - "the connection to '{resource}' broke after the change was sent; \ - it may have been saved" + "the connection to '{resource}' broke after the change was sent \ + (the replay was answered HTTP {status}); it may have been saved" ))); } check_write(response).await @@ -1852,6 +1878,30 @@ END:VCALENDAR assert!(matches!(err, CaldavError::Network(_)), "{err:?}"); } + /// ...and so is any other refusal of that replay: a 429 may be the server + /// throttling a second copy of what it already stored. + #[tokio::test] + async fn any_refusal_of_a_replayed_put_is_unsure() { + let base = + first_attempt_lost_then(b"HTTP/1.1 429 Too Many Requests\r\ncontent-length: 0\r\n\r\n") + .await; + let resource = Url::parse(&format!("{base}/calendars/alice/work/abc.ics")).unwrap(); + let err = put_resource( + &client(), + &resource, + standup_body("Cut short"), + None, + &creds(&base), + ) + .await + .expect_err("the replay was refused"); + match err { + // The replay's answer is kept, for whoever reads the error. + CaldavError::Network(msg) => assert!(msg.contains("HTTP 429"), "{msg}"), + other => panic!("expected unsure, got {other:?}"), + } + } + /// Without a replay, a 412 is what it says: the copy moved on. #[tokio::test] async fn a_412_without_a_replay_is_a_conflict() { @@ -2206,6 +2256,44 @@ END:VCALENDAR\r } } + /// A resource whose blocks name different organizers is not written: a + /// refusal of Aperio's own, said as one (decision 144), not a protocol + /// error the caller has to treat as maybe written. + #[tokio::test] + async fn an_update_aperio_will_not_write_is_a_refusal() { + let mut server = Server::new_async().await; + let body = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\n\ +BEGIN:VEVENT\r\nUID:abc-123@aperio\r\nDTSTAMP:20260520T060000Z\r\nSUMMARY:Standup\r\n\ +DTSTART:20260520T080000Z\r\nDTEND:20260520T083000Z\r\nRRULE:FREQ=DAILY\r\n\ +ORGANIZER:mailto:alice@example.org\r\nEND:VEVENT\r\n\ +BEGIN:VEVENT\r\nUID:abc-123@aperio\r\nDTSTAMP:20260520T060000Z\r\nSUMMARY:Standup\r\n\ +RECURRENCE-ID:20260521T080000Z\r\nDTSTART:20260521T090000Z\r\nDTEND:20260521T093000Z\r\n\ +ORGANIZER:mailto:bob@example.org\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n"; + let _get = serve_copy(&mut server, body.to_string()).await; + let put = server + .mock( + "PUT", + mockito::Matcher::Regex(r"^/calendars/alice/work/.+\.ics$".into()), + ) + .expect(0) + .create_async() + .await; + let cal_url = Url::parse(&format!("{}/calendars/alice/work/", server.url())).unwrap(); + let err = update_event( + &client(), + sample_existing_event(&cal_url), + &creds(&server.url()), + &WriteCtx::default(), + ) + .await + .unwrap_err(); + match err { + CaldavError::Forbidden(msg) => assert_eq!(msg, "unsafe-to-write: mixed-organizers"), + other => panic!("expected a refusal, got {other:?}"), + } + put.assert_async().await; + } + #[tokio::test] async fn update_event_412_surfaces_as_conflict() { let mut server = Server::new_async().await; diff --git a/crates/adapter-caldav/src/lib.rs b/crates/adapter-caldav/src/lib.rs index c443c9f2..7318cf38 100644 --- a/crates/adapter-caldav/src/lib.rs +++ b/crates/adapter-caldav/src/lib.rs @@ -926,6 +926,23 @@ impl CaldavAdapter { } } +/// [`to_core_error`] for an update. A status with which the server turned the +/// write down whole ([`cal_core::WriteRefusal::refused_status`]) is a refusal, +/// not a protocol error: a caller deciding whether the write may have landed — +/// splitting a series undoes its new part only when the cut certainly did not +/// (decision 144) — must be told that nothing was written. +fn to_update_error(err: CaldavError) -> CoreError { + match err { + CaldavError::Http { status, message } if cal_core::WriteRefusal::refused_status(status) => { + tracing::warn!(status, %message, "the server refused the update"); + CoreError::Forbidden( + cal_core::WriteRefusal::ServerRefused.message(&format!("HTTP {status}")), + ) + } + other => to_core_error(other), + } +} + /// Translate a CalDAV-specific error into the shared `cal_core::Error` /// shape so the rest of the app can pattern-match it uniformly. fn to_core_error(err: CaldavError) -> CoreError { @@ -1091,7 +1108,7 @@ impl CalendarFeature for CaldavAdapter { let ctx = self.write_ctx().await?; events::update_event(&self.http, event, &self.credentials, &ctx) .await - .map_err(to_core_error) + .map_err(to_update_error) } async fn add_event_exdate( @@ -1693,6 +1710,39 @@ fn contact_matches(c: &Contact, needle_lower: &str) -> bool { .any(|e| e.value.to_lowercase().contains(needle_lower)) } +#[cfg(test)] +mod update_refusal_tests { + use super::*; + + /// A status with which the server turned the update down whole is a + /// refusal (decision 144); a server failure and the named statuses keep + /// their error. + #[test] + fn an_update_the_server_turned_down_is_a_refusal() { + for status in [400, 413, 415, 422, 429, 507] { + match to_update_error(CaldavError::Http { + status, + message: "no".into(), + }) { + CoreError::Forbidden(msg) => { + assert_eq!(msg, format!("server-refused: HTTP {status}")) + } + other => panic!("{status}: {other:?}"), + } + } + let http = |status| CaldavError::Http { + status, + message: "no".into(), + }; + assert!(matches!(to_update_error(http(500)), CoreError::Protocol(_))); + assert!(matches!(to_update_error(http(412)), CoreError::Conflict(_))); + assert!(matches!( + to_update_error(CaldavError::Network("reset".into())), + CoreError::Network(_) + )); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/adapter-ews/src/lib.rs b/crates/adapter-ews/src/lib.rs index 68935dd5..9c6ad73c 100644 --- a/crates/adapter-ews/src/lib.rs +++ b/crates/adapter-ews/src/lib.rs @@ -1000,7 +1000,7 @@ impl CalendarFeature for EwsAdapter { }; api::update_event(&self.client, &event, zones.as_ref()) .await - .map_err(to_core_error) + .map_err(to_update_error) } async fn delete_event(&self, event_id: &str, send_cancellations: bool) -> CoreResult<()> { @@ -1384,6 +1384,75 @@ impl ContactsFeature for EwsAdapter { } } +/// [`to_core_error`] for an update. A status with which the server turned the +/// write down whole ([`cal_core::WriteRefusal::refused_status`]) is a refusal, +/// not a protocol error: a caller deciding whether the write may have landed — +/// splitting a series undoes its new part only when the cut certainly did not +/// (decision 144) — must be told that nothing was written. +/// +/// Only the SOAP codes with which Exchange turns an `UpdateItem` down while it +/// checks or throttles the request count ([`REFUSED_UPDATE_CODES`]): a meeting +/// update may save the item and then fail while sending it (send-as denied, a +/// quota, the store going away), so any other code says nothing either way. +/// Such a code is read from a SOAP fault sent with HTTP 500 too — the usual +/// shape of `ErrorServerBusy`. The codes [`to_core_error`] already names +/// (sign-in, not found) keep their error. +fn to_update_error(err: EwsError) -> CoreError { + match err { + EwsError::Http { status, message } if cal_core::WriteRefusal::refused_status(status) => { + tracing::warn!(status, %message, "Exchange refused the update"); + CoreError::Forbidden( + cal_core::WriteRefusal::ServerRefused.message(&format!("HTTP {status}")), + ) + } + EwsError::Http { + status: 500, + message, + } => match soap::check_for_fault(&message) { + Err(EwsError::Soap { code, message: why }) if refused_update_code(&code).is_some() => { + let code = refused_update_code(&code).unwrap_or_default(); + tracing::warn!(%code, %why, "Exchange refused the update"); + CoreError::Forbidden(cal_core::WriteRefusal::ServerRefused.message(code)) + } + _ => to_core_error(EwsError::Http { + status: 500, + message, + }), + }, + EwsError::Soap { code, message } if refused_update_code(&code).is_some() => { + let code = refused_update_code(&code).unwrap_or_default(); + tracing::warn!(%code, %message, "Exchange refused the update"); + CoreError::Forbidden(cal_core::WriteRefusal::ServerRefused.message(code)) + } + other => to_core_error(other), + } +} + +/// The SOAP codes with which Exchange turns an `UpdateItem` down before it +/// saves anything: the request does not validate, or the server throttles. +/// See [`to_update_error`]. The update is sent with `AlwaysOverwrite`, so no +/// ChangeKey is checked before the save, and a conflict code +/// (`ErrorIrresolvableConflict`, `ErrorStaleObject`) comes from somewhere +/// else: it is not on this list and stays unsure. +const REFUSED_UPDATE_CODES: &[&str] = &[ + "ErrorServerBusy", + "ErrorInvalidRequest", + "ErrorSchemaValidation", + "ErrorInvalidPropertySet", + "ErrorInvalidPropertyDelete", + "ErrorInvalidPropertyUpdateSentMessage", + "ErrorCalendarInvalidRecurrence", + "ErrorInvalidIdMalformed", + "ErrorInvalidChangeKey", +]; + +/// The code, without the namespace prefix a fault's `faultcode` carries +/// (`a:ErrorServerBusy`), when it is one of [`REFUSED_UPDATE_CODES`]. +fn refused_update_code(code: &str) -> Option<&str> { + let bare = code.rsplit(':').next().unwrap_or(code); + REFUSED_UPDATE_CODES.contains(&bare).then_some(bare) +} + fn to_core_error(err: EwsError) -> CoreError { use EwsError::*; match err { @@ -1414,6 +1483,120 @@ fn to_core_error(err: EwsError) -> CoreError { } } +#[cfg(test)] +mod update_refusal_tests { + use super::*; + + fn soap(code: &str) -> EwsError { + EwsError::Soap { + code: code.into(), + message: "no".into(), + } + } + + /// Exchange turned the update down while checking or throttling it: + /// nothing was saved, and it is a refusal (decision 144). + #[test] + fn a_soap_error_answer_to_an_update_is_a_refusal() { + for code in [ + "ErrorServerBusy", + "ErrorInvalidPropertySet", + "ErrorCalendarInvalidRecurrence", + ] { + match to_update_error(soap(code)) { + CoreError::Forbidden(msg) => assert_eq!(msg, format!("server-refused: {code}")), + other => panic!("{code}: {other:?}"), + } + } + match to_update_error(EwsError::Http { + status: 429, + message: "slow down".into(), + }) { + CoreError::Forbidden(msg) => assert_eq!(msg, "server-refused: HTTP 429"), + other => panic!("{other:?}"), + } + } + + /// ...but any other code may come after the item was saved — a meeting + /// update saves, then sends — and stays unsure; a named error keeps its + /// name. + #[test] + fn a_server_failure_stays_unsure_and_a_named_error_keeps_its_name() { + for code in [ + "ErrorInternalServerError", + "ErrorInternalServerTransientError", + "ErrorTimeoutExpired", + "ErrorSendAsDenied", + "ErrorQuotaExceeded", + "ErrorSubmissionQuotaExceeded", + "ErrorMessageSizeExceeded", + "ErrorMailboxStoreUnavailable", + "Unknown", + "s:Server", + "ErrorIrresolvableConflict", + "ErrorStaleObject", + ] { + assert!( + matches!(to_update_error(soap(code)), CoreError::Protocol(_)), + "{code}" + ); + } + assert!(matches!( + to_update_error(soap("ErrorAccessDenied")), + CoreError::Authentication(_) + )); + assert!(matches!( + to_update_error(soap("ErrorItemNotFound")), + CoreError::NotFound(_) + )); + assert!(matches!( + to_update_error(EwsError::Http { + status: 503, + message: "busy".into(), + }), + CoreError::Protocol(_) + )); + } + + fn fault_500(code: &str) -> EwsError { + EwsError::Http { + status: 500, + message: format!( + r#" + + + + a:{code} + The server cannot service this request right now. Try again later. + + +"# + ), + } + } + + /// Exchange throttles with a SOAP fault sent as HTTP 500: read from the + /// fault, it is the refusal it names; any other 500 stays unsure. + #[test] + fn a_throttling_fault_sent_as_500_is_a_refusal() { + match to_update_error(fault_500("ErrorServerBusy")) { + CoreError::Forbidden(msg) => assert_eq!(msg, "server-refused: ErrorServerBusy"), + other => panic!("{other:?}"), + } + assert!(matches!( + to_update_error(fault_500("ErrorInternalServerError")), + CoreError::Protocol(_) + )); + assert!(matches!( + to_update_error(EwsError::Http { + status: 500, + message: "Server Error".into(), + }), + CoreError::Protocol(_) + )); + } +} + #[cfg(test)] mod state_persistence_tests { use super::*; diff --git a/crates/adapter-google/src/api.rs b/crates/adapter-google/src/api.rs index 56fb8859..2f03d4bc 100644 --- a/crates/adapter-google/src/api.rs +++ b/crates/adapter-google/src/api.rs @@ -215,7 +215,7 @@ impl ApiState { .await .map_err(|err| { warn!(?err, "refresh-token grant failed"); - err + refresh_refused(err) })?; let mut guard = self.tokens.lock().await; guard.access_token = fresh.access_token; @@ -229,6 +229,24 @@ impl ApiState { } } +/// A token endpoint that answered the refresh with 400 or 401 — a revoked or +/// expired grant (`invalid_grant`), a client it does not know — refused the +/// sign-in, not the request that needed it: said as a 401, the write that was +/// waiting for the token reads as a sign-in failure. As its own status, an +/// update read it as the calendar server refusing the change (decision 147). +/// Anything else keeps its error. +fn refresh_refused(err: GoogleError) -> GoogleError { + match err { + GoogleError::Http { status, message } if status == 400 || status == 401 => { + GoogleError::Http { + status: 401, + message: format!("token refresh refused (HTTP {status}): {message}"), + } + } + other => other, + } +} + /// The one non-success answer that is not a failure: Google refusing a /// DIRECTORY listing to an account that has none. /// diff --git a/crates/adapter-google/src/lib.rs b/crates/adapter-google/src/lib.rs index fc80e48f..2f22f59a 100644 --- a/crates/adapter-google/src/lib.rs +++ b/crates/adapter-google/src/lib.rs @@ -353,7 +353,7 @@ impl CalendarFeature for GoogleAdapter { async fn update_event(&self, event: Event) -> CoreResult { api::update_event(&self.state, &event) .await - .map_err(to_core_error) + .map_err(to_update_error) } async fn delete_event(&self, event_id: &str, send_cancellations: bool) -> CoreResult<()> { @@ -720,6 +720,64 @@ fn is_read_only_google_list(list_id: &str) -> bool { || list_id == contacts::GOOGLE_DIRECTORY_LIST_ID } +/// [`to_core_error`] for an update. A status with which the server turned the +/// write down whole ([`cal_core::WriteRefusal::refused_status`]) is a refusal, +/// not a protocol error: a caller deciding whether the write may have landed — +/// splitting a series undoes its new part only when the cut certainly did not +/// (decision 144) — must be told that nothing was written. +fn to_update_error(err: GoogleError) -> CoreError { + match err { + GoogleError::Http { status, message } if cal_core::WriteRefusal::refused_status(status) => { + tracing::warn!(status, %message, "Google refused the update"); + CoreError::Forbidden(cal_core::WriteRefusal::ServerRefused.message( + &match server_reason(&message) { + Some(reason) => format!("HTTP {status}: {reason}"), + None => format!("HTTP {status}"), + }, + )) + } + other => to_core_error(other), + } +} + +/// The reason a JSON error answer gives (`{"error":{"message":"…"}}`), +/// trimmed, for the sentence that says the server refused: "HTTP 400" alone +/// tells the user nothing they can act on. +/// +/// The error keeps only the first 300 characters of the answer, so a longer +/// envelope arrives cut and is no JSON any more. The first `"message"` +/// string is then read as far as it goes: it is the error's own, before the +/// details and the request ids that make an envelope long. +fn server_reason(body: &str) -> Option { + let reason = match serde_json::from_str::(body) { + Ok(value) => value.get("error")?.get("message")?.as_str()?.to_string(), + Err(_) => { + let rest = &body[body.find("\"message\"")? + "\"message\"".len()..]; + let rest = rest.trim_start().strip_prefix(':')?.trim_start(); + let rest = rest.strip_prefix('"')?; + let mut out = String::new(); + let mut escaped = false; + for c in rest.chars() { + match (escaped, c) { + (true, _) => { + out.push(c); + escaped = false; + } + (false, '\\') => escaped = true, + (false, '"') => break, + (false, _) => out.push(c), + } + } + out + } + }; + let reason = reason.trim(); + if reason.is_empty() { + return None; + } + Some(reason.chars().take(160).collect()) +} + fn to_core_error(err: GoogleError) -> CoreError { use GoogleError::*; match err { @@ -746,6 +804,148 @@ mod delta_tests { use chrono::TimeZone; use mockito::{Matcher, Server}; + /// A series head, cut short: what splitting a series writes. + fn truncated_master() -> Event { + Event { + keep_attendees: false, + keep_fields: Vec::new(), + clear_attendees: false, + organized_elsewhere: false, + id: "master-1".into(), + calendar_id: "primary".into(), + title: "Teamrunde".into(), + description: None, + location: None, + start: chrono::Utc.with_ymd_and_hms(2026, 6, 1, 7, 0, 0).unwrap(), + end: chrono::Utc.with_ymd_and_hms(2026, 6, 1, 8, 0, 0).unwrap(), + all_day: false, + recurrence: Some(cal_core::EventRecurrence { + rrule: "FREQ=WEEKLY;UNTIL=20260824T065959Z".into(), + exceptions: vec![], + tzid: None, + }), + color_label: None, + color_hex: None, + reminders: vec![], + sound: None, + attendees: vec![], + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + etag: None, + organizer: None, + attendee_responses: vec![], + send_invitations: false, + truncate_tail_overrides: false, + cancelled: false, + scheduling_silenced: false, + } + } + + /// Google read the truncate and turned it down whole: nothing was written, + /// and the caller must know that for certain (decision 144), not as a + /// protocol error it has to treat as maybe landed. + #[tokio::test] + async fn an_update_google_turned_down_is_a_refusal() { + for status in [400, 429] { + let mut server = Server::new_async().await; + let _patch = server + .mock( + "PATCH", + Matcher::Regex(r"^/calendars/primary/events/master-1".into()), + ) + .with_status(status) + .with_body(r#"{"error":{"message":"no"}}"#) + .create_async() + .await; + let err = adapter_for(&server) + .update_event(truncated_master()) + .await + .unwrap_err(); + // The server's own reason travels with the status. + match err { + CoreError::Forbidden(msg) => { + assert_eq!(msg, format!("server-refused: HTTP {status}: no")) + } + other => panic!("{status}: {other:?}"), + } + } + } + + /// The error keeps only the first 300 characters of an answer, so a + /// realistic envelope arrives cut: its reason is still read. + #[test] + fn the_reason_is_read_from_a_cut_answer() { + let long = "The recurrence rule does not generate an occurrence on the start date \ + of the event, so the event cannot be saved as it is."; + let body = format!( + r#"{{"error":{{"code":400,"message":"{long}","errors":[{{"domain":"global","reason":"invalid","message":"{long}"}}],"innerError":{{"date":"2026-09-26T10:00:00","request-id":"0f6c1d2e-9a8b-4c3d-8e7f-6a5b4c3d2e1f"}}}}}}"# + ); + let cut: String = body.chars().take(300).collect(); + assert!( + serde_json::from_str::(&cut).is_err(), + "the test must cut" + ); + assert_eq!(server_reason(&cut).as_deref(), Some(long)); + // A reason cut itself is read as far as it goes. + let cut_early: String = body.chars().take(60).collect(); + assert!(server_reason(&cut_early).is_some_and(|r| long.starts_with(&r))); + // No message, no reason. + assert_eq!( + server_reason(r#"{"error":{"code":"ErrorInvalidRequest"}}"#), + None + ); + assert_eq!(server_reason("Bad Request"), None); + } + + const PATCH_PATH: &str = r"^/calendars/primary/events/master-1"; + + /// The save met an expired access token, and the token endpoint refused + /// the refresh (a revoked grant answers 400 `invalid_grant`): a sign-in + /// failure, as the error the user can act on — not the calendar server + /// refusing the change. + #[tokio::test] + async fn a_refused_token_refresh_during_an_update_is_a_sign_in_failure() { + let mut server = Server::new_async().await; + let _write = server + .mock("PATCH", Matcher::Regex(PATCH_PATH.into())) + .with_status(401) + .create_async() + .await; + let _token = server + .mock("POST", "/token") + .with_status(400) + .with_body(r#"{"error":"invalid_grant","error_description":"Token has been expired or revoked."}"#) + .create_async() + .await; + let err = adapter_for(&server) + .update_event(truncated_master()) + .await + .unwrap_err(); + match err { + CoreError::Authentication(msg) => assert!(msg.contains("invalid_grant"), "{msg}"), + other => panic!("expected a sign-in failure, got {other:?}"), + } + } + + /// A server error says nothing about what was written: unsure, as before. + #[tokio::test] + async fn an_update_that_failed_in_the_server_is_not_a_refusal() { + let mut server = Server::new_async().await; + let _patch = server + .mock( + "PATCH", + Matcher::Regex(r"^/calendars/primary/events/master-1".into()), + ) + .with_status(503) + .create_async() + .await; + let err = adapter_for(&server) + .update_event(truncated_master()) + .await + .unwrap_err(); + assert!(matches!(err, CoreError::Protocol(_)), "{err:?}"); + } + /// Build an adapter whose API + token endpoints point at the mock /// server. The access token is valid for an hour so no refresh fires. fn adapter_for(server: &Server) -> GoogleAdapter { diff --git a/crates/adapter-microsoft-graph/src/api.rs b/crates/adapter-microsoft-graph/src/api.rs index d73a58ec..2c4a463f 100644 --- a/crates/adapter-microsoft-graph/src/api.rs +++ b/crates/adapter-microsoft-graph/src/api.rs @@ -240,7 +240,7 @@ impl ApiState { .await .map_err(|err| { warn!(?err, "refresh-token grant failed"); - err + refresh_refused(err) })?; let mut guard = self.tokens.lock().await; guard.access_token = fresh.access_token; @@ -252,6 +252,24 @@ impl ApiState { } } +/// A token endpoint that answered the refresh with 400 or 401 — a revoked or +/// expired grant (`invalid_grant`), a client it does not know — refused the +/// sign-in, not the request that needed it: said as a 401, the write that was +/// waiting for the token reads as a sign-in failure. As its own status, an +/// update read it as the calendar server refusing the change (decision 147). +/// Anything else keeps its error. +fn refresh_refused(err: GraphError) -> GraphError { + match err { + GraphError::Http { status, message } if status == 400 || status == 401 => { + GraphError::Http { + status: 401, + message: format!("token refresh refused (HTTP {status}): {message}"), + } + } + other => other, + } +} + async fn decode_json(response: reqwest::Response) -> GraphResult { let status = response.status(); let text = response.text().await.unwrap_or_default(); @@ -527,11 +545,13 @@ pub async fn delete_event(state: &ApiState, event_id: &str) -> GraphResult<()> { } /// `POST /me/events/{id}/cancel` — the ORGANIZER cancels a meeting: Graph -/// emails a cancellation to every attendee and marks the event cancelled -/// server-side (it stays on the organizer's calendar as `isCancelled` until -/// deleted). Organizer-only; Graph rejects it for a non-organizer or a -/// non-meeting. The caller (`delete_event` with `send_cancellations`) pairs it -/// with a follow-up DELETE to actually remove the row. +/// emails a cancellation to every attendee and marks the event cancelled. +/// Microsoft documents that the action moves the event to Deleted Items, and a +/// move gives an Outlook item a new id unless immutable ids are asked for +/// (this adapter does not), so the id may be gone afterwards. Organizer-only; +/// Graph rejects it for a non-organizer or a non-meeting. The caller +/// (`delete_event` with `send_cancellations`) pairs it with a follow-up DELETE +/// to remove the row wherever it still is, and takes a 404 there as done. pub async fn cancel_event(state: &ApiState, event_id: &str) -> GraphResult<()> { let id_enc = urlencoding(event_id); let path = format!("/me/events/{id_enc}/cancel"); diff --git a/crates/adapter-microsoft-graph/src/lib.rs b/crates/adapter-microsoft-graph/src/lib.rs index 6e73cc41..6ae8f30a 100644 --- a/crates/adapter-microsoft-graph/src/lib.rs +++ b/crates/adapter-microsoft-graph/src/lib.rs @@ -350,12 +350,13 @@ impl CalendarFeature for MicrosoftGraphAdapter { async fn update_event(&self, event: Event) -> CoreResult { api::update_event(&self.state, &event) .await - .map_err(to_core_error) + .map_err(to_update_error) } async fn delete_event(&self, event_id: &str, send_cancellations: bool) -> CoreResult<()> { // Graph's event-id is mailbox-wide unique — no calendar // walk required, unlike Google. + let mut cancelled = false; if send_cancellations { // Organizer cancellation: notify attendees first (Graph marks the // event cancelled), then remove it. Graph's `/cancel` is @@ -371,7 +372,7 @@ impl CalendarFeature for MicrosoftGraphAdapter { // transient failure doesn't silently drop the cancellation and // delete anyway. match api::cancel_event(&self.state, event_id).await { - Ok(()) => {} + Ok(()) => cancelled = true, Err(GraphError::Http { status, .. }) if status == 400 || status == 403 => { tracing::debug!( status, @@ -382,9 +383,14 @@ impl CalendarFeature for MicrosoftGraphAdapter { Err(err) => return Err(to_core_error(err)), } } - api::delete_event(&self.state, event_id) - .await - .map_err(to_core_error) + match api::delete_event(&self.state, event_id).await { + // `/cancel` moved the event to Deleted Items, and a move gives an + // Outlook item a new id: the old one is gone, which is what was + // asked. Reported as a failure, undoing a split's new series said + // it could not, while it was cancelled and off the calendar. + Err(GraphError::Http { status: 404, .. }) if cancelled => Ok(()), + other => other.map_err(to_core_error), + } } async fn get_free_busy(&self, emails: &[&str], range: DateRange) -> CoreResult> { @@ -732,6 +738,64 @@ fn urlencoding(s: &str) -> String { out } +/// [`to_core_error`] for an update. A status with which the server turned the +/// write down whole ([`cal_core::WriteRefusal::refused_status`]) is a refusal, +/// not a protocol error: a caller deciding whether the write may have landed — +/// splitting a series undoes its new part only when the cut certainly did not +/// (decision 144) — must be told that nothing was written. +fn to_update_error(err: GraphError) -> CoreError { + match err { + GraphError::Http { status, message } if cal_core::WriteRefusal::refused_status(status) => { + tracing::warn!(status, %message, "Graph refused the update"); + CoreError::Forbidden(cal_core::WriteRefusal::ServerRefused.message( + &match server_reason(&message) { + Some(reason) => format!("HTTP {status}: {reason}"), + None => format!("HTTP {status}"), + }, + )) + } + other => to_core_error(other), + } +} + +/// The reason a JSON error answer gives (`{"error":{"message":"…"}}`), +/// trimmed, for the sentence that says the server refused: "HTTP 400" alone +/// tells the user nothing they can act on. +/// +/// The error keeps only the first 300 characters of the answer, so a longer +/// envelope arrives cut and is no JSON any more. The first `"message"` +/// string is then read as far as it goes: it is the error's own, before the +/// details and the request ids that make an envelope long. +fn server_reason(body: &str) -> Option { + let reason = match serde_json::from_str::(body) { + Ok(value) => value.get("error")?.get("message")?.as_str()?.to_string(), + Err(_) => { + let rest = &body[body.find("\"message\"")? + "\"message\"".len()..]; + let rest = rest.trim_start().strip_prefix(':')?.trim_start(); + let rest = rest.strip_prefix('"')?; + let mut out = String::new(); + let mut escaped = false; + for c in rest.chars() { + match (escaped, c) { + (true, _) => { + out.push(c); + escaped = false; + } + (false, '\\') => escaped = true, + (false, '"') => break, + (false, _) => out.push(c), + } + } + out + } + }; + let reason = reason.trim(); + if reason.is_empty() { + return None; + } + Some(reason.chars().take(160).collect()) +} + fn to_core_error(err: GraphError) -> CoreError { use GraphError::*; match err { @@ -758,6 +822,187 @@ mod delta_tests { use chrono::TimeZone; use mockito::{Matcher, Server}; + /// A series head, cut short: what splitting a series writes. + fn truncated_master() -> Event { + Event { + keep_attendees: false, + keep_fields: Vec::new(), + clear_attendees: false, + organized_elsewhere: false, + id: "ev-1".into(), + calendar_id: "cal-1".into(), + title: "Teamrunde".into(), + description: None, + location: None, + start: chrono::Utc.with_ymd_and_hms(2026, 6, 1, 7, 0, 0).unwrap(), + end: chrono::Utc.with_ymd_and_hms(2026, 6, 1, 8, 0, 0).unwrap(), + all_day: false, + recurrence: Some(cal_core::EventRecurrence { + rrule: "FREQ=WEEKLY;UNTIL=20260824T065959Z".into(), + exceptions: vec![], + tzid: None, + }), + color_label: None, + color_hex: None, + reminders: vec![], + sound: None, + attendees: vec![], + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + etag: None, + organizer: None, + attendee_responses: vec![], + send_invitations: false, + truncate_tail_overrides: false, + cancelled: false, + scheduling_silenced: false, + } + } + + /// Graph read the truncate and turned it down whole: a refusal, so a split + /// takes its new part back (decision 144). + #[tokio::test] + async fn an_update_graph_turned_down_is_a_refusal() { + let mut server = Server::new_async().await; + // The read before the write may fail; it is only for the body. + let _get = server + .mock("GET", "/me/events/ev-1") + .with_status(404) + .create_async() + .await; + let _patch = server + .mock("PATCH", "/me/events/ev-1") + .with_status(400) + .with_body(r#"{"error":{"code":"ErrorInvalidRequest"}}"#) + .create_async() + .await; + let err = adapter_for(&server) + .update_event(truncated_master()) + .await + .unwrap_err(); + // An answer without a message of its own gives the status alone. + match err { + CoreError::Forbidden(msg) => assert_eq!(msg, "server-refused: HTTP 400"), + other => panic!("{other:?}"), + } + } + + /// The error keeps only the first 300 characters of an answer, so a + /// realistic envelope arrives cut: its reason is still read. + #[test] + fn the_reason_is_read_from_a_cut_answer() { + let long = "The recurrence rule does not generate an occurrence on the start date \ + of the event, so the event cannot be saved as it is."; + let body = format!( + r#"{{"error":{{"code":400,"message":"{long}","errors":[{{"domain":"global","reason":"invalid","message":"{long}"}}],"innerError":{{"date":"2026-09-26T10:00:00","request-id":"0f6c1d2e-9a8b-4c3d-8e7f-6a5b4c3d2e1f"}}}}}}"# + ); + let cut: String = body.chars().take(300).collect(); + assert!( + serde_json::from_str::(&cut).is_err(), + "the test must cut" + ); + assert_eq!(server_reason(&cut).as_deref(), Some(long)); + // A reason cut itself is read as far as it goes. + let cut_early: String = body.chars().take(60).collect(); + assert!(server_reason(&cut_early).is_some_and(|r| long.starts_with(&r))); + // No message, no reason. + assert_eq!( + server_reason(r#"{"error":{"code":"ErrorInvalidRequest"}}"#), + None + ); + assert_eq!(server_reason("Bad Request"), None); + } + + const PATCH_PATH: &str = r"^/me/events/ev-1$"; + + /// The save met an expired access token, and the token endpoint refused + /// the refresh (a revoked grant answers 400 `invalid_grant`): a sign-in + /// failure, as the error the user can act on — not the calendar server + /// refusing the change. + #[tokio::test] + async fn a_refused_token_refresh_during_an_update_is_a_sign_in_failure() { + let mut server = Server::new_async().await; + let _get = server + .mock("GET", "/me/events/ev-1") + .with_status(404) + .create_async() + .await; + let _write = server + .mock("PATCH", Matcher::Regex(PATCH_PATH.into())) + .with_status(401) + .create_async() + .await; + let _token = server + .mock("POST", "/token") + .with_status(400) + .with_body(r#"{"error":"invalid_grant","error_description":"Token has been expired or revoked."}"#) + .create_async() + .await; + let err = adapter_for(&server) + .update_event(truncated_master()) + .await + .unwrap_err(); + match err { + CoreError::Authentication(msg) => assert!(msg.contains("invalid_grant"), "{msg}"), + other => panic!("expected a sign-in failure, got {other:?}"), + } + } + + /// `/cancel` moves the event to Deleted Items, which gives it a new id: the + /// DELETE that follows may find nothing, and that is the event gone. + #[tokio::test] + async fn a_delete_that_finds_nothing_after_the_cancel_is_done() { + let mut server = Server::new_async().await; + let _cancel = server + .mock("POST", "/me/events/ev-1/cancel") + .with_status(202) + .create_async() + .await; + let _delete = server + .mock("DELETE", "/me/events/ev-1") + .with_status(404) + .create_async() + .await; + adapter_for(&server) + .delete_event("ev-1", true) + .await + .expect("cancelled, and gone"); + } + + /// ...but only after a cancel: without one, a 404 is an event not found, + /// and after one, any other failure is still a failure. + #[tokio::test] + async fn a_delete_that_finds_nothing_without_a_cancel_is_not_found() { + let mut server = Server::new_async().await; + let _delete = server + .mock("DELETE", "/me/events/ev-1") + .with_status(404) + .create_async() + .await; + let err = adapter_for(&server) + .delete_event("ev-1", false) + .await + .unwrap_err(); + assert!(matches!(err, CoreError::NotFound(_)), "{err:?}"); + + let mut server = Server::new_async().await; + let _cancel = server + .mock("POST", "/me/events/ev-1/cancel") + .with_status(202) + .create_async() + .await; + let _delete = server + .mock("DELETE", "/me/events/ev-1") + .with_status(500) + .create_async() + .await; + let err = adapter_for(&server) + .delete_event("ev-1", true) + .await + .unwrap_err(); + assert!(matches!(err, CoreError::Protocol(_)), "{err:?}"); + } + fn adapter_for(server: &Server) -> MicrosoftGraphAdapter { let mut adapter = MicrosoftGraphAdapter::new( "client".into(), diff --git a/crates/cal-core/bindings/WriteRefusal.ts b/crates/cal-core/bindings/WriteRefusal.ts index b57074c3..d43ff64d 100644 --- a/crates/cal-core/bindings/WriteRefusal.ts +++ b/crates/cal-core/bindings/WriteRefusal.ts @@ -3,4 +3,4 @@ /** * The reason a write did not happen. */ -export type WriteRefusal = "reply-only-invitation" | "server-refused" | "identity-unknown" | "occurrence-not-writable"; +export type WriteRefusal = "reply-only-invitation" | "server-refused" | "identity-unknown" | "occurrence-not-writable" | "unsafe-to-write"; diff --git a/crates/cal-core/src/write_refusal.rs b/crates/cal-core/src/write_refusal.rs index 3100b4b0..1062ad21 100644 --- a/crates/cal-core/src/write_refusal.rs +++ b/crates/cal-core/src/write_refusal.rs @@ -41,6 +41,12 @@ pub enum WriteRefusal { /// the same thing: this occurrence could not be saved on its own. Carving /// it out instead is never done behind their back (decision 92). OccurrenceNotWritable, + /// Aperio did not write, because the event could not be written without + /// risking what else its resource holds: the resource cannot be read + /// block by block, it holds no such event, or its components name + /// different organizers. The detail is a machine token for the log, as + /// for [`Self::OccurrenceNotWritable`]. + UnsafeToWrite, } impl WriteRefusal { @@ -51,9 +57,26 @@ impl WriteRefusal { Self::ServerRefused => "server-refused", Self::IdentityUnknown => "identity-unknown", Self::OccurrenceNotWritable => "occurrence-not-writable", + Self::UnsafeToWrite => "unsafe-to-write", } } + /// Whether a server that answered a write with this HTTP status turned the + /// write down whole: it read the request and wrote nothing (bad request, + /// too large, wrong media type, unprocessable, too many requests, no + /// storage). The statuses the adapters already name — 401, 403, 404, 409, + /// 412 — carry their own error; a 5xx may come after a part was written + /// and says nothing either way. + /// + /// Where a write is refused this way, an adapter reports + /// [`Self::ServerRefused`] rather than a protocol error, so that a caller + /// deciding whether the write may have landed (splitting a series undoes + /// its new part only when the cut certainly did not, decision 144) is told + /// the truth. + pub const fn refused_status(status: u16) -> bool { + matches!(status, 400 | 413 | 415 | 422 | 429 | 507) + } + /// The message: the token, and the detail behind a colon when there is one. pub fn message(self, detail: &str) -> String { let detail = detail.trim(); @@ -74,6 +97,8 @@ impl WriteRefusal { Self::ReplyOnlyInvitation, Self::ServerRefused, Self::IdentityUnknown, + Self::OccurrenceNotWritable, + Self::UnsafeToWrite, ] { let token = refusal.token(); let rest = match message.strip_prefix(token) { @@ -117,6 +142,40 @@ mod tests { ); } + #[test] + fn every_refusal_is_read_back() { + for refusal in [ + WriteRefusal::ReplyOnlyInvitation, + WriteRefusal::ServerRefused, + WriteRefusal::IdentityUnknown, + WriteRefusal::OccurrenceNotWritable, + WriteRefusal::UnsafeToWrite, + ] { + let msg = refusal.message("detail"); + assert_eq!( + WriteRefusal::parse(&msg), + Some((refusal, "detail")), + "{msg}" + ); + // The token is the serialized name the surfaces look up. + assert_eq!( + serde_json::to_value(refusal).unwrap(), + serde_json::json!(refusal.token()), + ); + } + } + + #[test] + fn a_refusing_status_is_one_that_wrote_nothing() { + for status in [400, 413, 415, 422, 429, 507] { + assert!(WriteRefusal::refused_status(status), "{status}"); + } + // Named elsewhere (401, 403, 404, 409, 412), or unknown either way. + for status in [200, 401, 403, 404, 409, 412, 500, 502, 503, 504] { + assert!(!WriteRefusal::refused_status(status), "{status}"); + } + } + #[test] fn a_message_that_only_starts_like_one_is_not_a_refusal() { assert_eq!(WriteRefusal::parse("server-refused-by-proxy: x"), None); diff --git a/crates/host-core/src/cache/mod.rs b/crates/host-core/src/cache/mod.rs index f4cc3c4a..154f9dd8 100644 --- a/crates/host-core/src/cache/mod.rs +++ b/crates/host-core/src/cache/mod.rs @@ -281,9 +281,13 @@ const CONFIRM_THRESHOLD: i64 = 2; /// opposed to a network blip)? Substring match over the usual suspects — /// conservative on purpose: a false "auth" only makes the UI suggest /// re-checking the password. The OAuth needles matter because a revoked -/// Google/Graph grant surfaces as the TOKEN endpoint's HTTP 400 body -/// (`{"error":"invalid_grant",...}`) embedded in a protocol error, not -/// as a 401 — exactly the case where re-authenticating is the fix. +/// grant's text carries the TOKEN endpoint's body +/// (`{"error":"invalid_grant",...}`): Google and Graph now report such a +/// refused refresh as an authentication error ("token refresh refused (HTTP +/// 400): …"), but the builtin-oauth adapters (Drive, Dropbox, Webex), plugin +/// messages passed through verbatim and older plugin builds still carry it +/// as "protocol error: … HTTP 400: …" — exactly the case where +/// re-authenticating is the fix. pub fn is_auth_shaped(error: &str) -> bool { let lower = error.to_lowercase(); [ diff --git a/crates/host-core/src/cache/tests.rs b/crates/host-core/src/cache/tests.rs index 16debd60..dbdc0930 100644 --- a/crates/host-core/src/cache/tests.rs +++ b/crates/host-core/src/cache/tests.rs @@ -2628,13 +2628,16 @@ fn auth_shaped_heuristic() { assert!(super::is_auth_shaped("HTTP 401 Unauthorized")); assert!(super::is_auth_shaped("server said: invalid credentials")); assert!(super::is_auth_shaped("403 Forbidden")); - // Revoked OAuth grant: the token endpoint's HTTP 400 body embedded - // in a protocol error — the exact string shape both OAuth adapters - // record (Google/Graph map token failures to a 400, not a 401). + // Revoked OAuth grant: the token endpoint's HTTP 400 body embedded in + // a protocol error — the shape builtin-oauth adapters and older plugin + // builds record — and as Google and Graph report it now. assert!(super::is_auth_shaped( "protocol error: Google HTTP 400: {\"error\":\"invalid_grant\",\ \"error_description\":\"Token has been expired or revoked.\"}" )); + assert!(super::is_auth_shaped( + "authentication failed: token refresh refused (HTTP 400): {\"error\":\"invalid_grant\"}" + )); assert!(!super::is_auth_shaped("connection reset by peer")); assert!(!super::is_auth_shaped("timeout after 30s")); } diff --git a/locales/de/translation.json b/locales/de/translation.json index e071f1d4..9862903a 100644 --- a/locales/de/translation.json +++ b/locales/de/translation.json @@ -1640,6 +1640,7 @@ "serverRefused": "Der Kalenderserver hat diese Änderung abgelehnt ({{detail}}). Es wurde nichts geändert.", "identityUnknown": "Aperio konnte die eigenen Adressen dieses Kontos auf dem Kalenderserver nicht lesen und kann dort deshalb keine Besprechung ändern. Es wurde nichts geändert.", "occurrenceNotWritable": "Dieser eine Termin lässt sich in seiner Serie nicht einzeln ändern. Es wurde nichts geändert.", + "unsafeToWrite": "Aperio kann diesen Termin nicht sicher ändern: Was auf dem Server noch mit ihm gespeichert ist, stünde auf dem Spiel. Es wurde nichts geändert.", "forbidden": "Der Anbieter erlaubt diese Änderung nicht ({{detail}}). Es wurde nichts geändert.", "changedOnServer": "Dieser Termin wurde auf dem Server geändert, seit du ihn geöffnet hast. Es wurde nichts geändert. Öffne ihn erneut, um den aktuellen Stand zu sehen.", "reason": { @@ -1647,6 +1648,7 @@ "serverRefused": "der Kalenderserver hat es abgelehnt ({{detail}})", "identityUnknown": "Aperio konnte die eigenen Adressen dieses Kontos auf dem Kalenderserver nicht lesen", "occurrenceNotWritable": "dieser eine Termin lässt sich in seiner Serie nicht einzeln ändern", + "unsafeToWrite": "Aperio kann ihn auf dem Server nicht sicher ändern", "forbidden": "der Anbieter erlaubt es nicht ({{detail}})", "changedOnServer": "sie wurde auf dem Server geändert, seit du sie geöffnet hast" } diff --git a/locales/en/translation.json b/locales/en/translation.json index 736f4bd0..64605f78 100644 --- a/locales/en/translation.json +++ b/locales/en/translation.json @@ -1640,6 +1640,7 @@ "serverRefused": "The calendar server refused this change ({{detail}}). Nothing was changed.", "identityUnknown": "Aperio could not read this account's own addresses on the calendar server, so it cannot change a meeting there. Nothing was changed.", "occurrenceNotWritable": "This one occurrence cannot be changed on its own inside its series. Nothing was changed.", + "unsafeToWrite": "Aperio cannot change this event safely: what else is stored with it on the server would be put at risk. Nothing was changed.", "forbidden": "The provider does not allow this change ({{detail}}). Nothing was changed.", "changedOnServer": "This event changed on the server since you opened it. Nothing was changed. Open it again to see the current version.", "reason": { @@ -1647,6 +1648,7 @@ "serverRefused": "the calendar server refused it ({{detail}})", "identityUnknown": "Aperio could not read this account's own addresses on the calendar server", "occurrenceNotWritable": "this occurrence cannot be changed on its own inside its series", + "unsafeToWrite": "Aperio cannot change it safely on the server", "forbidden": "the provider does not allow it ({{detail}})", "changedOnServer": "it changed on the server since you opened it" } diff --git a/shared/eventWriteError.ts b/shared/eventWriteError.ts index 165aff45..f4f4cbff 100644 --- a/shared/eventWriteError.ts +++ b/shared/eventWriteError.ts @@ -17,6 +17,7 @@ const REFUSAL_KEYS: Record = { 'server-refused': 'dialogs.event.writeError.serverRefused', 'identity-unknown': 'dialogs.event.writeError.identityUnknown', 'occurrence-not-writable': 'dialogs.event.writeError.occurrenceNotWritable', + 'unsafe-to-write': 'dialogs.event.writeError.unsafeToWrite', }; const TOKENS = Object.keys(REFUSAL_KEYS) as WriteRefusal[]; @@ -28,6 +29,7 @@ const REFUSAL_REASON_KEYS: Record = { 'server-refused': 'dialogs.event.writeError.reason.serverRefused', 'identity-unknown': 'dialogs.event.writeError.reason.identityUnknown', 'occurrence-not-writable': 'dialogs.event.writeError.reason.occurrenceNotWritable', + 'unsafe-to-write': 'dialogs.event.writeError.reason.unsafeToWrite', }; /** An error as the hosts hand it over: a code and a message. */ diff --git a/shared/generated/WriteRefusal.ts b/shared/generated/WriteRefusal.ts index b57074c3..d43ff64d 100644 --- a/shared/generated/WriteRefusal.ts +++ b/shared/generated/WriteRefusal.ts @@ -3,4 +3,4 @@ /** * The reason a write did not happen. */ -export type WriteRefusal = "reply-only-invitation" | "server-refused" | "identity-unknown" | "occurrence-not-writable"; +export type WriteRefusal = "reply-only-invitation" | "server-refused" | "identity-unknown" | "occurrence-not-writable" | "unsafe-to-write"; diff --git a/src/state/eventWriteError.test.ts b/src/state/eventWriteError.test.ts index 69dc6dd5..41477cb9 100644 --- a/src/state/eventWriteError.test.ts +++ b/src/state/eventWriteError.test.ts @@ -158,3 +158,24 @@ describe('eventWriteFailureReason', () => { expect(eventWriteFailureReason(err, t)).toBe(eventWriteErrorMessage(err, t)); }); }); + +describe('a write Aperio will not risk', () => { + it('reads as its own refusal, on either surface, and as nothing written', () => { + // A CalDAV resource whose blocks name different organizers is not + // written: the adapter says so with a token of its own. + const err = command('forbidden', 'unsafe-to-write: mixed-organizers'); + expect(eventWriteRefusal(err)?.refusal).toBe('unsafe-to-write'); + expect(eventWriteErrorMessage(err, t)).toMatch(/nicht sicher ändern/); + expect(eventWriteErrorMessage(err, t)).not.toMatch(/mixed-organizers/); + expect(eventWriteFailureReason(err, t)).not.toMatch(/nichts geändert/); + expect(writeNeverLanded(err)).toBe(true); + }); + + it('counts a server that turned the write down as nothing written', () => { + // 400, 429 and their kin, from every adapter's update (decision 147). + expect(writeNeverLanded(command('forbidden', 'server-refused: HTTP 429'))).toBe(true); + expect(eventWriteErrorMessage(command('forbidden', 'server-refused: HTTP 429'), t)).toMatch( + /abgelehnt \(HTTP 429\)/, + ); + }); +});