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.

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

Firestore blows up on GET {source: "cache"} if document exists in cache #1632

Closed
scottmas opened this issue Aug 3, 2018 · 14 comments
Closed
Assignees
Milestone

Comments

@scottmas
Copy link

@scottmas scottmas commented Aug 3, 2018

Issue

When the data gets in a certain state (I don't know how), when I request a document with {source: 'cache'} that doesn't exist in the cache, the library blows up with the following error. While I am using React Native, the RN Firebase folks have triaged the issue and are relatively confident it's not on their end.

Error follows:

'NSInvalidArgumentException', reason: '-[FSTDeletedDocument data]: unrecognized selector sent to instance 0x600000244230'
*** First throw call stack:
(
	0   CoreFoundation                      0x0000000110da91e6 __exceptionPreprocess + 294
	1   libobjc.A.dylib                     0x000000010f695031 objc_exception_throw + 48
	2   CoreFoundation                      0x0000000110e2a784 -[NSObject(NSObject) doesNotRecognizeSelector:] + 132
	3   CoreFoundation                      0x0000000110d2b898 ___forwarding___ + 1432
	4   CoreFoundation                      0x0000000110d2b278 _CF_forwarding_prep_0 + 120
	5   OllieApp                            0x000000010c2783de -[FIRDocumentSnapshot dataWithServerTimestampBehavior:] + 238
	6   OllieApp                            0x000000010c2782df -[FIRDocumentSnapshot data] + 47
	7   OllieApp                            0x000000010ca82911 +[RNFirebaseFirestoreDocumentReference snapshotToDictionary:] + 305
	8   OllieApp                            0x000000010ca816b7 __62-[RNFirebaseFirestoreDocumentReference get:resolver:rejecter:]_block_invoke + 151
	9   OllieApp                            0x000000010c2d3883 __59-[FSTFirestoreClient getDocumentFromLocalCache:completion:]_block_invoke + 499
	10  OllieApp                            0x000000010c2b3504 _ZZ34-[FSTDispatchQueue dispatchAsync:]ENK3$_1clEv + 36
	11  OllieApp                            0x000000010c2b34cd _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZ34-[FSTDispatchQueue dispatchAsync:]E3$_1EEEvDpOT_ + 45
	12  OllieApp                            0x000000010c2b3299 _ZNSt3__110__function6__funcIZ34-[FSTDispatchQueue dispatchAsync:]E3$_1NS_9allocatorIS2_EEFvvEEclEv + 41
	13  OllieApp                            0x000000010c2b0c8b _ZNKSt3__18functionIFvvEEclEv + 123
	14  OllieApp                            0x000000010c24d27c _ZN8firebase9firestore4util10AsyncQueue15ExecuteBlockingERKNSt3__18functionIFvvEEE + 476
	15  OllieApp                            0x000000010c24fc07 _ZZN8firebase9firestore4util10AsyncQueue4WrapERKNSt3__18functionIFvvEEEENK3$_0clEv + 39
	16  OllieApp                            0x000000010c24fbcd _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN8firebase9firestore4util10AsyncQueue4WrapERKNS_8functionIFvvEEEE3$_0EEEvDpOT_ + 45
	17  OllieApp                            0x000000010c24f989 _ZNSt3__110__function6__funcIZN8firebase9firestore4util10AsyncQueue4WrapERKNS_8functionIFvvEEEE3$_0NS_9allocatorISB_EES7_EclEv + 41
	18  OllieApp                            0x000000010c2b0c8b _ZNKSt3__18functionIFvvEEclEv + 123
	19  OllieApp                            0x000000010c25db01 _ZZN8firebase9firestore4util8internal13DispatchAsyncEPU28objcproto17OS_dispatch_queue8NSObjectONSt3__18functionIFvvEEEENK3$_0clEPv + 33
	20  OllieApp                            0x000000010c25dad8 _ZZN8firebase9firestore4util8internal13DispatchAsyncEPU28objcproto17OS_dispatch_queue8NSObjectONSt3__18functionIFvvEEEEN3$_08__invokeEPv + 24
	21  libdispatch.dylib                   0x0000000114bf87ec _dispatch_client_callout + 8
	22  libdispatch.dylib                   0x0000000114c00be5 _dispatch_queue_serial_drain + 1305
	23  libdispatch.dylib                   0x0000000114c014fa _dispatch_queue_invoke + 328
	24  libdispatch.dylib                   0x0000000114bfd344 _dispatch_queue_override_invoke + 726
	25  libdispatch.dylib                   0x0000000114c0436c _dispatch_root_queue_drain + 664
	26  libdispatch.dylib                   0x0000000114c04076 _dispatch_worker_thread3 + 132
	27  libsystem_pthread.dylib             0x000000011511d1ca _pthread_wqthread + 1387
	28  libsystem_pthread.dylib             0x000000011511cc4d start_wqthread + 13
)

The firestore debug logs are not helpful:

Committing transaction: <LevelDbTransaction Start MutationQueue: 0 changes (0 bytes):>
Committing transaction: <LevelDbTransaction NextMutationBatchAfterBatchID: 0 changes (0 bytes):>
Committing transaction: <LevelDbTransaction ReadDocument: 0 changes (0 bytes):>
-[FSTDeletedDocument data]: unrecognized selector sent to instance 0x60c00005fd40
...(The above error follows)

Steps to reproduce:

  1. Get your data in a corrupted state. Not sure how.
  2. Run the following code:
await firebase.firestore()
    .collection('my-collection')
    .doc('some-doc-id')
    .get({ source: 'server' });

await firebase.firestore()
    .collection('my-collection')
    .doc('some-doc-id')
    .get({ source: 'cache' })

//Native exception thrown...

Note: After nuking local data, the error goes away and the request happens just fine.

Environment

IOS
macOS Sierra
RN 54
RN FIrebase 4.3.5
Firestore 5.4.1 in
Not using Typescript.

@scottmas
Copy link
Author

@scottmas scottmas commented Aug 3, 2018

Additional info: I can clean up the corrupted data by simply doing another request with source: 'server'. Then once I request source: "cache" again, it works fine.

So it's likely some discrepancy between the local database and the server.

@wilhuff
Copy link
Contributor

@wilhuff wilhuff commented Aug 3, 2018

Given step 1 of "Get your data in a corrupted state. Not sure how" this isn't going to be a terribly actionable bug.

Please try to narrow this down somehow. Does this happen with a certain kind of data? Duration?

Worst case, could you get a dump of the leveldb database if you find a document that has this behavior? We'll need the dump and a full name of the document (or the query you're running that's hitting it).

If you're running in the simulator on a recent Xcode you can find the database in a location matching this shell glob:

~/Library/Developer/CoreSimulator/Devices/*/data/Containers/Data/Application/*/Documents/firestore/__FIRAPP_DEFAULT/*/main

@scottmas
Copy link
Author

@scottmas scottmas commented Aug 6, 2018

I have a dump of the database and the query. I just sent it to one of the Firestore core team members.

@wilhuff wilhuff assigned mikelehen and unassigned scottmas Aug 6, 2018
mikelehen pushed a commit that referenced this issue Aug 6, 2018
get with source='cache' was not properly handling cached deleted documents.
@mikelehen
Copy link
Contributor

@mikelehen mikelehen commented Aug 6, 2018

Thanks for the report! I've reproduced the issue and believe I have a fix (see referenced PR above).

mikelehen added a commit that referenced this issue Aug 6, 2018
get with source='cache' was not properly handling cached deleted documents.
@scottmas
Copy link
Author

@scottmas scottmas commented Aug 6, 2018

Cool! Does this bug affect Android too? I'm not certain but I'm relatively sure I've experienced it on Android as well.

@mikelehen
Copy link
Contributor

@mikelehen mikelehen commented Aug 6, 2018

@scottmas It shouldn't crash, but it will probably give you a failed Task rejected with an error saying "Failed to get document from cache." rather than giving you a snapshot with .exists() false. I'm working on porting the fix to web / Android right now.

@scottmas
Copy link
Author

@scottmas scottmas commented Aug 6, 2018

Okay great, what error code does it raise? I'm currently handling the code firestore/unavailable which is the code I'm currently handling when the document doesn't exist in the cache (in a non-blow-up scenario).

@mikelehen
Copy link
Contributor

@mikelehen mikelehen commented Aug 6, 2018

Yes, that is the correct code although I'm not sure if/when you'd encounter it as a string firestore/unavailable. On Android you'd be checking .getCode() against FirebaseFirestoreException.Code.UNAVAILABLE I believe.

@scottmas
Copy link
Author

@scottmas scottmas commented Aug 6, 2018

I'm using react-native-firebase, so they must do some translation layer like that.

@mikelehen
Copy link
Contributor

@mikelehen mikelehen commented Aug 10, 2018

Closing this as da1c534 should resolve this. It'll be included in our next release.

@mikelehen mikelehen closed this Aug 10, 2018
@morganchen12 morganchen12 added this to the 5.6.0 milestone Aug 10, 2018
@NessDan
Copy link

@NessDan NessDan commented Sep 30, 2019

@mikelehen I'm having a similar problem but with the add method.

If you load the page "Online" but switch to "Offline" and run this code, it won't ever throw an error:

try {
    var db = firebase.firestore();
    db.collection("emails")
    .add({
        email: emailVal
    })
    .then(function(docRef) {
        console.log("Document written with ID: ", docRef.id);
    })
    .catch(handleError);
} catch (e) {
    handleError(e);
}

@mikelehen
Copy link
Contributor

@mikelehen mikelehen commented Sep 30, 2019

@NessDan That sounds like expected behavior. The Firestore SDK is designed to work seamlessly offline and online, so when you perform write operations offline, they are queued until the client comes back online and then they're sent.

If you want your writes to fail while offline, one option is to do them via the transaction API, since it is not designed to work offline.

@NessDan
Copy link

@NessDan NessDan commented Sep 30, 2019

@mikelehen Thanks for the heads up! I'm assuming that, if someone tried to run the .add() method and left the site before the request went through, it'd never send out correct?

@mikelehen
Copy link
Contributor

@mikelehen mikelehen commented Sep 30, 2019

@NessDan That's correct. If this is a concern (the data is critical), you might want a UI indicator to let the user know that the data hasn't been written yet, e.g.:

collection.add(...).then(() => {
  clearUIIndicator();
});
showUIIndicator("Saving...  [make sure you are online]");

Though you might want to add a small delay (200ms or something) before you show the UI indicator so that it doesn't flash in/out when you are online. All depends on the UX you are going for.

@firebase firebase locked and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants