From 225ff9ad2270ec3959edcc45ba3a09082ac89d0f Mon Sep 17 00:00:00 2001 From: cdricms <36056008+cdricms@users.noreply.github.com> Date: Fri, 7 Aug 2026 10:49:32 +0200 Subject: [PATCH] fix: stop the device picker hanging on an offer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two causes. The connect step had no timeout, so an unreachable device was retried indefinitely instead of falling through to hold-for-later; it now gives up after 15s and holds the offer as designed. The picker also waited on the whole exchange, which includes a person on the other device deciding — up to two minutes. It now closes on tap and reports the outcome as a message, and a decline or an unanswered offer is shown as information rather than an error, since the offer did arrive. --- apple/Tests/ContactsModelTests.swift | 29 ++++++++++++++ .../Features/Contacts/ContactsModel.swift | 17 ++++++++ .../Features/Contacts/DevicePickerSheet.swift | 12 +++--- .../VniDrop/UI/Feedback/UserFacingError.swift | 19 +++++++++ crates/vnidrop/src/runtime/contacts.rs | 12 ++++-- localization/strings.json | 40 +++++++++++++++++++ .../composeResources/values-de/strings.xml | 2 + .../composeResources/values-es/strings.xml | 2 + .../composeResources/values-fr/strings.xml | 2 + .../composeResources/values-it/strings.xml | 2 + .../composeResources/values-nl/strings.xml | 2 + .../composeResources/values-pl/strings.xml | 2 + .../composeResources/values-pt/strings.xml | 2 + .../composeResources/values-ru/strings.xml | 2 + .../composeResources/values/strings.xml | 2 + 15 files changed, 139 insertions(+), 8 deletions(-) diff --git a/apple/Tests/ContactsModelTests.swift b/apple/Tests/ContactsModelTests.swift index 223ba88..51680b4 100644 --- a/apple/Tests/ContactsModelTests.swift +++ b/apple/Tests/ContactsModelTests.swift @@ -414,6 +414,35 @@ final class ContactsModelTests: XCTestCase { XCTAssertFalse(delivered) } + /// A refusal by the person on the other device is information, not an error. + func testADeclinedOfferIsReportedWithoutAnErrorTone() async { + let gateway = FakeCoreGateway() + gateway.offerTransferResult = .failure( + InvitationError.raw("permission error: device did not accept the transfer: receiver-declined") + ) + let defaults = UserDefaults(suiteName: "contacts-declined-\(UUID().uuidString)")! + let preferences = AppPreferencesRepository( + defaults: defaults, + fallback: AppPreferencesDefaults( + username: "tester", + receiveFolder: ReceiveFolder(kind: .fileSystemPath, value: "/tmp", displayName: "Downloads"), + themeMode: .system + ) + ) + let messages = UiMessageController() + let model = ContactsModel( + repository: gateway, + messages: messages, + preferences: preferences, + fileSystemService: FakeFileSystemService() + ) + + let delivered = await model.offerTransfer(transferId: 7, to: contact("peer")) + + XCTAssertFalse(delivered) + XCTAssertEqual(messages.current?.tone, .info) + } + func testUnreachableContactIsSurfacedForRepairing() async { let gateway = FakeCoreGateway() gateway.contactsResult = .success([contact("peer", canSend: false)]) diff --git a/apple/VniDrop/Features/Contacts/ContactsModel.swift b/apple/VniDrop/Features/Contacts/ContactsModel.swift index 019e7a2..5f2a147 100644 --- a/apple/VniDrop/Features/Contacts/ContactsModel.swift +++ b/apple/VniDrop/Features/Contacts/ContactsModel.swift @@ -345,6 +345,14 @@ final class ContactsModel: ObservableObject { } } + /// Fire-and-report variant of ``offerTransfer(transferId:to:)``. + /// + /// Owned by the model rather than a view so the request survives the picker + /// being dismissed: the answer depends on a person at the other device. + func offerTransferInBackground(transferId: UInt64, to contact: DeviceContact) { + Task { await offerTransfer(transferId: transferId, to: contact) } + } + /// Push an existing transfer to a remembered device. /// /// Returns whether it landed, so the caller can distinguish "accepted" from @@ -365,6 +373,15 @@ final class ContactsModel: ObservableObject { messages.tryShow(UiMessage(text: text, tone: outcome.delivered ? .success : .info)) await refresh() return outcome.delivered + case .failure(let error) where error.offerRefusal != nil: + // The offer was delivered and a person said no, or nobody answered. + // Neither is a failure of this device, so neither is shown as one. + let text = error.offerRefusal == .declined + ? L10n.Contacts.declinedByDevice(device: contact.displayName) + : L10n.Contacts.noAnswer(device: contact.displayName) + messages.tryShow(UiMessage(text: .dynamic(text), tone: .info)) + await refresh() + return false case .failure(let error): messages.error(error) return false diff --git a/apple/VniDrop/Features/Contacts/DevicePickerSheet.swift b/apple/VniDrop/Features/Contacts/DevicePickerSheet.swift index e656bdc..1d439f4 100644 --- a/apple/VniDrop/Features/Contacts/DevicePickerSheet.swift +++ b/apple/VniDrop/Features/Contacts/DevicePickerSheet.swift @@ -31,10 +31,12 @@ struct DevicePickerSheet: View { } else { List(reachable) { contact in Button { - Task { - await model.offerTransfer(transferId: transferId, to: contact) - dismiss() - } + // Close first. The other device's user has to accept, + // which can take as long as they take, and holding a + // modal open on someone else's decision reads as a + // hang. The outcome arrives as a message instead. + dismiss() + model.offerTransferInBackground(transferId: transferId, to: contact) } label: { HStack { VStack(alignment: .leading, spacing: 2) { @@ -49,7 +51,7 @@ struct DevicePickerSheet: View { } } } - .disabled(!model.state.busyEndpoints.isEmpty) + .disabled(model.state.busyEndpoints.contains(contact.endpointId)) } } } diff --git a/apple/VniDrop/UI/Feedback/UserFacingError.swift b/apple/VniDrop/UI/Feedback/UserFacingError.swift index 25f1ede..ebbf2ea 100644 --- a/apple/VniDrop/UI/Feedback/UserFacingError.swift +++ b/apple/VniDrop/UI/Feedback/UserFacingError.swift @@ -3,6 +3,12 @@ import VnidropCore /// Maps technical failures to stable, user-facing catalog keys. Ported from /// `ui/feedback/UserFacingError.kt`. Never exposes raw `reason=` blobs. +/// How an offered transfer ended without being accepted. +enum OfferRefusal { + case declined + case noAnswer +} + extension Error { func toUiText() -> UiText { if let invitation = self as? InvitationError { @@ -57,6 +63,19 @@ extension Error { || haystack.contains("user canceled") } + /// The other device answered, and the answer was no. + /// + /// Not a failure of this device: the offer was delivered and a person + /// declined it, so it is reported as information rather than an error. + var offerRefusal: OfferRefusal? { + let haystack = technicalDetail.lowercased() + if haystack.contains("receiver-declined") || haystack.contains("declined-recently") { + return .declined + } + if haystack.contains("no-response") { return .noAnswer } + return nil + } + /// Prefers a `VnidropError` reason; else the localized description. var technicalDetail: String { if let vni = self as? VnidropError { diff --git a/crates/vnidrop/src/runtime/contacts.rs b/crates/vnidrop/src/runtime/contacts.rs index 7b2664e..23eebe6 100644 --- a/crates/vnidrop/src/runtime/contacts.rs +++ b/crates/vnidrop/src/runtime/contacts.rs @@ -4,7 +4,7 @@ //! [`crate::pairing`]; this is where those meet the endpoint and the UniFFI //! surface. -use std::sync::Arc; +use std::{sync::Arc, time::Duration}; use anyhow::{Context, Result}; use iroh::{EndpointAddr, EndpointId}; @@ -27,6 +27,12 @@ use crate::{ util::now_ms, }; +/// How long to wait for a device to answer before treating it as not running. +/// +/// Without this an offline peer never fails, it just keeps being retried, and +/// the offer is never handed to the hold-for-later path. +const OFFER_CONNECT_TIMEOUT: Duration = Duration::from_secs(15); + /// Whether a device may be polled again yet. /// /// Split out because the surrounding call needs two live nodes to exercise, @@ -419,9 +425,9 @@ impl CoreInner { ) -> Result { let addr = self.contact_addr(endpoint_id).await?; let client = OfferService::client(self.endpoint.clone(), addr); - let challenge = client - .request_challenge() + let challenge = tokio::time::timeout(OFFER_CONNECT_TIMEOUT, client.request_challenge()) .await + .map_err(|_| VnidropError::transfer(anyhow::anyhow!("device did not answer in time")))? .context("device is not reachable") .map_err(VnidropError::transfer)?; diff --git a/localization/strings.json b/localization/strings.json index b908b23..7dcf4f8 100644 --- a/localization/strings.json +++ b/localization/strings.json @@ -5284,6 +5284,46 @@ "nl": "{device} heeft de overdracht geaccepteerd", "ru": "{device} принял передачу" } + }, + "contacts_declined_by_device": { + "context": "Shown when the person on the other device declined an offered transfer. {device} = device name.", + "args": [ + { + "name": "device", + "type": "string" + } + ], + "translations": { + "en": "{device} declined the transfer", + "fr": "{device} a refusé le transfert", + "es": "{device} rechazó la transferencia", + "it": "{device} ha rifiutato il trasferimento", + "de": "{device} hat die Übertragung abgelehnt", + "pt": "{device} recusou a transferência", + "pl": "{device} odrzuciło przesyłkę", + "nl": "{device} heeft de overdracht geweigerd", + "ru": "{device} отклонил передачу" + } + }, + "contacts_no_answer": { + "context": "Shown when an offered transfer got no answer on the other device before timing out. {device} = device name.", + "args": [ + { + "name": "device", + "type": "string" + } + ], + "translations": { + "en": "{device} did not answer", + "fr": "{device} n’a pas répondu", + "es": "{device} no respondió", + "it": "{device} non ha risposto", + "de": "{device} hat nicht geantwortet", + "pt": "{device} não respondeu", + "pl": "{device} nie odpowiedziało", + "nl": "{device} heeft niet geantwoord", + "ru": "{device} не ответил" + } } } } diff --git a/shared/src/commonMain/composeResources/values-de/strings.xml b/shared/src/commonMain/composeResources/values-de/strings.xml index 95749e9..198c6b5 100644 --- a/shared/src/commonMain/composeResources/values-de/strings.xml +++ b/shared/src/commonMain/composeResources/values-de/strings.xml @@ -348,6 +348,8 @@ Gerät auswählen Derzeit ist kein Gerät erreichbar. Gespeicherte Geräte erscheinen hier nach einer Übertragung. %1$s hat die Übertragung angenommen + %1$s hat die Übertragung abgelehnt + %1$s hat nicht geantwortet %1$d Datei %1$d Dateien diff --git a/shared/src/commonMain/composeResources/values-es/strings.xml b/shared/src/commonMain/composeResources/values-es/strings.xml index fbf3875..110836e 100644 --- a/shared/src/commonMain/composeResources/values-es/strings.xml +++ b/shared/src/commonMain/composeResources/values-es/strings.xml @@ -348,6 +348,8 @@ Elegir un dispositivo Ningún dispositivo está disponible ahora. Los dispositivos guardados aparecen aquí tras una transferencia. %1$s aceptó la transferencia + %1$s rechazó la transferencia + %1$s no respondió %1$d archivo %1$d archivos diff --git a/shared/src/commonMain/composeResources/values-fr/strings.xml b/shared/src/commonMain/composeResources/values-fr/strings.xml index 6d9f507..43aae34 100644 --- a/shared/src/commonMain/composeResources/values-fr/strings.xml +++ b/shared/src/commonMain/composeResources/values-fr/strings.xml @@ -348,6 +348,8 @@ Choisir un appareil Aucun appareil n’est joignable pour le moment. Les appareils enregistrés apparaissent ici après un transfert. %1$s a accepté le transfert + %1$s a refusé le transfert + %1$s n’a pas répondu %1$d fichier %1$d fichiers diff --git a/shared/src/commonMain/composeResources/values-it/strings.xml b/shared/src/commonMain/composeResources/values-it/strings.xml index dabd605..f1e9983 100644 --- a/shared/src/commonMain/composeResources/values-it/strings.xml +++ b/shared/src/commonMain/composeResources/values-it/strings.xml @@ -348,6 +348,8 @@ Scegli un dispositivo Nessun dispositivo è raggiungibile ora. I dispositivi memorizzati compaiono qui dopo un trasferimento. %1$s ha accettato il trasferimento + %1$s ha rifiutato il trasferimento + %1$s non ha risposto %1$d file %1$d file diff --git a/shared/src/commonMain/composeResources/values-nl/strings.xml b/shared/src/commonMain/composeResources/values-nl/strings.xml index 32c6159..c5a3dc9 100644 --- a/shared/src/commonMain/composeResources/values-nl/strings.xml +++ b/shared/src/commonMain/composeResources/values-nl/strings.xml @@ -348,6 +348,8 @@ Kies een apparaat Er is nu geen apparaat bereikbaar. Onthouden apparaten verschijnen hier na een overdracht. %1$s heeft de overdracht geaccepteerd + %1$s heeft de overdracht geweigerd + %1$s heeft niet geantwoord %1$d bestand %1$d bestanden diff --git a/shared/src/commonMain/composeResources/values-pl/strings.xml b/shared/src/commonMain/composeResources/values-pl/strings.xml index 42e49eb..7cd110c 100644 --- a/shared/src/commonMain/composeResources/values-pl/strings.xml +++ b/shared/src/commonMain/composeResources/values-pl/strings.xml @@ -348,6 +348,8 @@ Wybierz urządzenie Żadne urządzenie nie jest teraz dostępne. Zapamiętane urządzenia pojawią się tu po przesłaniu. %1$s zaakceptowało przesyłkę + %1$s odrzuciło przesyłkę + %1$s nie odpowiedziało %1$d plik %1$d pliki diff --git a/shared/src/commonMain/composeResources/values-pt/strings.xml b/shared/src/commonMain/composeResources/values-pt/strings.xml index b7228e8..908d63b 100644 --- a/shared/src/commonMain/composeResources/values-pt/strings.xml +++ b/shared/src/commonMain/composeResources/values-pt/strings.xml @@ -348,6 +348,8 @@ Escolher um dispositivo Nenhum dispositivo está acessível agora. Os dispositivos guardados aparecem aqui após uma transferência. %1$s aceitou a transferência + %1$s recusou a transferência + %1$s não respondeu %1$d ficheiro %1$d ficheiros diff --git a/shared/src/commonMain/composeResources/values-ru/strings.xml b/shared/src/commonMain/composeResources/values-ru/strings.xml index 42445f8..0037800 100644 --- a/shared/src/commonMain/composeResources/values-ru/strings.xml +++ b/shared/src/commonMain/composeResources/values-ru/strings.xml @@ -348,6 +348,8 @@ Выберите устройство Сейчас ни одно устройство недоступно. Сохранённые устройства появятся здесь после передачи. %1$s принял передачу + %1$s отклонил передачу + %1$s не ответил %1$d файл %1$d файла diff --git a/shared/src/commonMain/composeResources/values/strings.xml b/shared/src/commonMain/composeResources/values/strings.xml index 2be10da..666614c 100644 --- a/shared/src/commonMain/composeResources/values/strings.xml +++ b/shared/src/commonMain/composeResources/values/strings.xml @@ -348,6 +348,8 @@ Choose a device No device can be reached right now. Remembered devices appear here after a transfer. %1$s accepted the transfer + %1$s declined the transfer + %1$s did not answer %1$d files %1$d file