also looking at this
fix(ios): preserve specific protocol errors during dispatch
#303
3 files
+51
-11
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -194,17 +194,16 @@ pub fn errorMessage(err: BridgeError) []const u8 { | ||
| 194 | 194 | /// the dispatcher exposes. Errors the protocol cannot name become the honest |
| 195 | 195 | /// catch-all instead of leaving the page without a reply. |
| 196 | 196 | pub fn fromHandlerError(err: anyerror) BridgeError { |
| 197 | // Preserve every explicitly declared protocol error before translating | |
| 198 | // implementation aliases. A hand-maintained subset loses useful errors | |
| 199 | // (for example WebViewHandleNotSet) when new handlers start returning them. | |
| 200 | inline for (@typeInfo(BridgeError).error_set.error_names.?) |name| { | |
| 201 | const protocol_error = @field(BridgeError, name); | |
| 202 | if (err == protocol_error) return protocol_error; | |
| 203 | } | |
| 197 | 204 | return switch (err) { |
| 198 | error.AllocationFailed, error.OutOfMemory => BridgeError.AllocationFailed, | |
| 199 | error.UnknownAction => BridgeError.UnknownAction, | |
| 200 | error.MissingData => BridgeError.MissingData, | |
| 201 | error.InvalidJSON => BridgeError.InvalidJSON, | |
| 202 | error.InvalidParameter => BridgeError.InvalidParameter, | |
| 203 | error.Busy => BridgeError.Busy, | |
| 204 | error.PlatformNotSupported, error.UnsupportedPlatform => BridgeError.PlatformNotSupported, | |
| 205 | error.NotFound => BridgeError.NotFound, | |
| 206 | error.PermissionDenied => BridgeError.PermissionDenied, | |
| 207 | error.Timeout => BridgeError.Timeout, | |
| 205 | error.OutOfMemory => BridgeError.AllocationFailed, | |
| 206 | error.UnsupportedPlatform => BridgeError.PlatformNotSupported, | |
| 208 | 207 | else => BridgeError.NativeCallFailed, |
| 209 | 208 | }; |
| 210 | 209 | } |
| @@ -379,7 +378,10 @@ test "errorCodeString returns correct codes" { | ||
| 379 | 378 | |
| 380 | 379 | test "handler error narrowing preserves protocol errors" { |
| 381 | 380 | const testing = std.testing; |
| 382 | try testing.expectEqual(BridgeError.Busy, fromHandlerError(error.Busy)); | |
| 381 | inline for (@typeInfo(BridgeError).error_set.error_names.?) |name| { | |
| 382 | const protocol_error = @field(BridgeError, name); | |
| 383 | try testing.expectEqual(protocol_error, fromHandlerError(protocol_error)); | |
| 384 | } | |
| 383 | 385 | try testing.expectEqual(BridgeError.PlatformNotSupported, fromHandlerError(error.UnsupportedPlatform)); |
| 384 | 386 | try testing.expectEqual(BridgeError.AllocationFailed, fromHandlerError(error.OutOfMemory)); |
| 385 | 387 | try testing.expectEqual(BridgeError.NativeCallFailed, fromHandlerError(error.NotInTheProtocol)); |
| @@ -822,6 +822,15 @@ test "an action the namespace does not serve is reported, not ignored" { | ||
| 822 | 822 | } |
| 823 | 823 | } |
| 824 | 824 | |
| 825 | test "both system presenter lookups report a missing webview explicitly" { | |
| 826 | if (!is_darwin) return error.SkipZigTest; | |
| 827 | const saved = ios_dispatch.getWebView(); | |
| 828 | defer ios_dispatch.setWebView(saved); | |
| 829 | ios_dispatch.setWebView(null); | |
| 830 | try testing.expectError(bridge_error.BridgeError.WebViewHandleNotSet, uikit.topmostViewController()); | |
| 831 | try testing.expectError(bridge_error.BridgeError.WebViewHandleNotSet, uikit.keyWindowScene()); | |
| 832 | } | |
| 833 | ||
| 825 | 834 | test "openURL without a url reports it rather than settling nothing" { |
| 826 | 835 | // Swift's `if let` fell through here and never resolved or rejected the |
| 827 | 836 | // callback, so the page waited out a 30-second timeout with no cause to |
| @@ -764,6 +764,10 @@ test "with no host shim present, the hand-off declines rather than failing" { | ||
| 764 | 764 | } |
| 765 | 765 | |
| 766 | 766 | test "a handler error reaches the page as something the protocol can say" { |
| 767 | inline for (@typeInfo(bridge_error.BridgeError).error_set.error_names.?) |name| { | |
| 768 | const protocol_error = @field(bridge_error.BridgeError, name); | |
| 769 | try testing.expectEqual(protocol_error, asBridgeError(protocol_error)); | |
| 770 | } | |
| 767 | 771 | // A handler's error set is wider than BridgeError. Whatever it raises still |
| 768 | 772 | // has to arrive as an error code, because the alternative is a handler that |
| 769 | 773 | // fails silently and leaves the caller on a promise that never settles. |
| @@ -790,6 +794,31 @@ test "a handler error reaches the page as something the protocol can say" { | ||
| 790 | 794 | ); |
| 791 | 795 | } |
| 792 | 796 | |
| 797 | test "share without a webview keeps its specific error in the correlated reply" { | |
| 798 | if (!builtin.target.os.tag.isDarwin()) return error.SkipZigTest; | |
| 799 | const saved = getWebView(); | |
| 800 | defer setWebView(saved); | |
| 801 | setWebView(null); | |
| 802 | ||
| 803 | var system = bridge_mobile_system.SystemBridge.init(testing.allocator); | |
| 804 | defer system.deinit(); | |
| 805 | system.handleMessage("share", "{\"text\":\"fixture\"}") catch |err| { | |
| 806 | const narrowed = asBridgeError(err); | |
| 807 | try testing.expectEqual(bridge_error.BridgeError.WebViewHandleNotSet, narrowed); | |
| 808 | var context = bridge_error.ErrorContext.init(narrowed, "share", bridge_error.errorMessage(narrowed)); | |
| 809 | context.request_id = 71; | |
| 810 | const json = try context.toJSON(testing.allocator); | |
| 811 | defer testing.allocator.free(json); | |
| 812 | const parsed = try std.json.parseFromSlice(std.json.Value, testing.allocator, json, .{}); | |
| 813 | defer parsed.deinit(); | |
| 814 | try testing.expectEqualStrings("WEBVIEW_HANDLE_NOT_SET", parsed.value.object.get("code").?.string); | |
| 815 | try testing.expectEqualStrings("share", parsed.value.object.get("action").?.string); | |
| 816 | try testing.expectEqual(@as(i64, 71), parsed.value.object.get("id").?.integer); | |
| 817 | return; | |
| 818 | }; | |
| 819 | return error.ShareSucceededWithoutWebView; | |
| 820 | } | |
| 821 | ||
| 793 | 822 | test "a request id survives the trip out to the host and back" { |
| 794 | 823 | // The shim may answer asynchronously, by which point the dispatch frame is |
| 795 | 824 | // gone and `request_context.current()` is empty. The id is therefore read |