Skip to content

Commit 583c12b

Browse files
committed
Box JSException storage in a class to fit the direct typed-error convention
1 parent aa02b44 commit 583c12b

7 files changed

Lines changed: 62 additions & 187 deletions

File tree

Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -610,37 +610,6 @@ private enum ExportSwiftConstants {
610610
static let supportedRawTypes = SwiftEnumRawType.supportedTypeNames
611611
}
612612

613-
/// Warns about Swift closures handed to JavaScript with an `async throws(JSException)` signature.
614-
/// Captureless closure values lose their thrown error at runtime due to a Swift compiler bug.
615-
private func asyncThrowsClosureWarning(node: some SyntaxProtocol) -> DiagnosticError {
616-
DiagnosticError(
617-
node: node,
618-
message:
619-
"async throwing closures passed to JavaScript may lose thrown errors due to a Swift compiler bug "
620-
+ "(swiftlang/swift#89320) unless the closure value captures state",
621-
hint:
622-
"Pass a closure that captures state, or see the BridgeJS closure documentation for details",
623-
severity: .warning
624-
)
625-
}
626-
627-
extension BridgeType {
628-
fileprivate var containsAsyncThrowsClosure: Bool {
629-
switch self {
630-
case .closure(let signature, _):
631-
return signature.isAsync && signature.isThrows
632-
case .nullable(let wrapped, _):
633-
return wrapped.containsAsyncThrowsClosure
634-
case .array(let element):
635-
return element.containsAsyncThrowsClosure
636-
case .dictionary(let value):
637-
return value.containsAsyncThrowsClosure
638-
default:
639-
return false
640-
}
641-
}
642-
}
643-
644613
extension AttributeSyntax {
645614
/// The attribute name as text when it is a simple identifier (e.g. "JS", "JSFunction").
646615
/// Prefer this over `attributeName.trimmedDescription` for name checks to avoid unnecessary string work.
@@ -1233,9 +1202,6 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
12331202

12341203
guard let type = resolvedType else { return nil }
12351204
returnType = type
1236-
if returnType.containsAsyncThrowsClosure {
1237-
errors.append(asyncThrowsClosureWarning(node: returnClause.type))
1238-
}
12391205
} else {
12401206
returnType = .void
12411207
}
@@ -2895,11 +2861,6 @@ private final class ImportSwiftMacrosAPICollector: SyntaxAnyVisitor {
28952861
guard let bridgeType = withLookupErrors({ parent.lookupType(for: type, errors: &$0) }) else {
28962862
return nil
28972863
}
2898-
if case .closure(let signature, useJSTypedClosure: true) = bridgeType,
2899-
signature.isAsync, signature.isThrows
2900-
{
2901-
errors.append(asyncThrowsClosureWarning(node: type))
2902-
}
29032864
let nameToken = param.secondName ?? param.firstName
29042865
let name = SwiftToSkeleton.normalizeIdentifier(nameToken.text)
29052866
let labelToken = param.secondName == nil ? nil : param.firstName

Plugins/BridgeJS/Tests/BridgeJSToolTests/ClosureAsyncThrowsWarningTests.swift

Lines changed: 0 additions & 122 deletions
This file was deleted.

Sources/JavaScriptKit/Documentation.docc/Articles/BridgeJS/Bringing-Swift-Closures-to-JavaScript.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,6 @@ const count = await fetchCount("/items"); // Promise<number>
7878
7979
**Cancellation is a non-goal.** There is no propagation between a Swift `Task` and a JavaScript `Promise` in either direction.
8080

81-
> Note: The reject path of async throwing typed closures is affected by a Swift compiler bug ([swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320)); BridgeJS emits a build-time warning for this signature. See <doc:Exporting-Swift-Closure> for details.
82-
8381
## Lifetime and release()
8482

8583
A ``JSTypedClosure`` keeps the Swift closure alive and exposes a JavaScript function that calls into it. To avoid leaks and use-after-free:

Sources/JavaScriptKit/Documentation.docc/Articles/BridgeJS/Exporting-Swift/Exporting-Swift-Closure.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,6 @@ Notes:
239239
- The same `JavaScriptEventLoop.installGlobalExecutor()` requirement applies as for async functions; there is no special handling for closures.
240240
- **Cancellation is a non-goal.** There is no propagation between a Swift `Task` and a JavaScript `Promise` in either direction; cancelling one side does not cancel the other.
241241
242-
> Warning: When an async throwing closure handed to JavaScript throws, the error is currently lost instead of rejecting the `Promise` with it, due to a Swift compiler bug on `wasm32` ([swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320), fix in progress in [swiftlang/swift#89715](https://github.com/swiftlang/swift/pull/89715)). Closures that capture state are unaffected, as are throwing JavaScript callbacks passed into Swift. BridgeJS emits a build-time warning for this signature.
243-
244242
## Supported Features
245243
246244
| Swift Feature | Status |
@@ -251,7 +249,7 @@ Notes:
251249
| Optional types in closures | ✅ |
252250
| Closure-typed `@JS` properties | ❌ |
253251
| Async closures `(A) async -> B` | ✅ |
254-
| Async throwing closures `(A) async throws(JSException) -> B` | ✅ (reject path of closures handed to JS pending [swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320)) |
252+
| Async throwing closures `(A) async throws(JSException) -> B` | ✅ |
255253
| Throwing closures `(A) throws(JSException) -> B` | ✅ |
256254
257255
## See Also

Sources/JavaScriptKit/JSException.swift

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,66 @@
1313
/// }
1414
/// ```
1515
public struct JSException: Error, Equatable, CustomStringConvertible {
16-
/// The value thrown from JavaScript.
17-
/// This can be any JavaScript value (error object, string, number, etc.).
18-
public var thrownValue: JSValue {
19-
return _thrownValue
16+
/// Boxes the exception payload in a class so `JSException` stays within the direct
17+
/// typed-error convention on wasm32.
18+
private final class Storage {
19+
/// The actual JavaScript value that was thrown.
20+
let thrownValue: JSValue
21+
22+
/// A description of the exception.
23+
let description: String
24+
25+
/// The stack trace of the exception.
26+
let stack: String?
27+
28+
init(thrownValue: JSValue, description: String, stack: String?) {
29+
self.thrownValue = thrownValue
30+
self.description = description
31+
self.stack = stack
32+
}
2033
}
2134

22-
/// The actual JavaScript value that was thrown.
35+
/// The boxed payload of the exception.
2336
///
2437
/// Marked as `nonisolated(unsafe)` to satisfy `Sendable` requirement
2538
/// from `Error` protocol.
26-
private nonisolated(unsafe) let _thrownValue: JSValue
39+
private nonisolated(unsafe) let storage: Storage
40+
41+
/// The value thrown from JavaScript.
42+
/// This can be any JavaScript value (error object, string, number, etc.).
43+
public var thrownValue: JSValue {
44+
return storage.thrownValue
45+
}
2746

2847
/// A description of the exception.
29-
public let description: String
48+
public var description: String {
49+
return storage.description
50+
}
3051

3152
/// The stack trace of the exception.
32-
public let stack: String?
53+
public var stack: String? {
54+
return storage.stack
55+
}
3356

3457
/// Initializes a new JSException instance with a value thrown from JavaScript.
3558
///
3659
/// Only available within the package. This must be called on the thread where the exception object created.
60+
/// The stringified representation is captured on the object owner thread to bring useful info
61+
/// to the catching thread even if they are different threads.
3762
@usableFromInline
3863
package init(_ thrownValue: JSValue) {
39-
self._thrownValue = thrownValue
40-
// Capture the stringified representation on the object owner thread
41-
// to bring useful info to the catching thread even if they are different threads.
4264
if let errorObject = thrownValue.object, let stack = errorObject.stack.string {
43-
self.description = "JSException(\(stack))"
44-
self.stack = stack
65+
self.storage = Storage(
66+
thrownValue: thrownValue,
67+
description: "JSException(\(stack))",
68+
stack: stack
69+
)
4570
} else {
46-
self.description = "JSException(\(thrownValue))"
47-
self.stack = nil
71+
self.storage = Storage(
72+
thrownValue: thrownValue,
73+
description: "JSException(\(thrownValue))",
74+
stack: nil
75+
)
4876
}
4977
}
5078

@@ -55,4 +83,10 @@ public struct JSException: Error, Equatable, CustomStringConvertible {
5583
public init(message: String) {
5684
self.init(JSError(message: message).jsValue)
5785
}
86+
87+
public static func == (lhs: JSException, rhs: JSException) -> Bool {
88+
return lhs.storage.thrownValue == rhs.storage.thrownValue
89+
&& lhs.storage.description == rhs.storage.description
90+
&& lhs.storage.stack == rhs.storage.stack
91+
}
5892
}

Tests/BridgeJSRuntimeTests/JavaScript/ClosureAsyncTests.mjs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ export async function runJsClosureAsyncTests(exports) {
4444
assert.equal(await parsed, "parsed:42");
4545
assert.equal(await parser("-7"), "parsed:-7");
4646

47-
// Blocked by swiftlang/swift#89320 (wasm32 typed-throws async miscompile for captureless closures); re-enable once swiftlang/swift#89715 lands.
48-
const ASYNC_THROWS_CLOSURE_REJECT_BLOCKED = true;
49-
if (!ASYNC_THROWS_CLOSURE_REJECT_BLOCKED) {
5047
let directionBReject = null;
5148
try {
5249
await parser("not-a-number");
@@ -56,7 +53,6 @@ export async function runJsClosureAsyncTests(exports) {
5653
}
5754
assert.notEqual(directionBReject, null);
5855
assert.equal(directionBReject.message, "AsyncParseError: not-a-number");
59-
}
6056

6157
assert.equal(await parser("100"), "parsed:100");
6258

@@ -71,7 +67,6 @@ export async function runJsClosureAsyncTests(exports) {
7167
assert.equal(await recorded, undefined);
7268
assert.equal(exports.lastRecordedValue(), "logged-value");
7369

74-
if (!ASYNC_THROWS_CLOSURE_REJECT_BLOCKED) {
7570
let voidReject = null;
7671
try {
7772
await recorder("boom");
@@ -81,7 +76,6 @@ export async function runJsClosureAsyncTests(exports) {
8176
}
8277
assert.notEqual(voidReject, null);
8378
assert.equal(voidReject.message, "AsyncRecorderError");
84-
}
8579

8680
const pointMaker = exports.makeAsyncPointMaker();
8781
const pointPromise = pointMaker(3);

Tests/JavaScriptEventLoopTests/JSClosure+AsyncTests.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,18 @@ class JSClosureAsyncTests: XCTestCase {
2424
XCTAssertEqual(result, 42.0)
2525
}
2626

27+
func testAsyncClosureReject() async throws {
28+
let closure = JSClosure.async { (_) async throws(JSException) -> JSValue in
29+
throw JSException(message: "AsyncClosureRejected")
30+
}.jsValue
31+
let result = await JSPromise(from: closure.function!())!.result
32+
guard case .failure(let rejectedValue) = result else {
33+
XCTFail("Expected the async closure promise to reject, got \(result)")
34+
return
35+
}
36+
XCTAssertEqual(rejectedValue.object?.message.string, "AsyncClosureRejected")
37+
}
38+
2739
func testAsyncClosureWithPriority() async throws {
2840
let priority = UnsafeSendableBox<TaskPriority?>(nil)
2941
let closure = JSClosure.async(priority: .high) { _ in

0 commit comments

Comments
 (0)