Skip to content

Support cancellation of tasks with FetchHTTPClient #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xbhatnag wants to merge 1 commit into
base: main
Choose a base branch
from
+53 −7
Open

Support cancellation of tasks with FetchHTTPClient #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions Examples/WASMClient/main.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import JavaScriptKit
typealias DefaultExecutorFactory = JavaScriptEventLoop
JavaScriptEventLoop.installGlobalExecutor()

let client = FetchHTTPClient()
let status = Status()

// Ask the user for the URL string.
Expand Down Expand Up @@ -52,9 +51,10 @@ if method == .post || method == .put {
}
}

status.set("⏳ Making \(method) request to \(url)")
func makeRequest(url: URL, method: HTTPRequest.Method, body: HTTPClientRequestBody<FetchHTTPClient.RequestBodyWriter>?) async throws {
let client = FetchHTTPClient()
status.set("⏳ Making \(method) request to \(url)")

do {
try await client.perform(
request: .init(
method: method,
Expand Down Expand Up @@ -117,6 +117,21 @@ do {
div("<binary>")
}
}
}

let requestTask = Task {
try await makeRequest(url: url, method: method, body: body)
}

let sleepTask = Task {
status.set("💤 Waiting for 10 seconds")
try await Task.sleep(for: .seconds(10))
status.set("🚫 Cancelling task")
requestTask.cancel()
}

do {
try await requestTask.value
} catch {
status.set("❌ Fetch failed: \(error)")
}
27 changes: 24 additions & 3 deletions Sources/FetchHTTPClient/FetchHTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,21 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {
}

// Perform the request
let requestInit = RequestInit(body: jsBody, method: request.method.rawValue, headers: requestHeaders)
let response = try await fetch(url.absoluteString, requestInit)
let abortController = try AbortController()
let signal = try abortController.signal
let requestInit = RequestInit(body: jsBody, method: request.method.rawValue, headers: requestHeaders, signal: signal)

// No matter what, we should abort the request at the end of this function
defer {
try? abortController.abort()
}

let response = try await withTaskCancellationHandler {
return try await fetch(url.absoluteString, requestInit)
} onCancel: {
try? abortController.abort()
}

let responseStatus = try response.status
let responseStatusText = try response.statusText
let stream = try response.body
Expand Down Expand Up @@ -163,8 +176,16 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {
if buffer.isEmpty {
// Read more data in from JS
let chunk: Chunk

// TODO: Find a way to make this safe.
nonisolated(unsafe) let reader = reader

do {
chunk = try await reader.read()
chunk = try await withTaskCancellationHandler {
try await reader.read()
} onCancel: {
try? reader.releaseLock()
}
} catch {
throw .first(error)
}
Expand Down
12 changes: 11 additions & 1 deletion Sources/FetchHTTPClient/JSImports.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ import JavaScriptKit
/// # Javascript Imports
/// This file defines the Javascript classes and functions imported into Swift.

// https://developer.mozilla.org/en-US/docs/Web/API/AbortController
@JSClass(from: .global) struct AbortController: @unchecked Sendable {
@JSGetter var signal: JSObject?
@JSFunction init() throws(JSException)
@JSFunction func abort() throws(JSException)
Copy link
Copy Markdown
Collaborator

@guoye-zhang guoye-zhang May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these functions safe to call from any threads?

Copy link
Copy Markdown
Collaborator Author

@xbhatnag xbhatnag May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no expert on how this translates to JS, but isn't JS single-threaded, making WASM single-threaded, making all the asynchronous contexts in Swift also single-threaded?

Copy link
Copy Markdown
Collaborator

@guoye-zhang guoye-zhang May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that multithreading is supported in WASM: https://webassembly.org/features/

Copy link
Copy Markdown
Collaborator Author

@xbhatnag xbhatnag May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today I learned :) @MaxDesiatov might know more about this specific question...

Copy link
Copy Markdown
Member

@MaxDesiatov MaxDesiatov May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our Swift SDK for Wasm is single-threaded by default. Multi-threading is available as an option, so I don't think @unchecked Sendable is justifiable here.

Copy link
Copy Markdown
Member

@MaxDesiatov MaxDesiatov May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could work as a temporary measure. I would like @kateinoigakukun to chime in, as he's been using the multi-threaded Swift SDK for Wasm extensively.

Copy link
Copy Markdown

@kateinoigakukun kateinoigakukun May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any JS methods against a JSObject should be called on the thread on which the JSObject is created. So as long as those methods are called on the thread the AbortController instance is created on, it's safe. However if we cannot guarantee that, we need to use JSRemote<T> to call them safely: swiftwasm/JavaScriptKit#711

Copy link
Copy Markdown
Collaborator

@guoye-zhang guoye-zhang May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does JS thread map to Swift concurrency? There is no guarantee of the thread that an async method runs on since threads can switch arbitrarily after an await right? Should we wrap everything in JSRemote or just use @MainActor? (I assume that MainActor maps to the main thread in JS as well)

Copy link
Copy Markdown

@kateinoigakukun kateinoigakukun May 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MainActor-isolated code is consistently mapped to the browser main thread in any case, and there is no guarantee that any other async code runs on. However, practically speaking, JavaScriptKit does not provide a global concurrent executor even under a multithreading environment, and it just provides TaskExecutor implementations, and those implementations guarantee no thread hop in a structured async code so that client code can control which thread a specific code can run on in an opt-in way.

I took a closer look on the code, and I can say FetchHTTPClient.perform itself is safe because all JS object access happens within a structured code, but ResponseBodyReader.read has to use JSRemote because the caller might be running on a different thread than the thread FetchHTTPClient.perform was called.

After swiftwasm/JavaScriptKit#747 lands, we can fix the potential thread issue by the following patch.

Patch 
diff --git a/Sources/FetchHTTPClient/FetchHTTPClient.swift b/Sources/FetchHTTPClient/FetchHTTPClient.swift
index cb6825e..d5c4bc4 100644
--- a/Sources/FetchHTTPClient/FetchHTTPClient.swift
+++ b/Sources/FetchHTTPClient/FetchHTTPClient.swift
@@ -120,7 +120,7 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {

         return try await responseHandler(
             HTTPResponse(status: .init(code: responseStatus, reasonPhrase: responseStatusText), headerFields: responseHeaders),
-            ResponseReader(reader: reader)
+            ResponseReader(reader: JSRemote(reader))
         )
     }

@@ -145,7 +145,7 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {
     }

     public struct ResponseReader: ConcludingAsyncReader, ~Copyable {
-        let reader: ReadableStreamDefaultReader
+        let reader: JSRemote<ReadableStreamDefaultReader>

         public consuming func consumeAndConclude<Return, Failure>(
             body: nonisolated(nonsending) (consuming sending FetchHTTPClient.ResponseBodyReader) async throws(Failure) -> Return
@@ -159,7 +159,7 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {
         public typealias ReadFailure = any Error
         public typealias Buffer = UniqueArray<UInt8>

-        let reader: ReadableStreamDefaultReader
+        let reader: JSRemote<ReadableStreamDefaultReader>
         var buffer = UniqueArray<UInt8>()

         public mutating func read<Return: ~Copyable, Failure>(
@@ -167,26 +167,35 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {
+        let reader: JSRemote<ReadableStreamDefaultReader>
         var buffer = UniqueArray<UInt8>()

         public mutating func read<Return: ~Copyable, Failure>(
@@ -167,26 +167,35 @@ public final class FetchHTTPClient: HTTPAPIs.HTTPClient {
         ) async throws(AsyncStreaming.EitherError<any Error, Failure>) -> Return where Failure: Error {
             let chunk: Chunk
             do {
-                chunk = try await self.reader.read()
+                chunk = try await reader.withJSObject { reader in
+                    try await reader.read()
+                }
+            } catch {
+                throw .first(error)
+            }
+            do {
+                try self.append(chunk: chunk)
             } catch {
                 throw .first(error)
             }
+            do {
+                return try await body(&self.buffer)
+            } catch {
+                throw .second(error)
+            }
+        }
+
+        private mutating func append(chunk: Chunk) throws {
             if !chunk.done {
                 guard let bytes = chunk.value, !bytes.isEmpty else {
                     // If not done, there must be bytes that can be read
-                    throw .first(FetchError.BadAssumptionJS)
+                    throw FetchError.BadAssumptionJS
                 }
                 buffer.reserveCapacity(bytes.count)
                 for b in bytes {
                     self.buffer.append(b)
                 }
             }
-
-            do {
-                return try await body(&self.buffer)
-            } catch {
-                throw .second(error)
-            }
         }
     }
 }

Copy link
Copy Markdown
Collaborator

@guoye-zhang guoye-zhang May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope to understand a bit more how this works. Is fetch allowed to be called from any thread? Does JSRemote internally switch back to the creating thread to call methods?

Wondering if there is an efficiency difference between wrapping the reader in JSRemote or just slapping MainActor on everything to let Swift handle thread hopping.

}

// https://developer.mozilla.org/en-US/docs/Web/API/Headers
@JSClass(from: .global) struct Headers {
@JSFunction init() throws(JSException)
Expand Down Expand Up @@ -58,11 +65,13 @@ import JavaScriptKit
let body: JSObject?
let method: String?
let headers: Headers?
let signal: JSObject?

init(body: JSObject?, method: String?, headers: Headers?) {
init(body: JSObject?, method: String?, headers: Headers?, signal: JSObject?) {
self.body = body
self.method = method
self.headers = headers
self.signal = signal
}
}

Expand All @@ -76,6 +85,7 @@ import JavaScriptKit
// TODO: Find a way to remove the @unchecked. This object has to be moved through the different Swift reader types.
@JSClass(from: .global) struct ReadableStreamDefaultReader: @unchecked Sendable {
Copy link
Copy Markdown
Member

@MaxDesiatov MaxDesiatov May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, @unchecked Sendable is most probably not correct here.

cc @kateinoigakukun

@JSFunction func read() async throws(JSException) -> Chunk
@JSFunction func releaseLock() throws(JSException)
}

// https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream
Expand Down
Loading