[go: nahoru, domu]
Skip to content
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

Jump to bottom

Firestore db.runTransaction() async causes nil unwrapping fatal error if no data is returned #9426

Closed
ionothanus opened this issue Mar 8, 2022 · 4 comments · Fixed by #9502
Closed

Firestore db.runTransaction() async causes nil unwrapping fatal error if no data is returned #9426

ionothanus opened this issue Mar 8, 2022 · 4 comments · Fixed by #9502
Assignees
@morganchen12 @schmidt-sebastian
Labels
api: firestore Swift API
Milestone
Firebase 9

Comments

@ionothanus
Copy link

[REQUIRED] Step 1: Describe your environment

  • Xcode version: 13.2.1
  • Firebase SDK version: 8.10.0
  • Installation method: Swift Package Manager
  • Firebase Component: Firestore
  • Target platform(s): iOS 15.2

[REQUIRED] Step 2: Describe the problem

Steps to reproduce:

  1. Start a transaction using the following async declaration of db.runTransaction():
func runTransaction(_ updateBlock: @escaping (Transaction, NSErrorPointer) -> Any?) async throws -> Any
  1. Successfully perform the transaction, without encountering or returning an error (i.e., leave error?.pointee unset).
  2. return nil at the end of the transaction update block, as demonstrated in the basic transaction example in the documentation.

Expected result: the transaction is completed with a nil return value.
Actual result: the error Thread 1: Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value is encountered in functional line 1885.

Partial stack trace:

Thread 1 Queue : com.apple.main-thread (serial)
#0	0x0000000185918d7c in _swift_runtime_on_report ()
#1	0x0000000185998e54 in _swift_stdlib_reportFatalErrorInFile ()
#2	0x00000001855ce1a8 in closure #1 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) ()
#3	0x00000001855cdf48 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) ()
#4	0x00000001855cd83c in _assertionFailure(_:_:file:line:flags:) ()
#5	0x00000001855ce254 in _diagnoseUnexpectedNilOptional(_filenameStart:_filenameLength:_filenameIsASCII:_line:_isImplicitUnwrap:) ()
#6	0x0000000104866e3c in @objc completion handler block implementation for @escaping @callee_unowned @convention(block) (@unowned Swift.AnyObject?, @unowned NSError?) -> () with result type Any ()
#7	0x0000000104ca95e8 in std::__1::__function::__value_func<void (firebase::firestore::util::Status)>::operator()(firebase::firestore::util::Status&&) const [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/functional:1885
#8	0x0000000104ca95d0 in std::__1::function<void (firebase::firestore::util::Status)>::operator()(firebase::firestore::util::Status) const [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/functional:2560
#9	0x0000000104ca95d0 in firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()::operator()() const [inlined] at /Users/ionothanus/Library/Developer/Xcode/DerivedData/Detour_On_Route-agyrglamqsjpfrcxugnkspranqcq/SourcePackages/checkouts/firebase-ios-sdk/Firestore/core/src/core/firestore_client.cc:502
#10	0x0000000104ca95b8 in decltype(std::__1::forward<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()&>(fp)()) std::__1::__invoke<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()&>(firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()&) [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/type_traits:3694
#11	0x0000000104ca95b8 in void std::__1::__invoke_void_return_wrapper<void, true>::__call<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()&>(firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()&) [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/__functional_base:348
#12	0x0000000104ca95b8 in std::__1::__function::__alloc_func<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'(), std::__1::allocator<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()>, void ()>::operator()() [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/functional:1558
#13	0x0000000104ca95b8 in std::__1::__function::__func<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'(), std::__1::allocator<firebase::firestore::core::FirestoreClient::Transaction(int, std::__1::function<void (std::__1::shared_ptr<firebase::firestore::core::Transaction>, std::__1::function<void (firebase::firestore::util::Status)>)>, std::__1::function<void (firebase::firestore::util::Status)>)::$_1::operator()(firebase::firestore::util::Status) const::'lambda'()>, void ()>::operator()() at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/functional:1732
#14	0x0000000104d6b644 in std::__1::__function::__value_func<void ()>::operator()() const [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/functional:1885
#15	0x0000000104d6b630 in std::__1::function<void ()>::operator()() const [inlined] at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS15.2.sdk/usr/include/c++/v1/functional:2560
#16	0x0000000104d6b630 in firebase::firestore::util::Task::ExecuteAndRelease() at /Users/ionothanus/Library/Developer/Xcode/DerivedData/Detour_On_Route-agyrglamqsjpfrcxugnkspranqcq/SourcePackages/checkouts/firebase-ios-sdk/Firestore/core/src/util/task.cc:102
#17	0x00000001061ca3b4 in _dispatch_client_callout ()
#18	0x00000001061da898 in _dispatch_main_queue_callback_4CF ()
#19	0x0000000180b95d84 in __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ ()
#20	0x0000000180b4ff5c in __CFRunLoopRun ()
#21	0x0000000180b63468 in CFRunLoopRunSpecific ()
#22	0x000000019c70738c in GSEventRunModal ()
#23	0x00000001835065d0 in -[UIApplication _run] ()
#24	0x0000000183284f74 in UIApplicationMain ()
#25	0x0000000188822314 in closure #1 in KitRendererCommon(_:) ()
#26	0x0000000188751508 in runApp<τ_0_0>(_:) ()
#27	0x0000000188732cb8 in static App.main() ()

Relevant Code:

    struct SomeCodableObject: Codable, Identifiable {
        // your fields here...
        @DocumentID var id: String?
    }

    // authenticate with Firebase, initialize the app, etc., etc.

    func updateSubscription(subscription: SomeCodableObject) async throws {
        let docRef = db.collection('someCollection').document('someDocument')
        let _ = try await db.runTransaction({ transaction, error in
            do {
                try transaction.setData(from: subscription, forDocument: docRef)
            } catch let setError as NSError {
                error?.pointee = setError

                // In my testing, returning nil here does not seem to be an issue.
                return nil
            }

            // Returning nil here causes the error.
            return nil
            // Returning a value does not cause the error.
            // return subscription
        })
    }

Workarounds I've found so far:

  1. return a non-nil value at the end of the successful transaction.
  2. don't use the async declaration. If you want async, wrap the continuation yourself with a withCheckedThrowingContinuation() wrapper.
@morganchen12
Copy link
Contributor

Thanks for reporting @ionothanus. This crash occurs because of the way Swift imports async Objective-C methods that return an error. In the case of runTransaction(block:completion:), the value you return from the update block is simply passed through to the completion closure and can be nil for values you'd like to ignore. However, Swift imports the success value in the resulting async method as a nonnull type, which causes a crash if you return nil.

There's a couple ways we could fix this, but they would all be breaking changes:

  • Check if the updateBlock's return value is nil and return NSNull instead. This would break implementations that depend on the return value being nil in Objective-C, or Swift implementations with the same behavior that do not use the async method.
  • Remove the async method from the API.
  • Disallow returning nil in the update block.

I'll figure out which path we want to pursue and then update this thread.

@morganchen12 morganchen12 mentioned this issue Mar 22, 2022
Merged
@morganchen12
Copy link
Contributor
morganchen12 commented Mar 22, 2022 •
edited
Loading

Hi @ionothanus, sorry for the slow update. I've found a workaround that may not require a breaking change. Can you test the changes in #9502?

Looking at the original importing behavior, the return type is imported as Any, which by my understanding should not cause a crash if it's nil because nil is representable by Any, so it's unclear to me what's actually going on in Swift internals.

@ionothanus
Copy link
Author

No problem, it's not blocking me. #9502 doesn't appear to have resolved the issue for me, I can still reproduce the issue with the method demonstrated above. (FWIW, it's my first time trying to include a branch rather than a released version for this project, but I pointed my XCode/SwiftPM dependency at your branch, and as far as I can tell it's using the right commit.)

@morganchen12
Copy link
Contributor

Ok, looks like this will have to be a breaking change. Thanks for testing!

@morganchen12 morganchen12 self-assigned this Mar 24, 2022
@morganchen12 morganchen12 added this to the Firebase 9 milestone Mar 29, 2022
@firebase firebase locked and limited conversation to collaborators May 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@morganchen12 morganchen12

@schmidt-sebastian schmidt-sebastian

Labels
api: firestore Swift API
Projects
None yet
Milestone
Firebase 9
Development

Successfully merging a pull request may close this issue.

Fix nil async/await import error firebase/firebase-ios-sdk
5 participants
@paulb777 @morganchen12 @schmidt-sebastian @google-oss-bot @ionothanus