Skip to content

Commit e894a24

Browse files
committed
Eighth-pass audit: fix silent send/profile errors, draft loss on failed send
Three bugs in the Swift shell + core: 1. ChatView never displayed core.view.error. Errors from send_text ("message too long", "contact list is full", "cannot send to a blocked contact", "gave up sending N messages", "invalid key") were set on model.error and rendered into view.error, but ChatView had no error display. The user tapped send, it failed, and they saw nothing. Now shows the error above the composer. 2. ChatView cleared the local draft unconditionally after sendText, even when send_text refused the message (blocked, too long, cap full). The core preserved model.compose on those paths, but the Swift draft was already wiped -- the failure was both invisible and destructive. Now only clears draft when core.view.error is empty (i.e., the core accepted the message). 3. EditProfileView never displayed core.view.error. "display name too long", "bio too long", "photo too large" were silently swallowed when the user tapped Save. Now shows the error in a section. Also in core: send_text and SaveProfile now clear model.error at the top, so a stale error from a previous failed attempt does not persist after the user fixes the issue and tries again. Previously the error display would show the old error even after a successful send/save. Test: send_text_clears_a_stale_error_on_success verifies the clearing.
1 parent c24f528 commit e894a24

3 files changed

Lines changed: 71 additions & 15 deletions

File tree

apple/SkrepkaApp/Forms.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,15 @@ struct EditProfileView: View {
158158
}
159159
Section("Name") { TextField("Display name", text: $name) }
160160
Section("Bio") { TextField("About you", text: $bio, axis: .vertical) }
161+
// SaveProfile sets model.error on validation failures ("display name
162+
// too long", "bio too long", "photo too large") but leaves the page
163+
// open. Without displaying it the user taps Save, nothing happens, and
164+
// they have no idea why.
165+
if !core.view.error.isEmpty {
166+
Section {
167+
Text(core.view.error).foregroundColor(.red)
168+
}
169+
}
161170
}
162171
.navigationTitle("Edit profile")
163172
.toolbar {

apple/SkrepkaApp/Views.swift

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -319,22 +319,45 @@ struct ChatView: View {
319319
}
320320

321321
private var composer: some View {
322-
HStack(spacing: 8) {
323-
TextField("Message", text: $draft, axis: .vertical)
324-
.textFieldStyle(.roundedBorder)
325-
.lineLimit(1...4)
326-
.disabled(core.view.activePeerBlocked)
327-
Button {
328-
let text = draft.trimmingCharacters(in: .whitespacesAndNewlines)
329-
guard !text.isEmpty else { return }
330-
core.update(.composeChanged(text))
331-
core.update(.sendText)
332-
draft = ""
333-
} label: {
334-
Image(systemName: "arrow.up.circle.fill").font(.title)
322+
VStack(spacing: 0) {
323+
// Errors from send_text ("message too long", "contact list is full",
324+
// "gave up sending …", "invalid key") are set on model.error and rendered
325+
// into view.error, but without this the chat page swallowed them — the
326+
// user tapped send, it failed, and they saw nothing.
327+
if !core.view.error.isEmpty {
328+
Text(core.view.error)
329+
.font(.caption)
330+
.foregroundColor(.red)
331+
.frame(maxWidth: .infinity, alignment: .leading)
332+
.padding(.horizontal)
333+
.padding(.top, 4)
334+
}
335+
HStack(spacing: 8) {
336+
TextField("Message", text: $draft, axis: .vertical)
337+
.textFieldStyle(.roundedBorder)
338+
.lineLimit(1...4)
339+
.disabled(core.view.activePeerBlocked)
340+
Button {
341+
let text = draft.trimmingCharacters(in: .whitespacesAndNewlines)
342+
guard !text.isEmpty else { return }
343+
core.update(.composeChanged(text))
344+
core.update(.sendText)
345+
// Only clear the local draft when the core actually accepted the
346+
// message. send_text refuses (and sets error) on a blocked peer,
347+
// an oversized body, or a full contact list — and it preserves
348+
// model.compose on those paths so the user's text is not lost.
349+
// Clearing draft unconditionally wiped it from the UI even though
350+
// the core kept it, so the failure was both invisible and
351+
// destructive: the text was gone with no explanation.
352+
if core.view.error.isEmpty {
353+
draft = ""
354+
}
355+
} label: {
356+
Image(systemName: "arrow.up.circle.fill").font(.title)
357+
}
358+
.disabled(draft.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
359+
|| core.view.activePeerBlocked)
335360
}
336-
.disabled(draft.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
337-
|| core.view.activePeerBlocked)
338361
}
339362
.padding(.horizontal).padding(.vertical, 8)
340363
.background(.bar)

core/src/app.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,8 @@ impl App for Skrepka {
959959
bio,
960960
photo,
961961
} => {
962+
// Clear a stale error from a previous failed save.
963+
model.error = None;
962964
// The recipient's `parse_payload` enforces these caps and
963965
// silently drops oversized profiles. Without these checks the
964966
// sender's local profile would be saved, broadcast to every
@@ -1574,6 +1576,9 @@ impl Skrepka {
15741576
let Some(peer) = model.active_peer.clone() else {
15751577
return render();
15761578
};
1579+
// Clear a stale error from a previous failed send, so the error display
1580+
// does not persist after the user fixes the issue and sends again.
1581+
model.error = None;
15771582
// Blocking cuts the peer off in both directions (PROTOCOL.md §4). The
15781583
// ingest side drops their messages; the send side must refuse too, or
15791584
// a blocked peer still receives our texts, acks, and the liveness
@@ -4435,6 +4440,25 @@ mod tests {
44354440
assert!(m.error.as_ref().is_some_and(|e| e.contains("too long")));
44364441
}
44374442

4443+
/// A stale error from a previous failed send must be cleared on the next
4444+
/// attempt, so the error display does not persist after the user fixes the
4445+
/// issue and sends again successfully.
4446+
#[test]
4447+
fn send_text_clears_a_stale_error_on_success() {
4448+
let app = Skrepka;
4449+
let mut m = with_identity();
4450+
let peer = peer_hex(9);
4451+
m.active_peer = Some(peer.clone());
4452+
// Simulate a prior failure that left an error behind.
4453+
m.error = Some("message too long".into());
4454+
4455+
m.compose = "hello".into();
4456+
let _ = app.update(Event::SendText, &mut m);
4457+
4458+
assert!(m.error.is_none(), "a successful send clears the stale error");
4459+
assert!(!m.outbox.is_empty(), "the message was queued");
4460+
}
4461+
44384462
// -----------------------------------------------------------------------
44394463
// SaveProfile field length checks
44404464
// -----------------------------------------------------------------------

0 commit comments

Comments
 (0)