Skip to content

Commit 0b14d47

Browse files
authored
Cancelling uploadMedia task should cancel underlying HTTP request (#794)
* Export a couple more API * Add `RequestExecutionErrorReason::CancellationError` * Report cancellation error in the Swift wrapper * Cancelling `uploadMedia` task should cancel underlying HTTP request * Fix swiftlint issues
1 parent 0c52d3f commit 0b14d47

File tree

7 files changed

+91
-6
lines changed

7 files changed

+91
-6
lines changed

native/swift/Sources/wordpress-api/Exports.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ public typealias MediaUploadRequest = WordPressAPIInternal.MediaUploadRequest
111111
public typealias MediaWithEditContext = WordPressAPIInternal.MediaWithEditContext
112112
public typealias MediaWithViewContext = WordPressAPIInternal.MediaWithViewContext
113113
public typealias MediaWithEmbedContext = WordPressAPIInternal.MediaWithEmbedContext
114+
public typealias MediaCreateParams = WordPressAPIInternal.MediaCreateParams
115+
public typealias MediaUpdateParams = WordPressAPIInternal.MediaUpdateParams
114116
public typealias MediaListParams = WordPressAPIInternal.MediaListParams
115117
public typealias MediaRequestExecutor = WordPressAPIInternal.MediaRequestExecutor
116118

native/swift/Sources/wordpress-api/Extensions.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,12 @@ extension CommentType: ExpressibleByStringLiteral {
6464
self.init(stringLiteral)
6565
}
6666
}
67+
68+
extension WpApiError {
69+
public var isCancellationError: Bool {
70+
if case .RequestExecutionFailed(statusCode: _, redirects: _, reason: .cancellationError) = self {
71+
return true
72+
}
73+
return false
74+
}
75+
}

native/swift/Sources/wordpress-api/SafeRequestExecutor.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,14 @@ public final class WpRequestExecutor: SafeRequestExecutor {
9797
return handleDeviceIsOfflineError(error)
9898
}
9999

100+
if let urlError = error as? URLError, urlError.code == .cancelled {
101+
return .failure(.RequestExecutionFailed(
102+
statusCode: nil,
103+
redirects: nil,
104+
reason: .cancellationError
105+
))
106+
}
107+
100108
return .failure(.RequestExecutionFailed(
101109
statusCode: nil,
102110
redirects: nil,

native/swift/Sources/wordpress-api/WordPressAPI.swift

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ public actor WordPressAPI {
138138
) async throws -> MediaRequestCreateResponse {
139139
precondition(localFileURL.isFileURL)
140140
precondition(progress.completedUnitCount == 0 && progress.totalUnitCount > 0)
141+
precondition(progress.cancellationHandler == nil)
141142

142143
let requestId = WpUuid()
143144

@@ -168,13 +169,18 @@ public actor WordPressAPI {
168169
)
169170
}
170171

171-
if progress.cancellationHandler == nil {
172-
progress.cancellationHandler = {
173-
uploadTask.cancel()
174-
}
172+
progress.cancellationHandler = {
173+
uploadTask.cancel()
175174
}
176175

177-
return try await uploadTask.value
176+
return try await withTaskCancellationHandler {
177+
try await uploadTask.value
178+
} onCancel: {
179+
// Please note: for some reason calling `uploadTask.cancel()` here does not actually cancel
180+
// the HTTP request. But calling `progress.cancel()` does, even though `progress.cancel()` eventually
181+
// calls `uploadTask.cancel()`.
182+
progress.cancel()
183+
}
178184
}
179185
#endif
180186

native/swift/Tests/integration-tests/MediaTests.swift

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,62 @@ struct MediaTests {
3939

4040
try await restoreTestServer()
4141
}
42-
#endif
42+
43+
@Test
44+
func cancelProgress() async throws {
45+
let progress = Progress.discreteProgress(totalUnitCount: 100)
46+
#expect(progress.fractionCompleted == 0)
47+
48+
let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil))
49+
await #expect(
50+
throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError),
51+
performing: {
52+
let task = Task {
53+
_ = try await api.uploadMedia(
54+
params: .init(),
55+
fromLocalFileURL: file,
56+
fulfilling: progress
57+
)
58+
}
59+
60+
let cancellable = progress.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in
61+
progress.cancel()
62+
}
63+
defer { cancellable.cancel() }
64+
65+
try await task.value
66+
}
67+
)
68+
69+
try await restoreTestServer()
70+
}
71+
72+
@Test
73+
func cancelTask() async throws {
74+
let progress = Progress.discreteProgress(totalUnitCount: 100)
75+
#expect(progress.fractionCompleted == 0)
76+
let file = try #require(Bundle.module.url(forResource: "test-data/test_media.jpg", withExtension: nil))
77+
await #expect(
78+
throws: WpApiError.RequestExecutionFailed(statusCode: nil, redirects: nil, reason: .cancellationError),
79+
performing: {
80+
let task = Task {
81+
_ = try await api.uploadMedia(
82+
params: .init(),
83+
fromLocalFileURL: file,
84+
fulfilling: progress
85+
)
86+
}
87+
88+
let cancellable = progress.publisher(for: \.fractionCompleted).first { $0 > 0 }.sink { _ in
89+
task.cancel()
90+
}
91+
defer { cancellable.cancel() }
92+
93+
try await task.value
94+
}
95+
)
96+
97+
try await restoreTestServer()
98+
}
99+
#endif
43100
}

wp_api/src/api_error.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ pub enum RequestExecutionErrorReason {
566566
DeviceIsOfflineError {
567567
error_message: String,
568568
},
569+
CancellationError,
569570
HttpError {
570571
reason: String,
571572
},
@@ -656,6 +657,7 @@ impl WpSupportsLocalization for RequestExecutionErrorReason {
656657
WpMessages::just(error_message)
657658
}
658659
RequestExecutionErrorReason::HttpTimeoutError => WpMessages::http_timeout_error(),
660+
RequestExecutionErrorReason::CancellationError => WpMessages::http_cancellation_error(),
659661
}
660662
}
661663
}

wp_localization/localization/en-US/main.ftl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ non_existent_site_error = A server with the specified hostname could not be foun
2222
http_authentication_required_error = The server at {$url} requires authentication. Please provide your username and password.
2323
http_forbidden_error = The server at {$url} denied access to the requested resource. Please check your site's configuration.
2424
http_timeout_error = The connection timed out
25+
http_cancellation_error = The request was cancelled.
2526
2627
http_authentication_rejected_error = The server at {$url} rejected your credentials. Please provide a valid username and password.
2728

0 commit comments

Comments
 (0)