From d162a89ac28fa3a228a2ba9a20b776caa1ed8f60 Mon Sep 17 00:00:00 2001 From: jack Date: Sun, 6 Jul 2025 22:14:41 +0200 Subject: [PATCH] Add debugging and fix read receipts for messages without delivery status - Add detailed logging to track read receipt generation - Handle messages that don't have delivery status (older messages) - Send read receipts for all unread messages when opening chat --- DELIVERY_CONFIRMATION_SUMMARY.md | 74 ++++++++++++++++++++++++++ bitchat/ViewModels/ChatViewModel.swift | 21 +++++++- test_delivery_confirmation.md | 48 +++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 DELIVERY_CONFIRMATION_SUMMARY.md create mode 100644 test_delivery_confirmation.md diff --git a/DELIVERY_CONFIRMATION_SUMMARY.md b/DELIVERY_CONFIRMATION_SUMMARY.md new file mode 100644 index 00000000..a1289dfb --- /dev/null +++ b/DELIVERY_CONFIRMATION_SUMMARY.md @@ -0,0 +1,74 @@ +# Delivery Confirmation Feature Summary + +## Overview +This branch implements a complete delivery confirmation system for bitchat, providing visual feedback for message delivery status and read receipts. + +## Key Components Implemented + +### 1. Delivery Status Tracking +- **DeliveryTracker.swift**: Core service managing delivery confirmations + - Tracks pending deliveries with timeouts (30s for private, 60s for rooms) + - Handles retries for favorite peers (up to 3 attempts) + - Thread-safe implementation with proper lock management + +### 2. Protocol Updates +- **BitchatProtocol.swift**: + - Added `DeliveryStatus` enum with states: sending, sent, delivered, read, failed + - Added `DeliveryAck` structure for acknowledgments + - Added `ReadReceipt` structure for read confirmations + - New message types: `.deliveryAck` and `.readReceipt` + +### 3. UI Implementation +- **ContentView.swift**: + - Added `DeliveryStatusView` component showing: + - Gray circle (○) for sending + - Gray checkmark (✓) for sent + - Green double checkmarks (✓✓) for delivered + - Blue double checkmarks (✓✓) for read + - Red triangle (⚠) for failed + - Proper vertical alignment with message text + +### 4. Message Flow +- **BluetoothMeshService.swift**: + - Added ACK generation and sending + - Added read receipt generation when viewing chats + - Encrypted ACKs for privacy + - Proper message ID handling + +### 5. Critical Bug Fixes + +#### Deadlock Fix +- **Issue**: macOS app freeze when sending private messages +- **Cause**: Nested lock acquisition in DeliveryTracker +- **Fix**: Restructured code to release locks before calling methods that acquire the same lock + +#### Message ID Preservation +- **Issue**: ACKs referenced wrong message IDs +- **Cause**: BinaryProtocol was reading but discarding ID with `let _ =` +- **Fix**: Changed to `let id =` and passed ID to BitchatMessage constructor + +## Technical Details + +### Privacy Considerations +- ACKs and read receipts are encrypted end-to-end +- Only the original sender can decrypt and process them +- No message content is included in confirmations + +### Performance +- Debounced UI updates (100ms) +- Efficient lock usage to prevent deadlocks +- Automatic cleanup of old delivery data + +### Error Handling +- Graceful timeout handling +- Retry logic for favorites +- Clear failure messages + +## Testing +See `test_delivery_confirmation.md` for comprehensive test plan. + +## Future Enhancements +- Group read receipts (show who read in rooms) +- Delivery timestamps in UI +- Persistent delivery status across app restarts +- Network quality indicators \ No newline at end of file diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 631687d9..5cd476c8 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -896,12 +896,19 @@ class ChatViewModel: ObservableObject { } private func markPrivateMessagesAsRead(from peerID: String) { - guard let messages = privateChats[peerID] else { return } + guard let messages = privateChats[peerID] else { + print("[Delivery] No messages found for peer \(peerID)") + return + } + + print("[Delivery] Checking \(messages.count) messages from peer \(peerID) for read receipts") // Find messages from the peer that haven't been read yet for message in messages { // Only send read receipts for messages from the other peer (not our own) // and only if the status is delivered (not already read) + print("[Delivery] Message \(message.id) from \(message.sender), senderPeerID: \(message.senderPeerID ?? "nil"), status: \(message.deliveryStatus?.displayText ?? "none")") + if message.senderPeerID == peerID { if let status = message.deliveryStatus { switch status { @@ -916,11 +923,23 @@ class ChatViewModel: ObservableObject { print("[Delivery] Sending read receipt for message \(message.id)") case .read: // Already read, no need to send another receipt + print("[Delivery] Message \(message.id) already marked as read") break default: // Message not yet delivered, can't mark as read + print("[Delivery] Message \(message.id) has status \(status.displayText), not sending read receipt") break } + } else { + // No delivery status - this might be an older message + // Send read receipt anyway for backwards compatibility + print("[Delivery] Message \(message.id) has no delivery status, sending read receipt anyway") + let receipt = ReadReceipt( + originalMessageID: message.id, + readerID: meshService.myPeerID, + readerNickname: nickname + ) + meshService.sendReadReceipt(receipt, to: peerID) } } } diff --git a/test_delivery_confirmation.md b/test_delivery_confirmation.md new file mode 100644 index 00000000..fb62fbe3 --- /dev/null +++ b/test_delivery_confirmation.md @@ -0,0 +1,48 @@ +# Delivery Confirmation Test Plan + +## Test Setup +1. Build and run the bitchat app on two devices (or two instances on one device) +2. Connect the devices and exchange nicknames + +## Test Cases + +### 1. Basic Delivery Confirmation +- Send a private message from Device A to Device B +- Expected: + - Device A shows single gray checkmark (sent) + - When Device B receives, Device A shows double green checkmarks (delivered) + - Message IDs should match in logs + +### 2. Read Receipts +- Device B opens/views the chat with Device A +- Expected: + - Device A shows double blue checkmarks (read) + - Console logs show "Generating read receipt" and "Received READ receipt" + +### 3. Timeout Handling +- Send a message to an offline peer +- Expected: + - After 30 seconds, status changes to "Failed: Message not delivered" + +### 4. Message ID Consistency +- Monitor console logs when sending messages +- Expected: + - Message ID in "Tracking message" log matches ID in "Processing ACK" log + - No UUID mismatches + +## Debug Commands +```bash +# Filter delivery-related logs +log stream --predicate 'processImagePath contains "bitchat"' | grep -E "(DeliveryTracker|Delivery|ACK|receipt)" + +# Check for message ID issues +log stream --predicate 'processImagePath contains "bitchat"' | grep -E "(Tracking message|Processing ACK|message ID)" +``` + +## Known Working Flow +1. Alice sends message to Bob (ID: abc123) +2. Bob receives message and generates ACK +3. Bob sends encrypted ACK back to Alice +4. Alice receives and processes ACK, updates UI to "delivered" +5. When Bob views chat, read receipt is sent +6. Alice receives read receipt, updates UI to blue checkmarks \ No newline at end of file