Skip to content

[cloud_firestore] Fixes crash on Android when run a transaction #87

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

Merged

Conversation

hoc081098
Copy link
Contributor

@hoc081098 hoc081098 commented Sep 4, 2019

Description

  • Fixes crash on Android when run a transaction and without internet connection.
  • This occurred because:

Tasks.await(transactionTCSTask, timeout, TimeUnit.MILLISECONDS)
throws a Exception
result.error("Error performing transaction", e.getMessage(), null) will be called
→ transaction result is null
→ in addOnCompleteListener#onComplete:

    if (task.isSuccessful()) { // result is null
      result.success(task.getResult());
    }

will also be called
result#success has been called after result#error
→ This cause java.lang.IllegalStateException: Reply already submitted

Related Issues

fix #79

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • If the pull request affects only one plugin, the PR title starts with the name of the plugin in brackets (e.g. [cloud_firestore])
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@hoc081098
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Contributor

@collinjackson collinjackson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The cloud_firestore integration test crashed on this PR but fortunately the fix was easy. I've updated your PR with 928e052. (Yay tests!)

09-04 14:33:35.278  5207  5207 E AndroidRuntime: FATAL EXCEPTION: main
09-04 14:33:35.278  5207  5207 E AndroidRuntime: Process: io.flutter.plugins.firebase.firestoreexample, PID: 5207
09-04 14:33:35.278  5207  5207 E AndroidRuntime: java.lang.IllegalArgumentException: Unsupported value: io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$TransactionResult@d350ae3
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugin.common.StandardMessageCodec.writeValue(StandardMessageCodec.java:294)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugins.firebase.cloudfirestore.FirestoreMessageCodec.writeValue(CloudFirestorePlugin.java:886)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugin.common.StandardMethodCodec.encodeSuccessEnvelope(StandardMethodCodec.java:57)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler$1.success(MethodChannel.java:225)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$3.onComplete(CloudFirestorePlugin.java:446)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at com.google.android.gms.tasks.zzj.run(Unknown Source:4)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:873)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:99)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:193)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:6669)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
diff --git a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
index 71029b6a..eddabe3a 100644
--- a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
+++ b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
@@ -443,7 +443,7 @@ public class CloudFirestorePlugin implements MethodCallHandler {
 
                       TransactionResult transactionResult = task.getResult();
                       if (transactionResult.exception == null) {
-                        result.success(transactionResult);
+                        result.success(transactionResult.result);
                       } else {
                         result.error(
                             "Error performing transaction",

I also made some minor fixes to the version number and reformatting.

@hoc081098
Copy link
Contributor Author

hoc081098 commented Sep 5, 2019

Nice work :)

@creativecreatorormaybenot
Copy link
Contributor

creativecreatorormaybenot commented Sep 7, 2019

Wow. Thanks so much for this PR. I was seeing this crash for quite a while now. In Crashlytics it shows as the following:

DartMessenger.java line 35

io.flutter.embedding.engine.dart.DartMessenger$Reply.reply

This PR resolves #102.
It was just now that I figured out why this was happening as well. Lucky that I saw this PR before finishing my implementation - saving me some work. This shows that it really is benificial to create an issue with a PR or include a stack trace or similar in the PR (or issue) description so that it becomes searchable. I searched for the crash, but could not find anything.

@creativecreatorormaybenot
Copy link
Contributor

@collinjackson Is there a reason this is not being merged?

@Tolik7
Copy link

Tolik7 commented Sep 30, 2019

Guys, do you have any progress?!
I have used cloud_firestore: 0.12.9+4 and I get crash of my app due 'offline' transaction every time!
When it can be retested again? thanks.

@hoc081098
Copy link
Contributor Author

Just waiting 👍

@jorgercg
Copy link

jorgercg commented Oct 4, 2019

Guys, do you have any progress?!
I have used cloud_firestore: 0.12.9+4 and I get crash of my app due 'offline' transaction every time!
When it can be retested again? thanks.

Just as a reminder, you can always use it like that:
cloud_firestore: git: url: https://github.com/FirebaseExtended/flutterfire.git path: packages/cloud_firestore ref: 2e7c183

@collinjackson collinjackson merged commit a24db9c into firebase:master Oct 8, 2019
@Tolik7
Copy link

Tolik7 commented Oct 15, 2019

Fixed, thanks guys!

kroikie pushed a commit to collinjackson/flutterfire that referenced this pull request Nov 15, 2019
…base#87)

* Fix transaction crash on Android device
* Update pub version
* Bump for release
* Reformat
* Fix crash
@bpaul7101
Copy link

I still get this crashing. Only on Android - LIVE devices and also simulators.

Basically occurs when I try and run a transaction when the device is offline

Crashalytics

CloudFirestorePlugin.java line 644

Caused by java.lang.AssertionError: INTERNAL ASSERTION FAILED: A transaction object cannot be used after its update callback has been invoked.

   at com.google.firebase.firestore.util.Assert.fail(com.google.firebase:firebase-firestore@@21.4.0:46)
   at com.google.firebase.firestore.util.Assert.hardAssert(com.google.firebase:firebase-firestore@@21.4.0:31)
   at com.google.firebase.firestore.core.Transaction.ensureCommitNotCalled(com.google.firebase:firebase-firestore@@21.4.0:246)
   at com.google.firebase.firestore.core.Transaction.write(com.google.firebase:firebase-firestore@@21.4.0:241)
   at com.google.firebase.firestore.core.Transaction.set(com.google.firebase:firebase-firestore@@21.4.0:106)
   at com.google.firebase.firestore.Transaction.set(com.google.firebase:firebase-firestore@@21.4.0:91)
   at com.google.firebase.firestore.Transaction.set(com.google.firebase:firebase-firestore@@21.4.0:67)
   at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$7.doInBackground(CloudFirestorePlugin.java:644)
   at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$7.doInBackground(CloudFirestorePlugin.java:638)
   at android.os.AsyncTask$2.call(AsyncTask.java:333)
   at java.util.concurrent.FutureTask.run(FutureTask.java:266)
   at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:245)
   at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
   at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
   at java.lang.Thread.run(Thread.java:764)

my android/app/build.gradle (Snapshot)

dependencies {
testImplementation 'junit:junit:4.12'
androidTestImplementation 'androidx.test:runner :1.1.1'
androidTestImplementation 'androidx.test.espresso:espresso-core: 3.1.1'
implementation 'com.google.firebase:firebase-analytics: 17.2.2'
implementation 'com.google.firebase:firebase-firestore: 21.4.0'
implementation 'com.google.firebase:firebase-crashlytics: 17.0.0-beta01'
}

Pubspec (Relevant plugins)

cloud_firestore: 0.13.3
cloud_functions: 0.4.1+5
google_sign_in: 4.1.4
firebase_auth: 0.15.2
firebase_storage: 3.0.11

Flutter doctor

[✓] Flutter (Channel stable, v1.12.13+hotfix.5, on Mac OS X 10.15.3 19D76, locale en-SG)

[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.0-rc2)
[✓] Xcode - develop for iOS and macOS (Xcode 11.3.1)
[✓] Android Studio (version 3.5)
[✓] IntelliJ IDEA Community Edition (version 2018.2.3)
[✓] VS Code (version 1.42.1)
[✓] Connected device (11 available)

@fenchai23
Copy link

Same here:

E/CloudFirestorePlugin(10092): java.lang.Exception: DoTransaction failed: UNAVAILABLE: Unable to resolve host firestore.googleapis.com E/CloudFirestorePlugin(10092): java.util.concurrent.ExecutionException: java.lang.Exception: DoTransaction failed: UNAVAILABLE: Unable to resolve host firestore.googleapis.com E/CloudFirestorePlugin(10092): at com.google.android.gms.tasks.Tasks.zzb(Unknown Source:61) E/CloudFirestorePlugin(10092): at com.google.android.gms.tasks.Tasks.await(Unknown Source:33) E/CloudFirestorePlugin(10092): at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$4.apply(CloudFirestorePlugin.java:527) E/CloudFirestorePlugin(10092): at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$4.apply(CloudFirestorePlugin.java:479) E/CloudFirestorePlugin(10092): at com.google.firebase.firestore.FirebaseFirestore.lambda$runTransaction$0(com.google.firebase:firebase-firestore@@21.3.0:301) E/CloudFirestorePlugin(10092): at com.google.firebase.firestore.FirebaseFirestore$$Lambda$5.call(Unknown Source:6)

@larssn
Copy link
Contributor

larssn commented Jun 24, 2020

@fenchai23 Transactions are supposed to fail when offline; as it appears that your device was.
But I assumed it hard crashed? It's not supposed to do that :)

@firebase firebase locked and limited conversation to collaborators Aug 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cloud_firestore] Crash app when run transaction on Android device and have no internet connection
9 participants