-
-
Notifications
You must be signed in to change notification settings - Fork 246
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
Refactor BindingUtils
to enable the use of custom bindings
#1184
Changes from 11 commits
75bd42b
5af2195
5e09c88
ff712be
6b7f9c9
2b1eacf
04b086b
f4fd3a1
81eff14
1cdaee4
674375f
7e511f0
a586b46
9fd6c42
8327a4e
3fadd64
d831501
45c2a69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import 'dart:developer'; | ||
|
||
import 'package:flutter/widgets.dart'; | ||
|
||
import '../sentry_flutter.dart'; | ||
|
||
// The methods and properties are modelled after the the real binding class. | ||
ueman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
class BindingWrapper { | ||
/// The current [WidgetsBinding], if one has been created. | ||
/// Provides access to the features exposed by this mixin. | ||
/// The binding must be initialized before using this getter; | ||
/// this is typically done by calling [runApp] or [WidgetsFlutterBinding.ensureInitialized]. | ||
WidgetsBinding? get instance { | ||
try { | ||
return _ambiguate(WidgetsBinding.instance); | ||
} catch (e, s) { | ||
log( | ||
'WidgetsBinding.instance was not yet initialized', | ||
level: SentryLevel.error.toDartLogLevel(), | ||
error: e, | ||
stackTrace: s, | ||
name: 'Sentry', | ||
); | ||
marandaneto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return null; | ||
} | ||
} | ||
|
||
/// Returns an instance of the binding that implements [WidgetsBinding]. | ||
/// If no binding has yet been initialized, the [WidgetsFlutterBinding] class | ||
/// is used to create and initialize one. | ||
/// You only need to call this method if you need the binding to be | ||
/// initialized before calling [runApp]. | ||
WidgetsBinding ensureInitialized() => | ||
WidgetsFlutterBinding.ensureInitialized(); | ||
} | ||
|
||
WidgetsBinding? _ambiguate(WidgetsBinding? binding) => binding; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ import 'dart:ui'; | |
|
||
import 'package:flutter/widgets.dart'; | ||
import 'package:sentry/sentry.dart'; | ||
import '../binding_utils.dart'; | ||
import '../sentry_flutter_options.dart'; | ||
|
||
typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace); | ||
|
@@ -29,15 +28,11 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> { | |
@override | ||
void call(Hub hub, SentryFlutterOptions options) { | ||
_options = options; | ||
final binding = BindingUtils.getWidgetsBindingInstance(); | ||
|
||
if (binding == null) { | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this should stay, otherwise, it won't work anyway, so bail out early. |
||
final binding = options.bindingUtils.instance; | ||
|
||
// WidgetsBinding works with WidgetsFlutterBinding and other custom bindings | ||
final wrapper = dispatchWrapper ?? | ||
PlatformDispatcherWrapper(binding.platformDispatcher); | ||
PlatformDispatcherWrapper(binding?.platformDispatcher); | ||
|
||
_defaultOnError = wrapper.onError; | ||
|
||
|
@@ -97,15 +92,15 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> { | |
class PlatformDispatcherWrapper { | ||
PlatformDispatcherWrapper(this._dispatcher); | ||
|
||
final PlatformDispatcher _dispatcher; | ||
final PlatformDispatcher? _dispatcher; | ||
|
||
/// Should not be accessed if [isOnErrorSupported] == false | ||
ErrorCallback? get onError => | ||
(_dispatcher as dynamic).onError as ErrorCallback?; | ||
(_dispatcher as dynamic)?.onError as ErrorCallback?; | ||
|
||
/// Should not be accessed if [isOnErrorSupported] == false | ||
set onError(ErrorCallback? callback) { | ||
(_dispatcher as dynamic).onError = callback; | ||
(_dispatcher as dynamic)?.onError = callback; | ||
} | ||
|
||
bool isOnErrorSupported(SentryFlutterOptions options) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
import 'dart:async'; | ||
|
||
import 'package:sentry/sentry.dart'; | ||
import '../binding_utils.dart'; | ||
import '../sentry_flutter_options.dart'; | ||
import '../widgets_binding_observer.dart'; | ||
|
||
|
@@ -10,10 +9,12 @@ import '../widgets_binding_observer.dart'; | |
/// - [SentryWidgetsBindingObserver] | ||
/// - [WidgetsBindingObserver](https://api.flutter.dev/flutter/widgets/WidgetsBindingObserver-class.html) | ||
class WidgetsBindingIntegration extends Integration<SentryFlutterOptions> { | ||
SentryWidgetsBindingObserver? _observer; | ||
late SentryWidgetsBindingObserver _observer; | ||
late SentryFlutterOptions _options; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to keep it nullable, so if |
||
|
||
@override | ||
FutureOr<void> call(Hub hub, SentryFlutterOptions options) { | ||
_options = options; | ||
_observer = SentryWidgetsBindingObserver( | ||
hub: hub, | ||
options: options, | ||
|
@@ -22,23 +23,16 @@ class WidgetsBindingIntegration extends Integration<SentryFlutterOptions> { | |
// We don't need to call `WidgetsFlutterBinding.ensureInitialized()` | ||
// because `WidgetsFlutterBindingIntegration` already calls it. | ||
// If the instance is not created, we skip it to keep going. | ||
final instance = BindingUtils.getWidgetsBindingInstance(); | ||
if (instance != null) { | ||
instance.addObserver(_observer!); | ||
final instance = _options.bindingUtils.instance; | ||
if (instance == null) { | ||
instance?.addObserver(_observer); | ||
options.sdk.addIntegration('widgetsBindingIntegration'); | ||
} else { | ||
options.logger( | ||
SentryLevel.error, | ||
'widgetsBindingIntegration failed to be installed', | ||
); | ||
} | ||
} | ||
|
||
@override | ||
FutureOr<void> close() { | ||
final instance = BindingUtils.getWidgetsBindingInstance(); | ||
if (instance != null && _observer != null) { | ||
instance.removeObserver(_observer!); | ||
} | ||
final instance = _options.bindingUtils.instance; | ||
instance?.removeObserver(_observer); | ||
} | ||
} |
This file was deleted.
This file was deleted.
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.
If this is a refactor, we don't need an entry here, it's more about user-facing changes.
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.
I intend this to be user facing, although maybe it shouldn't be advertised.