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

Error when get document by firestore #533

Closed
arvitaly opened this issue Oct 20, 2017 · 13 comments
Closed

Error when get document by firestore #533

arvitaly opened this issue Oct 20, 2017 · 13 comments

Comments

@arvitaly
Copy link

arvitaly commented Oct 20, 2017

Issue

App crashed

import firebase from "react-native-firebase";
firebase.firestore().collection("test").doc("124").get().then((ref)=>{ console.log(ref) });
java.lang.ExceptionInInitializerError
 at io.invertase.firebase.firestore.RNFirebaseFirestoreDocumentReference$2.onComplete(RNFirebaseFirestoreDocumentReference.java:64)

Environment

  1. Application Target Platform: Android
  1. Development Operating System: Window Server 2016
  1. Build Tools: react-native run-android
  1. React Native version: "react-native": "0.49.3",
  1. RNFirebase Version: "react-native-firebase": "^3.0.4",
  1. Firebase Module: firestore
@chrisbianca
Copy link
Contributor

@arvitaly Can you send us the entire stack trace, not just the first couple of lines?

@arvitaly
Copy link
Author

Yes, of course

 java.lang.ExceptionInInitializerError
       at io.invertase.firebase.firestore.RNFirebaseFirestoreDocumentReference$2.onComplete(RNFirebaseFirestoreDocumentReference.java:65)
       at com.google.android.gms.tasks.zzf.run(Unknown Source)
       at android.os.Handler.handleCallback(Handler.java:739)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at android.os.Looper.loop(Looper.java:148)
       at android.app.ActivityThread.main(ActivityThread.java:5451)
       at java.lang.reflect.Method.invoke(Native Method)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
 Caused by: java.lang.IllegalArgumentException: Unknown pattern character 'X'
       at java.text.SimpleDateFormat.validatePatternCharacter(SimpleDateFormat.java:323)
       at java.text.SimpleDateFormat.validatePattern(SimpleDateFormat.java:312)
       at java.text.SimpleDateFormat.<init>(SimpleDateFormat.java:365)
       at java.text.SimpleDateFormat.<init>(SimpleDateFormat.java:258)
       at io.invertase.firebase.firestore.FirestoreSerialize.<clinit>(FirestoreSerialize.java:33)

@Ehesp
Copy link
Member

Ehesp commented Oct 20, 2017

@chrisbianca seems to be related to the issue you mentioned on Discord "Unknown pattern character 'X'"

@urajat
Copy link
Contributor

urajat commented Oct 23, 2017

@chrisbianca In commit fc3fc6d , the Z which I had introduced as a fix for #505 has been replaced with X. Not sure if this was deliberate. This is actually a bug - the single character X discards the minutes in the timezone and hence instead of 2017-10-18T01:12:43.274+0530 for IST, I see 2017-10-18T01:12:43.274+05 which is wrong! In console.log for these values, I am getting Invalid Date errors. new Date().toISOString() in Chrome console returns a value with Z in it. This issue and the timezone one likely have the same root cause - happening because of the same change. I can open a separate bug for this if you want.

@chrisbianca
Copy link
Contributor

@urajat Yes, I introduced this to fix a failing test in our test suite. However it seems that this failing test is caused by the fact that the Android emulator uses a different version of Java to an actual device!! In Java SDK 7+ (which I hadn't realised was only used in the emulator), X is the correct way of dealing with an ISO timezone, but it seems this does not work on an actual device. I'm just debugging at the moment to confirm that everything is correct and will push up a fix followed by a new release shortly.

@urajat
Copy link
Contributor

urajat commented Oct 23, 2017

@chrisbianca Checked https://developer.android.com/reference/java/text/SimpleDateFormat.html#iso8601timezone In this case, using XX should help. I am testing on the emulator where the single X works, but discards the timezone minutes. With XX or XXX, the values are interpreted correctly. XX appears to be the direct equivalent of Z.

@chrisbianca
Copy link
Contributor

@urajat Thanks for this. Can you do me a favour and see what happens when you run with XX on a device? According to this open bug, even though it states that X is supported, it doesn't actually work on most versions of Android: https://issuetracker.google.com/issues/65350732. Instead you get the error that's cited above

@urajat
Copy link
Contributor

urajat commented Oct 23, 2017

@chrisbianca I just checked with a Moto X Play device with Android 6.0.1. No combination of X works - X, XX, XXX - throws the Unknown pattern character 'X' exception. The Z option works correctly on the device too. According to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString, the toISOString method returns Z too - so the test case that you have should ideally pass even while using Z?

@chrisbianca
Copy link
Contributor

@urajat The Z option seems to work correctly when sending dates from Native to JS, but when reading dates that are sent from JS to Native I get a ParseException: Unparseable date error.

The test case we have that is failing sets a date from the JS side into Firestore, then reads it back out to check they're the same. It looks like Javascript toISOString() always sends the date as UTC time which is why the 'Z' parameter with apostrophes worked for parsing the dates. It looks like we might need to use different date formats for reading (JS to Native) compared to writing (Native to JS). I'm just struggling to figure out what they should be!

@chrisbianca
Copy link
Contributor

@urajat Ok, I think I have figured it out, though because I'm in the GMT/BST timezone, it's always an easier one to account for! Are you able to try out this commit and see whether it works for you:

2c7c768

You should be able to install directly by doing the following:

npm install --save https://github.com/invertase/react-native-firebase#v3.0.x

@chrisbianca
Copy link
Contributor

This has now been released as part of version 3.0.5

@urajat
Copy link
Contributor

urajat commented Oct 23, 2017

@chrisbianca Just got around to testing this. 3.0.5 works on the emulator (tested on Android 7.1.1). But on the device with 7.1.1, this doesn't work - I get an Invalid Date value for the dates which are being read even though they work perfectly on the emulator and are correct on Firestore too. I added debugging statements in buildTypeMap to compare the values on device vs emulator - there is no change and the parsing is working as expected. I even tried going back to 3.0.4 and manually making the 'X' to 'Z' change to see if that helps, but nope - same Invalid Date issue on device. In the morning, I was testing with a 6.0.1 device where the Z option worked. Not sure if there's some other place where there is some date interpretation happening.

@chrisbianca
Copy link
Contributor

I'm moving this over to a new issue: #545 as the initial bug is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants