Fix non-background-session webhook request replacement (#1487)

Fixes #1473. Again.

## Summary
When we issue webhook requests, we were failing to cancel any previous requests (which want to be cancelled) that were executed on not-the-background session. This moves to now checking both the background and non-background for cancellable requests.

## Any other notes
- Moves to explicitly cancelling, rather than silently replacing and succeeding, replaced requests. For example, a sensor update will now be explicitly cancelled if a new sensor update comes through, rather than following the success path.
- This resolves an issue where a sensor update occurs, fails on the non-background session, and retries with the same request data on the background session. This can lead to an older sensor update request, with outdated information, happening long after the most recent and correct one finishing.
This commit is contained in:
Zac West
2021-02-15 16:47:01 -08:00
committed by GitHub
parent 5368a74b69
commit ae1d3ac4a0
3 changed files with 91 additions and 9 deletions

View File

@@ -3,12 +3,20 @@ import ObjectMapper
import PromiseKit
import UserNotifications
internal enum WebhookError: LocalizedError, Equatable {
internal enum WebhookError: LocalizedError, Equatable, CancellableError {
case noApi
case unregisteredIdentifier(handler: String)
case unexpectedType(given: String, desire: String)
case unacceptableStatusCode(Int)
case unmappableValue
case replaced
var isCancelled: Bool {
switch self {
case .replaced: return true
default: return false
}
}
var errorDescription: String? {
switch self {
@@ -22,6 +30,9 @@ internal enum WebhookError: LocalizedError, Equatable {
return L10n.HaApi.ApiError.unacceptableStatusCode(statusCode)
case .unmappableValue:
return L10n.HaApi.ApiError.invalidResponse
case .replaced:
// this shouldn't be user-facing
return "<replaced>"
}
}
}
@@ -391,9 +402,9 @@ public class WebhookManager: NSObject {
persisted newPersisted: WebhookPersisted,
with newPromise: Promise<Void>
) {
currentBackgroundSessionInfo.session.getAllTasks { tasks in
let evaluate = { [self] (session: WebhookSessionInfo, tasks: [URLSessionTask]) in
tasks.filter { thisTask in
guard let (thisType, thisPersisted) = self.responseInfo(from: thisTask) else {
guard let (thisType, thisPersisted) = responseInfo(from: thisTask) else {
Current.Log.error("cancelling request without persistence info: \(thisTask)")
thisTask.cancel()
return false
@@ -405,14 +416,24 @@ public class WebhookManager: NSObject {
return false
}
}.forEach { existingTask in
let taskKey = TaskKey(sessionInfo: self.currentBackgroundSessionInfo, task: existingTask)
if let existingResolver = self.resolverForTask[taskKey] {
// connect the task we're about to cancel's promise to the replacement
newPromise.pipe { existingResolver.resolve($0) }
let taskKey = TaskKey(sessionInfo: session, task: existingTask)
if let existingResolver = resolverForTask[taskKey] {
existingResolver.reject(WebhookError.replaced)
}
existingTask.cancel()
}
}
currentRegularSessionInfo.session.getAllTasks { [self] tasks in
dataQueue.async {
evaluate(currentRegularSessionInfo, tasks)
}
}
currentBackgroundSessionInfo.session.getAllTasks { [self] tasks in
dataQueue.async {
evaluate(currentBackgroundSessionInfo, tasks)
}
}
}
private static func urlRequest(