From 78e6651e8785125f43cbfd7a753fa52fafe32e0e Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 1 Sep 2020 13:31:11 +1000 Subject: [PATCH] Fix missed error handling case --- .../Loki/API/Onion Requests/OnionRequestAPI.swift | 8 +++++++- SignalServiceKit/src/Loki/API/SnodeAPI.swift | 14 ++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift index 45a7c0e41..f0b90bac5 100644 --- a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift +++ b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift @@ -339,7 +339,13 @@ public enum OnionRequestAPI { } } promise.catch2 { error in // Must be invoked on LokiAPI.workQueue - guard case HTTP.Error.httpRequestFailed(_, _) = error else { return } + guard case HTTP.Error.httpRequestFailed(let statusCode, let json) = error else { return } + // Marking all the snodes in the path as unreliable here is aggressive, but otherwise users + // can get stuck with a failing path that just refreshes to the same path. + let path = paths.first { $0.contains(guardSnode) } + path?.forEach { snode in + SnodeAPI.handleError(withStatusCode: statusCode, json: json, forSnode: snode) // Intentionally don't throw + } dropAllPaths() // A snode in the path is bad; retry with a different path dropGuardSnode(guardSnode) } diff --git a/SignalServiceKit/src/Loki/API/SnodeAPI.swift b/SignalServiceKit/src/Loki/API/SnodeAPI.swift index 9e24ba37f..5c2255574 100644 --- a/SignalServiceKit/src/Loki/API/SnodeAPI.swift +++ b/SignalServiceKit/src/Loki/API/SnodeAPI.swift @@ -296,7 +296,7 @@ public final class SnodeAPI : NSObject { // MARK: Error Handling /// - Note: Should only be invoked from `LokiAPI.workQueue` to avoid race conditions. - internal static func handleError(withStatusCode statusCode: UInt, json: JSON?, forSnode snode: Snode, associatedWith publicKey: String) -> Error? { + internal static func handleError(withStatusCode statusCode: UInt, json: JSON?, forSnode snode: Snode, associatedWith publicKey: String? = nil) -> Error? { #if DEBUG assertOnQueue(SnodeAPI.workQueue) #endif @@ -307,7 +307,9 @@ public final class SnodeAPI : NSObject { print("[Loki] Couldn't reach snode at: \(snode); setting failure count to \(newFailureCount).") if newFailureCount >= SnodeAPI.snodeFailureThreshold { print("[Loki] Failure threshold reached for: \(snode); dropping it.") - SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + if let publicKey = publicKey { + SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + } SnodeAPI.dropSnodeFromSnodePool(snode) SnodeAPI.snodeFailureCount[snode] = 0 } @@ -321,8 +323,12 @@ public final class SnodeAPI : NSObject { return SnodeAPI.SnodeAPIError.clockOutOfSync case 421: // The snode isn't associated with the given public key anymore - print("[Loki] Invalidating swarm for: \(publicKey).") - SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + if let publicKey = publicKey { + print("[Loki] Invalidating swarm for: \(publicKey).") + SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + } else { + print("[Loki] Got a 421 without an associated public key.") + } case 432: // The proof of work difficulty is too low if let powDifficulty = json?["difficulty"] as? UInt {