Added a timeout for the MessageSendJob and additional logging

pull/946/head
Morgan Pretty 1 year ago
parent cac29a573a
commit a563cddba8

@ -990,7 +990,7 @@ extension Attachment {
return downloadUrl
.map { urlString -> String? in
urlString
.split(separator: "/")
.split(separator: "/") // stringlint:disable
.last
.map { String($0) }
}

@ -163,7 +163,8 @@ public enum MessageSendJob: JobExecutor {
// Store the sentTimestamp from the message in case it fails due to a clockOutOfSync error
let originalSentTimestamp: UInt64? = details.message.sentTimestamp
/// Perform the actual message sending
/// Perform the actual message sending - this will timeout if the entire process takes longer than `HTTP.defaultTimeout * 2`
/// which can occur if it needs to build a new onion path (which doesn't actually have any limits so can take forever in rare cases)
///
/// **Note:** No need to upload attachments as part of this process as the above logic splits that out into it's own job
/// so we shouldn't get here until attachments have already been uploaded
@ -183,13 +184,23 @@ public enum MessageSendJob: JobExecutor {
.flatMap { MessageSender.sendImmediate(data: $0, using: dependencies) }
.subscribe(on: queue, using: dependencies)
.receive(on: queue, using: dependencies)
.timeout(.milliseconds(Int(HTTP.defaultTimeout * 2 * 1000)), scheduler: queue, customError: {
MessageSenderError.sendJobTimeout
})
.sinkUntilComplete(
receiveCompletion: { result in
switch result {
case .finished: success(job, false, dependencies)
case .failure(let error):
SNLog("[MessageSendJob] Couldn't send message due to error: \(error).")
switch error {
case MessageSenderError.sendJobTimeout:
SNLog("[MessageSendJob] Couldn't send message due to error: \(error) (paths: \(OnionRequestAPI.paths.prettifiedDescription)).")
default:
SNLog("[MessageSendJob] Couldn't send message due to error: \(error).")
}
// Actual error handling
switch error {
case let senderError as MessageSenderError where !senderError.isRetryable:
failure(job, error, true, dependencies)

@ -14,6 +14,7 @@ public enum MessageSenderError: LocalizedError, Equatable {
case noUsername
case attachmentsNotUploaded
case blindingFailed
case sendJobTimeout
// Closed groups
case noThread
@ -43,6 +44,7 @@ public enum MessageSenderError: LocalizedError, Equatable {
case .noUsername: return "Missing username."
case .attachmentsNotUploaded: return "Attachments for this message have not been uploaded."
case .blindingFailed: return "Couldn't blind the sender"
case .sendJobTimeout: return "Send job timeout (likely due to path building taking too long)."
// Closed groups
case .noThread: return "Couldn't find a thread associated with the given group public key."
@ -66,6 +68,7 @@ public enum MessageSenderError: LocalizedError, Equatable {
case (.noKeyPair, .noKeyPair): return true
case (.invalidClosedGroupUpdate, .invalidClosedGroupUpdate): return true
case (.blindingFailed, .blindingFailed): return true
case (.sendJobTimeout, .sendJobTimeout): return true
case (.other(let lhsError), .other(let rhsError)):
// Not ideal but the best we can do

@ -547,10 +547,10 @@ public enum OnionRequestAPI {
switch result {
case .finished: break
case .failure(let error):
guard
case HTTPError.httpRequestFailed(let statusCode, let data) = error,
let guardSnode: Snode = guardSnode
else { return }
guard let guardSnode: Snode = guardSnode else {
return SNLog("Request failed with no guardSnode.")
}
guard case HTTPError.httpRequestFailed(let statusCode, let data) = error else { return }
let path = paths.first { $0.contains(guardSnode) }

Loading…
Cancel
Save