From bf8534214a4f5cc1b4aa37920756400f91d84989 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Mon, 7 Apr 2025 12:12:07 -0400 Subject: [PATCH 1/5] [Functions] 'call' API should reenter on main thread --- .../ObjCIntegration/FIRIntegrationTests.m | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m b/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m index 124f2eb9044..1d3f88981ee 100644 --- a/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m +++ b/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m @@ -290,4 +290,28 @@ - (void)testTimeout { [self waitForExpectations:@[ expectation ] timeout:10]; } +- (void)testFunctionsReturnsOnMainThread { + XCTestExpectation *expectation = [[XCTestExpectation alloc] init]; + FIRHTTPSCallable *function = [_functions HTTPSCallableWithName:@"scalarTest"]; + [function callWithObject:@17 + completion:^(FIRHTTPSCallableResult *_Nullable result, NSError *_Nullable error) { + XCTAssert(NSThread.isMainThread); + XCTAssertNotNil(result); + [expectation fulfill]; + }]; + [self waitForExpectations:@[ expectation ] timeout:10]; +} + +- (void)testFunctionErrorsOnMainThread { + XCTestExpectation *expectation = [[XCTestExpectation alloc] init]; + FIRHTTPSCallable *function = [_functions HTTPSCallableWithName:@"httpErrorTest"]; + [function callWithObject:@{} + completion:^(FIRHTTPSCallableResult *_Nullable result, NSError *_Nullable error) { + XCTAssert(NSThread.isMainThread); + XCTAssertNotNil(error); + [expectation fulfill]; + }]; + [self waitForExpectations:@[ expectation ] timeout:10]; +} + @end From f9c6adc392816b62e289fcae8a08b9d21f48767b Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Mon, 7 Apr 2025 12:22:18 -0400 Subject: [PATCH 2/5] Add changelog entry --- FirebaseFunctions/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/FirebaseFunctions/CHANGELOG.md b/FirebaseFunctions/CHANGELOG.md index f244f3f58e8..3c5253793be 100644 --- a/FirebaseFunctions/CHANGELOG.md +++ b/FirebaseFunctions/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased +- [fixed] Fix regression from 11.6.0 where `HTTPSCallable` did not invoke + completion block on main thread (#14653). + # 11.10.0 - [added] Streaming callable functions are now supported. From 3b0235a6b62e4e479b16573e7a283f45ca3ffb9a Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Mon, 7 Apr 2025 17:51:13 -0400 Subject: [PATCH 3/5] Add Swift test and fixes --- FirebaseFunctions/Sources/HTTPSCallable.swift | 2 +- .../Tests/Integration/IntegrationTests.swift | 32 +++++++++++++++++++ .../Tests/Unit/FunctionsTests.swift | 2 +- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/FirebaseFunctions/Sources/HTTPSCallable.swift b/FirebaseFunctions/Sources/HTTPSCallable.swift index b423ac4195a..c2bbf4fb985 100644 --- a/FirebaseFunctions/Sources/HTTPSCallable.swift +++ b/FirebaseFunctions/Sources/HTTPSCallable.swift @@ -79,7 +79,7 @@ open class HTTPSCallable: NSObject { completion: @escaping (HTTPSCallableResult?, Error?) -> Void) { if #available(iOS 13, macCatalyst 13, macOS 10.15, tvOS 13, watchOS 7, *) { - Task { + Task { @MainActor in do { let result = try await call(data) completion(result, nil) diff --git a/FirebaseFunctions/Tests/Integration/IntegrationTests.swift b/FirebaseFunctions/Tests/Integration/IntegrationTests.swift index 878cec1c9a0..0fa9c21e862 100644 --- a/FirebaseFunctions/Tests/Integration/IntegrationTests.swift +++ b/FirebaseFunctions/Tests/Integration/IntegrationTests.swift @@ -867,6 +867,38 @@ class IntegrationTests: XCTestCase { XCTAssertEqual(response, expected) } } + + func testFunctionsReturnsOnMainThread() { + let expectation = expectation(description: #function) + functions.httpsCallable( + "scalarTest", + requestAs: Int16.self, + responseAs: Int.self + ).call(17) { result in + guard case .success = result else { + return XCTFail("Unexpected failure.") + } + XCTAssert(Thread.isMainThread) + expectation.fulfill() + } + waitForExpectations(timeout: 5) + } + + func testFunctionsThrowsOnMainThread() { + let expectation = expectation(description: #function) + functions.httpsCallable( + "httpErrorTest", + requestAs: [Int].self, + responseAs: Int.self + ).call([]) { result in + guard case .failure = result else { + return XCTFail("Unexpected failure.") + } + XCTAssert(Thread.isMainThread) + expectation.fulfill() + } + waitForExpectations(timeout: 5) + } } // MARK: - Streaming diff --git a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift index 476fe116853..255b2785451 100644 --- a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift +++ b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift @@ -290,7 +290,7 @@ class FunctionsTests: XCTestCase { } XCTAssertEqual(error as NSError, networkError) - + XCTAssert(Thread.isMainThread) completionExpectation.fulfill() } From 9689fd80b5ff22fb6611fb95c4506577d66ebc0d Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Tue, 8 Apr 2025 16:29:04 -0400 Subject: [PATCH 4/5] More precise approach --- FirebaseFunctions/Sources/HTTPSCallable.swift | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/FirebaseFunctions/Sources/HTTPSCallable.swift b/FirebaseFunctions/Sources/HTTPSCallable.swift index c2bbf4fb985..fa5877ffc3b 100644 --- a/FirebaseFunctions/Sources/HTTPSCallable.swift +++ b/FirebaseFunctions/Sources/HTTPSCallable.swift @@ -79,12 +79,16 @@ open class HTTPSCallable: NSObject { completion: @escaping (HTTPSCallableResult?, Error?) -> Void) { if #available(iOS 13, macCatalyst 13, macOS 10.15, tvOS 13, watchOS 7, *) { - Task { @MainActor in + Task { do { let result = try await call(data) - completion(result, nil) + await MainActor.run { + completion(result, nil) + } } catch { - completion(nil, error) + await MainActor.run { + completion(nil, error) + } } } } else { From bdf79b32bc63f3d95c8543ce8b75ece32bc2f027 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Tue, 8 Apr 2025 16:55:31 -0400 Subject: [PATCH 5/5] Improved approach --- FirebaseFunctions/Sources/HTTPSCallable.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/FirebaseFunctions/Sources/HTTPSCallable.swift b/FirebaseFunctions/Sources/HTTPSCallable.swift index fa5877ffc3b..671a7b67dce 100644 --- a/FirebaseFunctions/Sources/HTTPSCallable.swift +++ b/FirebaseFunctions/Sources/HTTPSCallable.swift @@ -76,19 +76,15 @@ open class HTTPSCallable: NSObject { /// - data: Parameters to pass to the trigger. /// - completion: The block to call when the HTTPS request has completed. @objc(callWithObject:completion:) open func call(_ data: Any? = nil, - completion: @escaping (HTTPSCallableResult?, + completion: @escaping @MainActor (HTTPSCallableResult?, Error?) -> Void) { if #available(iOS 13, macCatalyst 13, macOS 10.15, tvOS 13, watchOS 7, *) { Task { do { let result = try await call(data) - await MainActor.run { - completion(result, nil) - } + await completion(result, nil) } catch { - await MainActor.run { - completion(nil, error) - } + await completion(nil, error) } } } else { @@ -102,9 +98,13 @@ open class HTTPSCallable: NSObject { ) { result in switch result { case let .success(callableResult): - completion(callableResult, nil) + DispatchQueue.main.async { + completion(callableResult, nil) + } case let .failure(error): - completion(nil, error) + DispatchQueue.main.async { + completion(nil, error) + } } } }