-
Notifications
You must be signed in to change notification settings - Fork 4k
[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
[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.
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.
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 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.///
).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?