Skip to content

Commit f949c23

Browse files
SelaseKaySelaseKay
authored
fix(firestore, android): defer transaction cleanup until stream cancellation (#18553)
Keep the transaction event channel registered until Dart cancels its subscription so successful transactions do not report MissingPluginException.
1 parent 94e36f1 commit f949c23

3 files changed

Lines changed: 63 additions & 22 deletions

File tree

‎packages/cloud_firestore/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/firestore/FlutterFirebaseFirestorePlugin.java‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -277,26 +277,21 @@ private String registerEventChannel(String prefix, String identifier, StreamHand
277277
return identifier;
278278
}
279279

280-
private void removeEventListener(String identifier) {
280+
private void removeTransaction(String transactionId) {
281+
transactions.remove(transactionId);
282+
transactionHandlers.remove(transactionId);
283+
284+
// onCancel invokes this method, so remove the handler without cancelling it again.
285+
synchronized (streamHandlers) {
286+
streamHandlers.remove(transactionId);
287+
}
288+
281289
synchronized (eventChannels) {
282-
EventChannel eventChannel = eventChannels.remove(identifier);
290+
EventChannel eventChannel = eventChannels.remove(transactionId);
283291
if (eventChannel != null) {
284292
eventChannel.setStreamHandler(null);
285293
}
286294
}
287-
288-
synchronized (streamHandlers) {
289-
StreamHandler streamHandler = streamHandlers.remove(identifier);
290-
if (streamHandler != null) {
291-
streamHandler.onCancel(null);
292-
}
293-
}
294-
}
295-
296-
private void removeTransaction(String transactionId) {
297-
transactions.remove(transactionId);
298-
removeEventListener(transactionId);
299-
transactionHandlers.remove(transactionId);
300295
}
301296

302297
private void removeEventListeners() {

‎packages/cloud_firestore/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/firestore/streamhandler/TransactionStreamHandler.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ public interface OnTransactionStartedListener {
3838
void onStarted(Transaction transaction);
3939
}
4040

41-
/** Callback when the transaction has reached a terminal state. */
42-
public interface OnTransactionCompleteListener {
43-
void onComplete(String transactionId);
41+
/** Callback when Dart cancels the transaction event stream. */
42+
public interface OnTransactionCancelledListener {
43+
void onCancelled(String transactionId);
4444
}
4545

4646
final OnTransactionStartedListener onTransactionStartedListener;
47-
final OnTransactionCompleteListener onTransactionCompleteListener;
47+
final OnTransactionCancelledListener onTransactionCancelledListener;
4848
final FirebaseFirestore firestore;
4949
final String transactionId;
5050
final Long timeout;
@@ -53,13 +53,13 @@ public interface OnTransactionCompleteListener {
5353

5454
public TransactionStreamHandler(
5555
OnTransactionStartedListener onTransactionStartedListener,
56-
OnTransactionCompleteListener onTransactionCompleteListener,
56+
OnTransactionCancelledListener onTransactionCancelledListener,
5757
FirebaseFirestore firestore,
5858
String transactionId,
5959
Long timeout,
6060
Long maxAttempts) {
6161
this.onTransactionStartedListener = onTransactionStartedListener;
62-
this.onTransactionCompleteListener = onTransactionCompleteListener;
62+
this.onTransactionCancelledListener = onTransactionCancelledListener;
6363
this.firestore = firestore;
6464
this.transactionId = transactionId;
6565
this.timeout = timeout;
@@ -186,14 +186,18 @@ public void onListen(Object arguments, EventSink events) {
186186
() -> {
187187
events.success(map);
188188
events.endOfStream();
189-
onTransactionCompleteListener.onComplete(transactionId);
190189
});
191190
});
192191
}
193192

194193
@Override
195194
public void onCancel(Object arguments) {
196195
semaphore.release();
196+
// FlutterFirebaseFirestorePlugin passes null when disposing all listeners and clears the
197+
// listener maps itself. A non-null value identifies Dart's EventChannel cancellation.
198+
if (arguments != null) {
199+
onTransactionCancelledListener.onCancelled(transactionId);
200+
}
197201
}
198202

199203
@Override

‎packages/cloud_firestore/cloud_firestore/example/integration_test/transaction_e2e.dart‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:math';
66

77
import 'package:cloud_firestore/cloud_firestore.dart';
88
import 'package:flutter/foundation.dart';
9+
import 'package:flutter/services.dart';
910
import 'package:flutter_test/flutter_test.dart';
1011

1112
void runTransactionTests() {
@@ -69,6 +70,47 @@ void runTransactionTests() {
6970
expect(response, equals(randomValue));
7071
});
7172

73+
test(
74+
'does not report an error when the transaction stream is cancelled',
75+
() async {
76+
final List<Object> reportedErrors = <Object>[];
77+
final FlutterExceptionHandler? previousOnError = FlutterError.onError;
78+
FlutterError.onError = (FlutterErrorDetails details) {
79+
reportedErrors.add(details.exception);
80+
};
81+
addTearDown(() {
82+
FlutterError.onError = previousOnError;
83+
});
84+
85+
final DocumentReference<Map<String, dynamic>> doc =
86+
await initializeTest('transaction-cancel-cleanup');
87+
88+
await firestore.runTransaction((Transaction transaction) async {
89+
transaction.set(doc, {
90+
'updatedAt': DateTime.now().toIso8601String(),
91+
});
92+
});
93+
94+
await Future<void>.delayed(const Duration(milliseconds: 100));
95+
96+
final Iterable<Object> transactionCancelErrors =
97+
reportedErrors.where((Object error) {
98+
final String text = error.toString();
99+
return error is MissingPluginException &&
100+
text.contains('firebase_firestore/transaction');
101+
});
102+
103+
expect(
104+
transactionCancelErrors,
105+
isEmpty,
106+
reason: 'Unexpected FlutterError(s): $reportedErrors',
107+
);
108+
},
109+
skip: kIsWeb || defaultTargetPlatform != TargetPlatform.android
110+
? 'Android-only EventChannel teardown race'
111+
: false,
112+
);
113+
72114
test(
73115
'runs after reading a document',
74116
() async {

0 commit comments

Comments
 (0)