Skip to content

null safety on sentry_flutter #337

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Mar 7, 2021
Merged

null safety on sentry_flutter #337

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
13eb5bf
migrate to null safety for sentry_flutter
fzyzcjy Mar 5, 2021
697f41c
make it compile on dart2.12/flutter2.0
fzyzcjy Mar 5, 2021
5cb2538
fix a few constructors
fzyzcjy Mar 6, 2021
07fcaee
avoid WidgetsBinding.instance! as it can be null
fzyzcjy Mar 6, 2021
b7b8e23
remove `as FutureOr Function()`
fzyzcjy Mar 6, 2021
5aeebd6
fix compile error
fzyzcjy Mar 6, 2021
8550c13
remove getter/setter which are only used for null checks
fzyzcjy Mar 6, 2021
9c3ccff
fixbug some warnings
fzyzcjy Mar 6, 2021
a443a43
generate mocks using mockito code generator
fzyzcjy Mar 6, 2021
68458a8
fix bugs about "!" on null value, and return null value
fzyzcjy Mar 6, 2021
20a9323
add a "when", otherwise mockito complains no stub value to return
fzyzcjy Mar 6, 2021
ad05773
make _observer nullable, since `call` may not be called
fzyzcjy Mar 6, 2021
10247d1
add changelog
fzyzcjy Mar 6, 2021
85b76b0
execute `flutter format .` on sentry-flutter
fzyzcjy Mar 6, 2021
fb2913a
fix analyze problems
fzyzcjy Mar 6, 2021
adb9c4a
change mockito version
fzyzcjy Mar 6, 2021
e3d8051
change signature of ensureInitialized
fzyzcjy Mar 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions flutter/lib/src/default_integrations.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import 'widgets_binding_observer.dart';
class WidgetsFlutterBindingIntegration
extends Integration<SentryFlutterOptions> {
WidgetsFlutterBindingIntegration(
[WidgetsBinding Function() ensureInitialized])
[WidgetsBinding? Function()? ensureInitialized])
: _ensureInitialized =
ensureInitialized ?? WidgetsFlutterBinding.ensureInitialized;

final WidgetsBinding Function() _ensureInitialized;
final WidgetsBinding? Function() _ensureInitialized;

@override
FutureOr<void> call(Hub hub, SentryFlutterOptions options) {
Expand Down Expand Up @@ -104,7 +104,7 @@ class LoadContextsIntegration extends Integration<SentryFlutterOptions> {
(event, {hint}) async {
try {
final infos = Map<String, dynamic>.from(
await _channel.invokeMethod('loadContexts'),
await (_channel.invokeMethod('loadContexts') as FutureOr<Map<dynamic, dynamic>>),
);
if (infos['contexts'] != null) {
final contexts = Contexts.fromJson(
Expand Down Expand Up @@ -137,7 +137,7 @@ class LoadContextsIntegration extends Integration<SentryFlutterOptions> {
if (infos['package'] != null) {
final package = Map<String, String>.from(infos['package'] as Map);
final sdk = event.sdk ?? options.sdk;
sdk.addPackage(package['sdk_name'], package['version']);
sdk.addPackage(package['sdk_name']!, package['version']!);
event = event.copyWith(sdk: sdk);
}
} catch (error) {
Expand Down Expand Up @@ -201,7 +201,7 @@ class NativeSdkIntegration extends Integration<SentryFlutterOptions> {
/// - [SentryWidgetsBindingObserver]
/// - [WidgetsBindingObserver](https://api.flutter.dev/flutter/widgets/WidgetsBindingObserver-class.html)
class WidgetsBindingIntegration extends Integration<SentryFlutterOptions> {
SentryWidgetsBindingObserver _observer;
late SentryWidgetsBindingObserver _observer;

@override
FutureOr<void> call(Hub hub, SentryFlutterOptions options) {
Expand Down Expand Up @@ -247,9 +247,9 @@ class LoadAndroidImageListIntegration
(event, {hint}) async {
try {
if (event.exception != null &&
event.exception.stackTrace != null &&
event.exception.stackTrace.frames != null) {
final needsSymbolication = event.exception.stackTrace.frames
event.exception!.stackTrace != null &&
event.exception!.stackTrace!.frames != null) {
final needsSymbolication = event.exception!.stackTrace!.frames
.any((element) => 'native' == element.platform);

// if there are no frames that require symbolication, we don't
Expand All @@ -264,7 +264,7 @@ class LoadAndroidImageListIntegration
// we call on every event because the loaded image list is cached
// and it could be changed on the Native side.
final imageList = List<Map<dynamic, dynamic>>.from(
await _channel.invokeMethod('loadImageList'),
await (_channel.invokeMethod('loadImageList') as FutureOr<Iterable<dynamic>>),
);

if (imageList.isEmpty) {
Expand All @@ -274,13 +274,13 @@ class LoadAndroidImageListIntegration
final newDebugImages = <DebugImage>[];

for (final item in imageList) {
final codeFile = item['code_file'] as String;
final codeId = item['code_id'] as String;
final imageAddr = item['image_addr'] as String;
final imageSize = item['image_size'] as int;
final codeFile = item['code_file'] as String?;
final codeId = item['code_id'] as String?;
final imageAddr = item['image_addr'] as String?;
final imageSize = item['image_size'] as int?;
final type = item['type'] as String;
final debugId = item['debug_id'] as String;
final debugFile = item['debug_file'] as String;
final debugId = item['debug_id'] as String?;
final debugFile = item['debug_file'] as String?;

final image = DebugImage(
type: type,
Expand Down
32 changes: 16 additions & 16 deletions flutter/lib/src/navigation/sentry_navigator_observer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const _navigationKey = 'navigation';
/// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html)
/// - [Navigating with arguments](https://flutter.dev/docs/cookbook/navigation/navigate-with-arguments)
class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
factory SentryNavigatorObserver({Hub hub}) {
factory SentryNavigatorObserver({Hub? hub}) {
return SentryNavigatorObserver._(hub ?? HubAdapter());
}

Expand All @@ -44,7 +44,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
final Hub hub;

@override
void didPush(Route<dynamic> route, Route<dynamic> previousRoute) {
void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
super.didPush(route, previousRoute);
_addBreadcrumb(
type: 'didPush',
Expand All @@ -54,7 +54,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
}

@override
void didReplace({Route<dynamic> newRoute, Route<dynamic> oldRoute}) {
void didReplace({Route<dynamic>? newRoute, Route<dynamic>? oldRoute}) {
super.didReplace(newRoute: newRoute, oldRoute: oldRoute);

_addBreadcrumb(
Expand All @@ -65,7 +65,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
}

@override
void didPop(Route<dynamic> route, Route<dynamic> previousRoute) {
void didPop(Route<dynamic> route, Route<dynamic>? previousRoute) {
super.didPop(route, previousRoute);

_addBreadcrumb(
Expand All @@ -76,9 +76,9 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
}

void _addBreadcrumb({
String type,
RouteSettings from,
RouteSettings to,
required String type,
RouteSettings? from,
RouteSettings? to,
}) {
hub.addBreadcrumb(RouteObserverBreadcrumb(
navigationType: type,
Expand All @@ -98,10 +98,10 @@ class RouteObserverBreadcrumb extends Breadcrumb {
factory RouteObserverBreadcrumb({
/// This should correspond to Flutters navigation events.
/// See https://api.flutter.dev/flutter/widgets/RouteObserver-class.html
@required String navigationType,
RouteSettings from,
RouteSettings to,
SentryLevel level,
required String navigationType,
RouteSettings? from,
RouteSettings? to,
SentryLevel? level,
}) {
final dynamic fromArgs = _formatArgs(from?.arguments);
final dynamic toArgs = _formatArgs(to?.arguments);
Expand All @@ -116,12 +116,12 @@ class RouteObserverBreadcrumb extends Breadcrumb {
}

RouteObserverBreadcrumb._({
@required String navigationType,
String from,
required String navigationType,
String? from,
dynamic fromArgs,
String to,
String? to,
dynamic toArgs,
SentryLevel level,
SentryLevel? level,
}) : assert(navigationType != null),
super(
category: _navigationKey,
Expand All @@ -135,7 +135,7 @@ class RouteObserverBreadcrumb extends Breadcrumb {
if (toArgs != null) 'to_arguments': toArgs,
});

static dynamic _formatArgs(Object args) {
static dynamic _formatArgs(Object? args) {
if (args == null) {
return null;
}
Expand Down
4 changes: 2 additions & 2 deletions flutter/lib/src/sentry_flutter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ mixin SentryFlutter {

static Future<void> init(
FlutterOptionsConfiguration optionsConfiguration, {
AppRunner appRunner,
AppRunner? appRunner,
PackageLoader packageLoader = _loadPackageInfo,
iOSPlatformChecker isIOSChecker = isIOS,
AndroidPlatformChecker isAndroidChecker = isAndroid,
Expand Down Expand Up @@ -53,7 +53,7 @@ mixin SentryFlutter {

await Sentry.init(
(options) async {
await optionsConfiguration(options);
await optionsConfiguration(options as SentryFlutterOptions);
},
appRunner: appRunner,
options: flutterOptions,
Expand Down
2 changes: 1 addition & 1 deletion flutter/lib/src/sentry_flutter_options.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import 'package:sentry/sentry.dart';
/// Note that some of these options require native Sentry integration, which is
/// not available on all platforms.
class SentryFlutterOptions extends SentryOptions {
SentryFlutterOptions({String dsn}) : super(dsn: dsn);
SentryFlutterOptions({String? dsn}) : super(dsn: dsn);

bool _enableAutoSessionTracking = true;

Expand Down
14 changes: 7 additions & 7 deletions flutter/lib/src/widgets_binding_observer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ import '../sentry_flutter.dart';
/// - [WidgetsBindingObserver](https://api.flutter.dev/flutter/widgets/WidgetsBindingObserver-class.html)
class SentryWidgetsBindingObserver with WidgetsBindingObserver {
SentryWidgetsBindingObserver({
Hub hub,
@required SentryFlutterOptions options,
Hub? hub,
required SentryFlutterOptions options,
}) {
_hub = hub ?? HubAdapter();
assert(options != null);
_options = options;
}

Hub _hub;
SentryFlutterOptions _options;
late Hub _hub;
late SentryFlutterOptions _options;

/// This method records lifecycle events.
/// It tries to mimic the behavior of ActivityBreadcrumbsIntegration of Sentry
Expand Down Expand Up @@ -66,7 +66,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
if (!_options.enableWindowMetricBreadcrumbs) {
return;
}
final window = WidgetsBinding.instance.window;
final window = WidgetsBinding.instance!.window;
_hub.addBreadcrumb(Breadcrumb(
message: 'Screen size changed',
category: 'device.screen',
Expand All @@ -86,7 +86,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
if (!_options.enableBrightnessChangeBreadcrumbs) {
return;
}
final brightness = WidgetsBinding.instance.window.platformBrightness;
final brightness = WidgetsBinding.instance!.window.platformBrightness;
final brightnessDescription =
brightness == Brightness.dark ? 'dark' : 'light';

Expand All @@ -107,7 +107,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
if (!_options.enableTextScaleChangeBreadcrumbs) {
return;
}
final newTextScaleFactor = WidgetsBinding.instance.window.textScaleFactor;
final newTextScaleFactor = WidgetsBinding.instance!.window.textScaleFactor;
_hub.addBreadcrumb(Breadcrumb(
message: 'Text scale factor changed to $newTextScaleFactor.',
type: 'system',
Expand Down
2 changes: 1 addition & 1 deletion flutter/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ homepage: https://docs.sentry.io/platforms/flutter/
repository: https://github.com/getsentry/sentry-dart

environment:
sdk: ">=2.8.0 <3.0.0"
sdk: '>=2.12.0 <3.0.0'
flutter: ">=1.17.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marandaneto Does it make sense to keep it this low? I wonder if it causes any problems because Flutter 1.17 doesn't has null safety. On the other hand
package_info for example allows versions as low as 1.12 while also having null safety.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah unless we have to, lets keep as it is, all the official plugins are down to 1.12

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw looking at https://dart.dev/null-safety

Sound null safety is available in Dart 2.12 and Flutter 2.

should we bump to min. v2?


dependencies:
Expand Down
14 changes: 7 additions & 7 deletions flutter/test/default_integrations_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ void main() {

TestWidgetsFlutterBinding.ensureInitialized();

Fixture fixture;
late Fixture fixture;

setUp(() {
fixture = Fixture();
Expand All @@ -26,7 +26,7 @@ void main() {

void _reportError({
bool silent = false,
FlutterExceptionHandler handler,
FlutterExceptionHandler? handler,
dynamic exception,
}) {
// replace default error otherwise it fails on testing
Expand All @@ -48,7 +48,7 @@ void main() {
_reportError(exception: exception);

final event = verify(
await fixture.hub.captureEvent(captureAny),
await fixture.hub.captureEvent(captureAny!),
).captured.first as SentryEvent;

expect(SentryLevel.fatal, event.level);
Expand All @@ -67,23 +67,23 @@ void main() {

_reportError(handler: defaultError);

verify(await fixture.hub.captureEvent(captureAny));
verify(await fixture.hub.captureEvent(captureAny!));

expect(true, called);
});

test('FlutterError do not capture if silent error', () async {
_reportError(silent: true);

verifyNever(await fixture.hub.captureEvent(captureAny));
verifyNever(await fixture.hub.captureEvent(captureAny!));
});

test('FlutterError captures if silent error but reportSilentFlutterErrors',
() async {
fixture.options.reportSilentFlutterErrors = true;
_reportError(silent: true);

verify(await fixture.hub.captureEvent(captureAny));
verify(await fixture.hub.captureEvent(captureAny!));
});

test('FlutterError adds integration', () async {
Expand All @@ -106,7 +106,7 @@ void main() {

test('nativeSdkIntegration do not throw', () async {
_channel.setMockMethodCallHandler((MethodCall methodCall) async {
throw null;
throw Exception();
});

final integration = NativeSdkIntegration(_channel);
Expand Down
4 changes: 2 additions & 2 deletions flutter/test/file_system_transport_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ void main() {

TestWidgetsFlutterBinding.ensureInitialized();

Fixture fixture;
late Fixture fixture;

setUp(() {
fixture = Fixture();
Expand All @@ -33,7 +33,7 @@ void main() {

test('FileSystemTransport returns emptyId if channel throws', () async {
_channel.setMockMethodCallHandler((MethodCall methodCall) async {
throw null;
throw Exception();
});

final transport = fixture.getSut(_channel);
Expand Down
14 changes: 8 additions & 6 deletions flutter/test/load_android_image_list_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:async';

import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:sentry/sentry.dart';
Expand Down Expand Up @@ -87,23 +89,23 @@ void main() {
final hub = Hub(options);

LoadAndroidImageListIntegration(_channel)(hub, options);
final ep = options.eventProcessors.first;
var event = getEvent();
final ep = options.eventProcessors.first as FutureOr<SentryEvent?> Function(SentryEvent?, {dynamic hint});
SentryEvent? event = getEvent();
event = await ep(event);

expect(1, event.debugMeta.images.length);
expect(1, event!.debugMeta!.images.length);
});

test('Event processor asserts image list', () async {
final options = SentryFlutterOptions()..dsn = fakeDsn;
final hub = Hub(options);

LoadAndroidImageListIntegration(_channel)(hub, options);
final ep = options.eventProcessors.first;
var event = getEvent();
final ep = options.eventProcessors.first as FutureOr<SentryEvent?> Function(SentryEvent?, {dynamic hint});
SentryEvent? event = getEvent();
event = await ep(event);

final image = event.debugMeta.images.first;
final image = event!.debugMeta!.images.first;

expect('/apex/com.android.art/javalib/arm64/boot.oat', image.codeFile);
expect('13577ce71153c228ecf0eb73fc39f45010d487f8', image.codeId);
Expand Down
Loading