Skip to content

Commit 0a9cbce

Browse files
authored
fix(crashlytics, iOS): reorder error reason logging to match Android implementation (#17713)
* fix(crashlytics, iOS): reorder error reason logging to match Android implementation * feat(crashlytics): add test event channel and CI detection for error reasons * fix(tests): update imports and correct test formatting for consistency * fix(tests): correct import path and streamline event stream listener in crashlytics e2e tests * fix(tests): improve event channel declaration and enhance event stream listener for consistency * feat(crashlytics): add test event channel and modify CI detection logic * fix(crashlytics): ensure main thread execution for test event success callback * fix(crashlytics): correct formatting of crashlytics error reason for consistency across platforms * refactor(crashlytics): remove CI check from error reporting in Android and iOS implementations * refactor(tests): comment out delay in crashlytics e2e test for faster execution * fix(tests): update event handling in crashlytics e2e test for accurate event capture * fix(tests): add missing import for async functionality in crashlytics e2e test * fix(tests): add logging for received events in crashlytics e2e test * chore: ensure testEventSink is not null before posting error reason * chore(tests): remove unused import from firebase_crashlytics_e2e_test.dart
1 parent 0956646 commit 0a9cbce

File tree

3 files changed

+83
-4
lines changed

3 files changed

+83
-4
lines changed

packages/firebase_crashlytics/firebase_crashlytics/android/src/main/java/io/flutter/plugins/firebase/crashlytics/FlutterFirebaseCrashlyticsPlugin.java

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.firebase.crashlytics.internal.Logger;
2222
import io.flutter.embedding.engine.plugins.FlutterPlugin;
2323
import io.flutter.plugin.common.BinaryMessenger;
24+
import io.flutter.plugin.common.EventChannel;
2425
import io.flutter.plugin.common.MethodCall;
2526
import io.flutter.plugin.common.MethodChannel;
2627
import io.flutter.plugin.common.MethodChannel.MethodCallHandler;
@@ -34,9 +35,11 @@
3435

3536
/** FlutterFirebaseCrashlyticsPlugin */
3637
public class FlutterFirebaseCrashlyticsPlugin
37-
implements FlutterFirebasePlugin, FlutterPlugin, MethodCallHandler {
38+
implements FlutterFirebasePlugin, FlutterPlugin, MethodCallHandler, EventChannel.StreamHandler {
3839
public static final String TAG = "FLTFirebaseCrashlytics";
3940
private MethodChannel channel;
41+
private EventChannel testEventChannel;
42+
private EventChannel.EventSink testEventSink;
4043

4144
private static final String FIREBASE_CRASHLYTICS_COLLECTION_ENABLED =
4245
"firebase_crashlytics_collection_enabled";
@@ -46,6 +49,9 @@ private void initInstance(BinaryMessenger messenger) {
4649
channel = new MethodChannel(messenger, channelName);
4750
channel.setMethodCallHandler(this);
4851
FlutterFirebasePluginRegistry.registerPlugin(channelName, this);
52+
testEventChannel =
53+
new EventChannel(messenger, "plugins.flutter.io/firebase_crashlytics_test_stream");
54+
testEventChannel.setStreamHandler(this);
4955
}
5056

5157
@Override
@@ -59,6 +65,10 @@ public void onDetachedFromEngine(@NonNull FlutterPluginBinding binding) {
5965
channel.setMethodCallHandler(null);
6066
channel = null;
6167
}
68+
if (testEventChannel != null) {
69+
testEventChannel.setStreamHandler(null);
70+
testEventChannel = null;
71+
}
6272
}
6373

6474
private Task<Map<String, Object>> checkForUnsentReports() {
@@ -134,6 +144,7 @@ private Task<Map<String, Object>> didCrashOnPreviousExecution() {
134144

135145
private Task<Void> recordError(final Map<String, Object> arguments) {
136146
TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
147+
Handler mainHandler = new Handler(Looper.getMainLooper());
137148

138149
cachedThreadPool.execute(
139150
() -> {
@@ -160,8 +171,12 @@ private Task<Void> recordError(final Map<String, Object> arguments) {
160171

161172
Exception exception;
162173
if (reason != null) {
174+
final String crashlyticsErrorReason = "thrown " + reason;
175+
if (testEventSink != null) {
176+
mainHandler.post(() -> testEventSink.success(crashlyticsErrorReason));
177+
}
163178
// Set a "reason" (to match iOS) to show where the exception was thrown.
164-
crashlytics.setCustomKey(Constants.FLUTTER_ERROR_REASON, "thrown " + reason);
179+
crashlytics.setCustomKey(Constants.FLUTTER_ERROR_REASON, crashlyticsErrorReason);
165180
exception =
166181
new FlutterError(dartExceptionMessage + ". " + "Error thrown " + reason + ".");
167182
} else {
@@ -466,4 +481,14 @@ public Task<Void> didReinitializeFirebaseCore() {
466481

467482
return taskCompletionSource.getTask();
468483
}
484+
485+
@Override
486+
public void onListen(Object arguments, EventChannel.EventSink events) {
487+
testEventSink = events;
488+
}
489+
490+
@Override
491+
public void onCancel(Object arguments) {
492+
testEventSink = null;
493+
}
469494
}

packages/firebase_crashlytics/firebase_crashlytics/ios/firebase_crashlytics/Sources/firebase_crashlytics/FLTFirebaseCrashlyticsPlugin.m

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#endif
1616

1717
NSString *const kFLTFirebaseCrashlyticsChannelName = @"plugins.flutter.io/firebase_crashlytics";
18+
NSString *const kFLTFirebaseCrashlyticsTestChannelName =
19+
@"plugins.flutter.io/firebase_crashlytics_test_stream";
1820

1921
// Argument Keys
2022
NSString *const kCrashlyticsArgumentException = @"exception";
@@ -34,6 +36,11 @@
3436
NSString *const kCrashlyticsArgumentUnsentReports = @"unsentReports";
3537
NSString *const kCrashlyticsArgumentDidCrashOnPreviousExecution = @"didCrashOnPreviousExecution";
3638

39+
@interface FLTFirebaseCrashlyticsPlugin () <FlutterStreamHandler>
40+
@property(nonatomic, strong) FlutterEventChannel *testEventChannel;
41+
@property(nonatomic, strong) FlutterEventSink testEventSink;
42+
@end
43+
3744
@implementation FLTFirebaseCrashlyticsPlugin
3845

3946
#pragma mark - FlutterPlugin
@@ -61,6 +68,10 @@ + (void)registerWithRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
6168
binaryMessenger:[registrar messenger]];
6269
FLTFirebaseCrashlyticsPlugin *instance = [FLTFirebaseCrashlyticsPlugin sharedInstance];
6370
[registrar addMethodCallDelegate:instance channel:channel];
71+
instance.testEventChannel =
72+
[FlutterEventChannel eventChannelWithName:kFLTFirebaseCrashlyticsTestChannelName
73+
binaryMessenger:[registrar messenger]];
74+
[instance.testEventChannel setStreamHandler:instance];
6475
}
6576

6677
- (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)flutterResult {
@@ -126,10 +137,15 @@ - (void)recordError:(id)arguments withMethodCallResult:(FLTFirebaseMethodCallRes
126137
}
127138

128139
if (![reason isEqual:[NSNull null]]) {
129-
reason = [NSString stringWithFormat:@"%@. Error thrown %@.", dartExceptionMessage, reason];
140+
NSString *crashlyticsErrorReason = [NSString stringWithFormat:@"thrown %@", reason];
141+
142+
if (self.testEventSink) {
143+
self.testEventSink(crashlyticsErrorReason);
144+
}
130145
// Log additional custom value to match Android.
131-
[[FIRCrashlytics crashlytics] setCustomValue:[NSString stringWithFormat:@"thrown %@", reason]
146+
[[FIRCrashlytics crashlytics] setCustomValue:crashlyticsErrorReason
132147
forKey:@"flutter_error_reason"];
148+
reason = [NSString stringWithFormat:@"%@. Error thrown %@.", dartExceptionMessage, reason];
133149
} else {
134150
reason = dartExceptionMessage;
135151
}
@@ -247,4 +263,15 @@ - (NSString *_Nonnull)flutterChannelName {
247263
return kFLTFirebaseCrashlyticsChannelName;
248264
}
249265

266+
- (FlutterError *_Nullable)onCancelWithArguments:(id _Nullable)arguments {
267+
self.testEventSink = nil;
268+
return nil;
269+
}
270+
271+
- (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments
272+
eventSink:(nonnull FlutterEventSink)events {
273+
self.testEventSink = events;
274+
return nil;
275+
}
276+
250277
@end

tests/integration_test/firebase_crashlytics/firebase_crashlytics_e2e_test.dart

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
import 'package:firebase_core/firebase_core.dart';
66
import 'package:firebase_crashlytics/firebase_crashlytics.dart';
77
import 'package:flutter/foundation.dart';
8+
import 'package:flutter/services.dart';
89
import 'package:flutter_test/flutter_test.dart';
910
import 'package:integration_test/integration_test.dart';
1011
import 'package:tests/firebase_options.dart';
12+
import 'dart:async';
1113

1214
void main() {
1315
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
@@ -98,6 +100,31 @@ void main() {
98100
);
99101
},
100102
);
103+
104+
test(
105+
'should have consistent error reason format',
106+
() async {
107+
const eventChannel = EventChannel('plugins.flutter.io/firebase_crashlytics_test_stream');
108+
final eventStream = eventChannel.receiveBroadcastStream();
109+
110+
final completer = Completer<String>();
111+
112+
final subscription = eventStream.listen((event) {
113+
completer.complete(event.toString());
114+
});
115+
116+
await FirebaseCrashlytics.instance.recordError(
117+
'foo exception',
118+
StackTrace.fromString('during testing'),
119+
reason: 'foo reason',
120+
);
121+
122+
final event = await completer.future;
123+
expect(event, 'thrown foo reason');
124+
await subscription.cancel();
125+
},
126+
skip: kIsWeb || defaultTargetPlatform == TargetPlatform.macOS,
127+
);
101128
});
102129

103130
group('log', () {

0 commit comments

Comments
 (0)