[cloud_firestore] Fixes crash on Android when run a transaction - #87
Conversation
|
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 What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
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.
|
Nice work :) |
|
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 35io.flutter.embedding.engine.dart.DartMessenger$Reply.replyThis PR resolves #102. |
|
@collinjackson Is there a reason this is not being merged? |
|
Guys, do you have any progress?! |
|
Just waiting 👍 |
Just as a reminder, you can always use it like that: |
|
Fixed, thanks guys! |
…base#87) * Fix transaction crash on Android device * Update pub version * Bump for release * Reformat * Fix crash
|
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.my android/app/build.gradle (Snapshot)
Pubspec (Relevant plugins)
Flutter doctor
|
|
Same here:
|
|
@fenchai23 Transactions are supposed to fail when offline; as it appears that your device was. |
Description
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:will also be called
→ result#success has been called after result#error
→ This cause
java.lang.IllegalStateException: Reply already submittedRelated 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.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?