Skip to content

Commit 6ac77d9

Browse files
rrousselGitSalakar
rrousselGit
and
Salakar
authored
feat(*): refactor error handling to preserve stack traces on platform exceptions (#8156)
Co-authored-by: Mike Diarmid <[email protected]>
1 parent 960a75a commit 6ac77d9

File tree

47 files changed

+548
-403
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+548
-403
lines changed

‎packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_document_reference.dart

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
4040
},
4141
},
4242
);
43-
} catch (e) {
44-
throw convertPlatformException(e);
43+
} catch (e, stack) {
44+
convertPlatformException(e, stack);
4545
}
4646
}
4747

@@ -56,8 +56,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
5656
'data': data,
5757
},
5858
);
59-
} catch (e) {
60-
throw convertPlatformException(e);
59+
} catch (e, stack) {
60+
convertPlatformException(e, stack);
6161
}
6262
}
6363

@@ -77,8 +77,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
7777
);
7878

7979
return DocumentSnapshotPlatform(firestore, _pointer.path, data!);
80-
} catch (e) {
81-
throw convertPlatformException(e);
80+
} catch (e, stack) {
81+
convertPlatformException(e, stack);
8282
}
8383
}
8484

@@ -89,8 +89,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
8989
'DocumentReference#delete',
9090
<String, dynamic>{'firestore': firestore, 'reference': this},
9191
);
92-
} catch (e) {
93-
throw convertPlatformException(e);
92+
} catch (e, stack) {
93+
convertPlatformException(e, stack);
9494
}
9595
}
9696

@@ -111,24 +111,27 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
111111
snapshotStream =
112112
MethodChannelFirebaseFirestore.documentSnapshotChannel(observerId!)
113113
.receiveBroadcastStream(
114-
<String, dynamic>{
115-
'reference': this,
116-
'includeMetadataChanges': includeMetadataChanges,
117-
},
118-
).listen((snapshot) {
119-
controller.add(
120-
DocumentSnapshotPlatform(
121-
firestore,
122-
snapshot['path'],
123-
<String, dynamic>{
124-
'data': snapshot['data'],
125-
'metadata': snapshot['metadata'],
126-
},
127-
),
128-
);
129-
}, onError: (error, stack) {
130-
controller.addError(convertPlatformException(error), stack);
131-
});
114+
<String, dynamic>{
115+
'reference': this,
116+
'includeMetadataChanges': includeMetadataChanges,
117+
},
118+
)
119+
.handleError(convertPlatformException)
120+
.listen(
121+
(snapshot) {
122+
controller.add(
123+
DocumentSnapshotPlatform(
124+
firestore,
125+
snapshot['path'],
126+
<String, dynamic>{
127+
'data': snapshot['data'],
128+
'metadata': snapshot['metadata'],
129+
},
130+
),
131+
);
132+
},
133+
onError: controller.addError,
134+
);
132135
},
133136
onCancel: () {
134137
snapshotStream?.cancel();

‎packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,20 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
107107
FirebaseFirestorePlatform.instance,
108108
data!,
109109
);
110-
} catch (e) {
110+
} catch (e, stack) {
111111
if (e.toString().contains('Named query has not been found')) {
112-
throw FirebaseException(
112+
Error.throwWithStackTrace(
113+
FirebaseException(
113114
plugin: 'cloud_firestore',
114115
code: 'non-existent-named-query',
115-
message:
116-
'Named query has not been found. Please check it has been loaded properly via loadBundle().');
116+
message: 'Named query has not been found. '
117+
'Please check it has been loaded properly via loadBundle().',
118+
),
119+
stack,
120+
);
117121
}
118122

119-
throw convertPlatformException(e);
123+
convertPlatformException(e, stack);
120124
}
121125
}
122126

@@ -130,8 +134,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
130134
.invokeMethod<void>('Firestore#clearPersistence', <String, dynamic>{
131135
'firestore': this,
132136
});
133-
} catch (e) {
134-
throw convertPlatformException(e);
137+
} catch (e, stack) {
138+
convertPlatformException(e, stack);
135139
}
136140
}
137141

@@ -160,8 +164,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
160164
.invokeMethod<void>('Firestore#disableNetwork', <String, dynamic>{
161165
'firestore': this,
162166
});
163-
} catch (e) {
164-
throw convertPlatformException(e);
167+
} catch (e, stack) {
168+
convertPlatformException(e, stack);
165169
}
166170
}
167171

@@ -177,8 +181,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
177181
.invokeMethod<void>('Firestore#enableNetwork', <String, dynamic>{
178182
'firestore': this,
179183
});
180-
} catch (e) {
181-
throw convertPlatformException(e);
184+
} catch (e, stack) {
185+
convertPlatformException(e, stack);
182186
}
183187
}
184188

@@ -195,14 +199,15 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
195199
snapshotStream =
196200
MethodChannelFirebaseFirestore.snapshotsInSyncChannel(observerId!)
197201
.receiveBroadcastStream(
198-
<String, dynamic>{
199-
'firestore': this,
200-
},
201-
).listen((event) {
202-
controller.add(null);
203-
}, onError: (error, stack) {
204-
controller.addError(convertPlatformException(error), stack);
205-
});
202+
<String, dynamic>{
203+
'firestore': this,
204+
},
205+
)
206+
.handleError(convertPlatformException)
207+
.listen(
208+
(event) => controller.add(null),
209+
onError: controller.addError,
210+
);
206211
},
207212
onCancel: () {
208213
snapshotStream?.cancel();
@@ -306,8 +311,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
306311
await channel.invokeMethod<void>('Firestore#terminate', <String, dynamic>{
307312
'firestore': this,
308313
});
309-
} catch (e) {
310-
throw convertPlatformException(e);
314+
} catch (e, stack) {
315+
convertPlatformException(e, stack);
311316
}
312317
}
313318

@@ -318,8 +323,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
318323
'Firestore#waitForPendingWrites', <String, dynamic>{
319324
'firestore': this,
320325
});
321-
} catch (e) {
322-
throw convertPlatformException(e);
326+
} catch (e, stack) {
327+
convertPlatformException(e, stack);
323328
}
324329
}
325330
}

‎packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_query.dart

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ class MethodChannelQuery extends QueryPlatform {
106106
);
107107

108108
return MethodChannelQuerySnapshot(firestore, data!);
109-
} catch (e) {
110-
throw convertPlatformException(e);
109+
} catch (e, stack) {
110+
convertPlatformException(e, stack);
111111
}
112112
}
113113

@@ -145,15 +145,19 @@ class MethodChannelQuery extends QueryPlatform {
145145
snapshotStream =
146146
MethodChannelFirebaseFirestore.querySnapshotChannel(observerId!)
147147
.receiveBroadcastStream(
148-
<String, dynamic>{
149-
'query': this,
150-
'includeMetadataChanges': includeMetadataChanges,
151-
},
152-
).listen((snapshot) {
153-
controller.add(MethodChannelQuerySnapshot(firestore, snapshot));
154-
}, onError: (error, stack) {
155-
controller.addError(convertPlatformException(error), stack);
156-
});
148+
<String, dynamic>{
149+
'query': this,
150+
'includeMetadataChanges': includeMetadataChanges,
151+
},
152+
)
153+
.handleError(convertPlatformException)
154+
.listen(
155+
(snapshot) {
156+
controller
157+
.add(MethodChannelQuerySnapshot(firestore, snapshot));
158+
},
159+
onError: controller.addError,
160+
);
157161
},
158162
onCancel: () {
159163
snapshotStream?.cancel();

‎packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_write_batch.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ class MethodChannelWriteBatch extends WriteBatchPlatform {
4848
'firestore': _firestore,
4949
'writes': _writes,
5050
});
51-
} catch (e) {
52-
throw convertPlatformException(e);
51+
} catch (e, stack) {
52+
convertPlatformException(e, stack);
5353
}
5454
}
5555

‎packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/utils/exception.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@ import 'package:flutter/services.dart';
99
/// Catches a [PlatformException] and returns an [Exception].
1010
///
1111
/// If the [Exception] is a [PlatformException], a [FirebaseException] is returned.
12-
Exception convertPlatformException(Object exception) {
12+
Never convertPlatformException(Object exception, StackTrace stackTrace) {
1313
if (exception is! Exception || exception is! PlatformException) {
14-
throw exception;
14+
Error.throwWithStackTrace(exception, stackTrace);
1515
}
1616

17-
return platformExceptionToFirebaseException(exception);
17+
Error.throwWithStackTrace(
18+
platformExceptionToFirebaseException(exception),
19+
stackTrace,
20+
);
1821
}
1922

2023
/// Converts a [PlatformException] into a [FirebaseException].

‎packages/cloud_functions/cloud_functions_platform_interface/lib/src/method_channel/method_channel_https_callable.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class MethodChannelHttpsCallable extends HttpsCallablePlatform {
3636
return result;
3737
}
3838
} catch (e, s) {
39-
throw convertPlatformException(e, s);
39+
convertPlatformException(e, s);
4040
}
4141
}
4242
}

‎packages/cloud_functions/cloud_functions_platform_interface/lib/src/method_channel/utils/exception.dart

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@ import '../../../cloud_functions_platform_interface.dart';
1111
/// Catches a [PlatformException] and returns an [Exception].
1212
///
1313
/// If the [Exception] is a [PlatformException], a [FirebaseFunctionsException] is returned.
14-
Exception convertPlatformException(Object exception, [StackTrace? stackTrace]) {
14+
Never convertPlatformException(Object exception, StackTrace stackTrace) {
1515
if (exception is! Exception || exception is! PlatformException) {
16-
throw exception;
16+
Error.throwWithStackTrace(exception, stackTrace);
1717
}
1818

19-
return platformExceptionToFirebaseFunctionsException(exception, stackTrace);
19+
Error.throwWithStackTrace(
20+
platformExceptionToFirebaseFunctionsException(exception, stackTrace),
21+
stackTrace,
22+
);
2023
}
2124

2225
/// Converts a [PlatformException] into a [FirebaseFunctionsException].
@@ -25,8 +28,9 @@ Exception convertPlatformException(Object exception, [StackTrace? stackTrace]) {
2528
/// the `details` of the exception exist. Firebase returns specific codes and
2629
/// messages which can be converted into user friendly exceptions.
2730
FirebaseException platformExceptionToFirebaseFunctionsException(
28-
PlatformException platformException,
29-
[StackTrace? stackTrace]) {
31+
PlatformException platformException,
32+
StackTrace stackTrace,
33+
) {
3034
Map<String, dynamic>? details = platformException.details != null
3135
? Map<String, dynamic>.from(platformException.details)
3236
: null;
@@ -41,8 +45,9 @@ FirebaseException platformExceptionToFirebaseFunctionsException(
4145
}
4246

4347
return FirebaseFunctionsException(
44-
code: code,
45-
message: message!,
46-
details: additionalData,
47-
stackTrace: stackTrace);
48+
code: code,
49+
message: message!,
50+
details: additionalData,
51+
stackTrace: stackTrace,
52+
);
4853
}

‎packages/cloud_functions/cloud_functions_platform_interface/test/method_channel/utils/exception_test.dart

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ void main() {
1717
test('should throw any exception', () async {
1818
AssertionError assertionError = AssertionError();
1919

20-
expect(() => convertPlatformException(assertionError),
21-
throwsA(isA<AssertionError>()));
20+
expect(
21+
() => convertPlatformException(assertionError, StackTrace.empty),
22+
throwsA(isA<AssertionError>()),
23+
);
2224
});
2325

2426
test(
@@ -30,11 +32,14 @@ void main() {
3032
);
3133

3234
expect(
33-
convertPlatformException(platformException),
35+
() => convertPlatformException(platformException, StackTrace.empty),
36+
throwsA(
3437
isA<FirebaseFunctionsException>()
3538
.having((e) => e.code, 'code', 'unknown')
3639
.having((e) => e.message, 'message', testMessage)
37-
.having((e) => e.details, 'details', isNull));
40+
.having((e) => e.details, 'details', isNull),
41+
),
42+
);
3843
});
3944

4045
test('should override code and message if provided to additional details',
@@ -46,11 +51,14 @@ void main() {
4651
details: {'code': code, 'message': testMessage});
4752

4853
expect(
49-
convertPlatformException(platformException),
54+
() => convertPlatformException(platformException, StackTrace.empty),
55+
throwsA(
5056
isA<FirebaseFunctionsException>()
5157
.having((e) => e.code, 'code', code)
5258
.having((e) => e.message, 'message', testMessage)
53-
.having((e) => e.details, 'details', isNull));
59+
.having((e) => e.details, 'details', isNull),
60+
),
61+
);
5462
});
5563

5664
test('should provide additionalData as details', () async {
@@ -60,15 +68,18 @@ void main() {
6068
details: {'additionalData': testAdditionalData});
6169

6270
expect(
63-
convertPlatformException(platformException),
71+
() => convertPlatformException(platformException, StackTrace.empty),
72+
throwsA(
6473
isA<FirebaseFunctionsException>()
6574
.having((e) => e.code, 'code', 'unknown')
6675
.having((e) => e.message, 'message', testMessage)
6776
.having(
6877
(e) => e.details,
6978
'details',
7079
isA<Map<String, dynamic>>()
71-
.having((e) => e['foo'], 'additionalData', 'bar')));
80+
.having((e) => e['foo'], 'additionalData', 'bar')),
81+
),
82+
);
7283
});
7384
});
7485
}

0 commit comments

Comments
 (0)