-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Firestore]: 'Document path cannot be empty.' Error results in an app crash. #14605
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.
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
Comments
Thanks for the bug report and the thorough investigation. @MichaelVerdon: Could you clarify your "expected behavior" when the Swift code snippet from the OP is executed? Namely, how would you like to see this "Document path cannot be empty." be reported by the Firestore SDK? For reference, here is the line of code where the exception is being thrown:
One immediate concern that I have is that Firestore uses its I almost want to say that if Objective-C exceptions crash a Flutter application, then fireflutter may have to wrap each call into the Firestore SDK inside a |
Hey there @dconeybe thank you for your response. This is what i was hoping in terms of behaviour.
Something to add, is that on the Android side of things. The errors are caught and do not result in an app crash which is exactly what we want and the crash only occurs on iOS telling me it is iOS specific. |
I looked into Android's behavior and it's not exactly ideal either: it does allow you to create a My recommendation is to add some checks to the fireflutter sdk before it calls down to the underlying Firestore SDK. Namely, any method that takes a document path should do some validity checks on it before calling down to the underling Firestore SDK, such as verifying that it is non-empty. A good list of checks to perform would be to grep through the This situation is, admittedly, sub-optimal; however, I'm not even sure what an "optimal" solution would look like. Throwing an objective-c exception actually seems like a reasonable thing to do for an iOS SDK (as the firebase-ios-sdk is currently doing and causing an issue for you). IMO the Android SDK should probably throw an exception just like iOS does; however, I'm hesitant to make that change for fear of needlessly breaking existing apps. |
Hey @MichaelVerdon. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically. If you have more information that will help us get to the bottom of this, just add a comment! |
Hey there @dconeybe, thanks for your response, so I take it you recommend we solve it on FlutterFire instead? Can you please elaborate on what you mean as I am unsure I understand? |
Hi @MichaelVerdon. My apologies for being unclear in my previous response. My suggestion is to perform validation of strings in the dart/flutter sdk ("fireflutter") before passing those strings down to the native Firestore SDK. For example, for strings that represent document paths or collection paths, verify the following:
If one or more of these requirements are found to be invalid, then the flutter/dart sdk should report the error in an appropriate way (for example, throw a dart exception or call an error callback). Only if these requirements are validated should the string then be passed down to the native Firestore SDK. Does this sound like something you could reasonably do in your dart/flutter sdk? |
Description
Hi there guys, I am coming over here from FlutterFire after we received this report: firebase/flutterfire#17196 where firestore related errors such as 'Document path cannot be empty.' appear to be caught but crash the application and I don't believe it should be doing that. As well as being able to reproduce this only on iOS, I was able to reproduce this using the native-sdk here. I believe the fix might be simple as I made a fix FlutterFire side for reference here but we believe it would be better addressed here as it is only a temporary solution for FlutterFire.
Reproducing the issue
Steps for reproduction:
Firebase SDK Version
11.10.0
Xcode Version
16.1
Installation Method
Swift Package Manager
Firebase Product(s)
Firestore
Targeted Platforms
iOS
Relevant Log Output
If using Swift Package Manager, the project's Package.resolved
Expand
Package.resolved
snippetReplace this line with the contents of your Package.resolved.
If using CocoaPods, the project's Podfile.lock
Expand
Podfile.lock
snippetReplace this line with the contents of your Podfile.lock!
The text was updated successfully, but these errors were encountered: