Skip to content

Commit

Permalink
Unify unhandled error reporting, add PlatformDispatcher.onError (flut…
Browse files Browse the repository at this point in the history
  • Loading branch information
dnfield authored Apr 9, 2022
1 parent 30f3d75 commit d24339f
Show file tree
Hide file tree
Showing 44 changed files with 723 additions and 219 deletions.
16 changes: 8 additions & 8 deletions lib/io/dart_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "third_party/tonic/converter/dart_converter.h"
#include "third_party/tonic/logging/dart_error.h"

using tonic::LogIfError;
using tonic::CheckAndHandleError;
using tonic::ToDart;

namespace flutter {
Expand All @@ -20,35 +20,35 @@ void DartIO::InitForIsolate(bool may_insecurely_connect_to_all_domains,
Dart_Handle io_lib = Dart_LookupLibrary(ToDart("dart:io"));
Dart_Handle result = Dart_SetNativeResolver(io_lib, dart::bin::LookupIONative,
dart::bin::LookupIONativeSymbol);
FML_CHECK(!LogIfError(result));
FML_CHECK(!CheckAndHandleError(result));

Dart_Handle embedder_config_type =
Dart_GetNonNullableType(io_lib, ToDart("_EmbedderConfig"), 0, nullptr);
FML_CHECK(!LogIfError(embedder_config_type));
FML_CHECK(!CheckAndHandleError(embedder_config_type));

Dart_Handle allow_insecure_connections_result = Dart_SetField(
embedder_config_type, ToDart("_mayInsecurelyConnectToAllDomains"),
ToDart(may_insecurely_connect_to_all_domains));
FML_CHECK(!LogIfError(allow_insecure_connections_result));
FML_CHECK(!CheckAndHandleError(allow_insecure_connections_result));

Dart_Handle dart_args[1];
dart_args[0] = ToDart(domain_network_policy);
Dart_Handle set_domain_network_policy_result = Dart_Invoke(
embedder_config_type, ToDart("_setDomainPolicies"), 1, dart_args);
FML_CHECK(!LogIfError(set_domain_network_policy_result));
FML_CHECK(!CheckAndHandleError(set_domain_network_policy_result));

Dart_Handle ui_lib = Dart_LookupLibrary(ToDart("dart:ui"));
Dart_Handle dart_validate_args[1];
dart_validate_args[0] = ToDart(may_insecurely_connect_to_all_domains);
Dart_Handle http_connection_hook_closure =
Dart_Invoke(ui_lib, ToDart("_getHttpConnectionHookClosure"),
/*number_of_arguments=*/1, dart_validate_args);
FML_CHECK(!LogIfError(http_connection_hook_closure));
FML_CHECK(!CheckAndHandleError(http_connection_hook_closure));
Dart_Handle http_lib = Dart_LookupLibrary(ToDart("dart:_http"));
FML_CHECK(!LogIfError(http_lib));
FML_CHECK(!CheckAndHandleError(http_lib));
Dart_Handle set_http_connection_hook_result = Dart_SetField(
http_lib, ToDart("_httpConnectionHook"), http_connection_hook_closure);
FML_CHECK(!LogIfError(set_http_connection_hook_result));
FML_CHECK(!CheckAndHandleError(set_http_connection_hook_result));
}

} // namespace flutter
1 change: 0 additions & 1 deletion lib/ui/dart_runtime_hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "third_party/tonic/scopes/dart_isolate_scope.h"

using tonic::DartConverter;
using tonic::LogIfError;
using tonic::ToDart;

namespace flutter {
Expand Down
53 changes: 50 additions & 3 deletions lib/ui/fixtures/ui_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,33 @@ void main() {}
/// validation.
void _finish() native 'Finish';

@pragma('vm:entry-point')
void customOnErrorTrue() {
PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) {
_finish();
return true;
};
throw Exception('true');
}

@pragma('vm:entry-point')
void customOnErrorFalse() {
PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) {
_finish();
return false;
};
throw Exception('false');
}

@pragma('vm:entry-point')
void customOnErrorThrow() {
PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) {
_finish();
throw Exception('throw2');
};
throw Exception('throw1');
}

@pragma('vm:entry-point')
void validateSceneBuilderAndScene() {
final SceneBuilder builder = SceneBuilder();
Expand Down Expand Up @@ -311,9 +338,9 @@ void hooksTests() {
}
}

void expectIdentical(Zone originalZone, Zone callbackZone) {
if (!identical(callbackZone, originalZone)) {
throw 'Callback called in wrong zone.';
void expectIdentical(Object a, Object b) {
if (!identical(a, b)) {
throw 'Expected $a to be identical to $b.';
}
}

Expand Down Expand Up @@ -368,6 +395,26 @@ void hooksTests() {
}
});

test('onError preserves the callback zone', () {
late Zone originalZone;
late Zone callbackZone;
final Object error = Exception('foo');
StackTrace? stackTrace;

runZoned(() {
originalZone = Zone.current;
PlatformDispatcher.instance.onError = (Object exception, StackTrace? stackTrace) {
callbackZone = Zone.current;
expectIdentical(exception, error);
expectNotEquals(stackTrace, null);
return true;
};
});

_callHook('_onError', 2, error, StackTrace.current);
expectIdentical(originalZone, callbackZone);
});

test('updateUserSettings can handle an empty object', () {
_callHook('_updateUserSettingsData', 1, '{}');
});
Expand Down
27 changes: 13 additions & 14 deletions lib/ui/hooks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,28 +115,27 @@ void _drawFrame() {
PlatformDispatcher.instance._drawFrame();
}

@pragma('vm:entry-point')
bool _onError(Object error, StackTrace? stackTrace) {
return PlatformDispatcher.instance._dispatchError(error, stackTrace ?? StackTrace.empty);
}

// ignore: always_declare_return_types, prefer_generic_function_type_aliases
typedef _ListStringArgFunction(List<String> args);

@pragma('vm:entry-point')
void _runMainZoned(Function startMainIsolateFunction,
Function userMainFunction,
List<String> args) {
void _runMain(Function startMainIsolateFunction,
Function userMainFunction,
List<String> args) {
startMainIsolateFunction(() {
runZonedGuarded<void>(() {
if (userMainFunction is _ListStringArgFunction) {
userMainFunction(args);
} else {
userMainFunction();
}
}, (Object error, StackTrace stackTrace) {
_reportUnhandledException(error.toString(), stackTrace.toString());
});
if (userMainFunction is _ListStringArgFunction) {
userMainFunction(args);
} else {
userMainFunction();
}
}, null);
}

void _reportUnhandledException(String error, String stackTrace) native 'PlatformConfiguration_reportUnhandledException';

/// Invokes [callback] inside the given [zone].
void _invoke(void Function()? callback, Zone zone) {
if (callback == null) {
Expand Down
4 changes: 2 additions & 2 deletions lib/ui/painting/paint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,10 @@ flutter::Paint DartConverter<flutter::Paint>::FromArguments(
int index,
Dart_Handle& exception) {
Dart_Handle paint_objects = Dart_GetNativeArgument(args, index);
FML_DCHECK(!LogIfError(paint_objects));
FML_DCHECK(!CheckAndHandleError(paint_objects));

Dart_Handle paint_data = Dart_GetNativeArgument(args, index + 1);
FML_DCHECK(!LogIfError(paint_data));
FML_DCHECK(!CheckAndHandleError(paint_data));

return flutter::Paint(paint_objects, paint_data);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/painting/rrect.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ RRect DartConverter<flutter::RRect>::FromArguments(Dart_NativeArguments args,
int index,
Dart_Handle& exception) {
Dart_Handle value = Dart_GetNativeArgument(args, index);
FML_DCHECK(!LogIfError(value));
FML_DCHECK(!CheckAndHandleError(value));
return FromDart(value);
}

Expand Down
54 changes: 54 additions & 0 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ typedef _SetNeedsReportTimingsFunc = void Function(bool value);
/// Signature for [PlatformDispatcher.onConfigurationChanged].
typedef PlatformConfigurationChangedCallback = void Function(PlatformConfiguration configuration);

/// Signature for [PlatformDispatcher.onError].
///
/// If this method returns false, the engine may use some fallback method to
/// provide information about the error.
///
/// After calling this method, the process or the VM may terminate. Some severe
/// unhandled errors may not be able to call this method either, such as Dart
/// compilation errors or process terminating errors.
typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace);

// A gesture setting value that indicates it has not been set by the engine.
const double _kUnsetGestureSetting = -1.0;

Expand Down Expand Up @@ -1012,6 +1022,50 @@ class PlatformDispatcher {
);
}

ErrorCallback? _onError;
Zone? _onErrorZone;

/// A callback that is invoked when an unhandled error occurs in the root
/// isolate.
///
/// This callback must return `true` if it has handled the error. Otherwise,
/// it must return `false` and a fallback mechanism such as printing to stderr
/// will be used, as configured by the specific platform embedding via
/// `Settings::unhandled_exception_callback`.
///
/// The VM or the process may exit or become unresponsive after calling this
/// callback. The callback will not be called for exceptions that cause the VM
/// or process to terminate or become unresponsive before the callback can be
/// invoked.
///
/// This callback is not directly invoked by errors in child isolates of the
/// root isolate. Programs that create new isolates must listen for errors on
/// those isolates and forward the errors to the root isolate. An example of
/// this can be found in the Flutter framework's `compute` function.
ErrorCallback? get onError => _onError;
set onError(ErrorCallback? callback) {
_onError = callback;
_onErrorZone = Zone.current;
}

bool _dispatchError(Object error, StackTrace stackTrace) {
if (_onError == null) {
return false;
}
assert(_onErrorZone != null);

if (identical(_onErrorZone, Zone.current)) {
return _onError!(error, stackTrace);
} else {
try {
return _onErrorZone!.runBinary<bool, Object, StackTrace>(_onError!, error, stackTrace);
} catch (e, s) {
_onErrorZone!.handleUncaughtError(e, s);
return false;
}
}
}

/// The route or path that the embedder requested when the application was
/// launched.
///
Expand Down
15 changes: 1 addition & 14 deletions lib/ui/ui_dart_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ std::shared_ptr<VolatilePathTracker> UIDartState::GetVolatilePathTracker()
}

void UIDartState::ScheduleMicrotask(Dart_Handle closure) {
if (tonic::LogIfError(closure) || !Dart_IsClosure(closure)) {
if (tonic::CheckAndHandleError(closure) || !Dart_IsClosure(closure)) {
return;
}

Expand Down Expand Up @@ -186,19 +186,6 @@ tonic::DartErrorHandleType UIDartState::GetLastError() {
return error;
}

void UIDartState::ReportUnhandledException(const std::string& error,
const std::string& stack_trace) {
if (unhandled_exception_callback_ &&
unhandled_exception_callback_(error, stack_trace)) {
return;
}

// Either the exception handler was not set or it could not handle the error,
// just log the exception.
FML_LOG(ERROR) << "Unhandled Exception: " << error << std::endl
<< stack_trace;
}

void UIDartState::LogMessage(const std::string& tag,
const std::string& message) const {
if (log_message_callback_) {
Expand Down
7 changes: 4 additions & 3 deletions lib/ui/ui_dart_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ class UIDartState : public tonic::DartState {

tonic::DartErrorHandleType GetLastError();

void ReportUnhandledException(const std::string& error,
const std::string& stack_trace);

// Logs `print` messages from the application via an embedder-specified
// logging mechanism.
//
Expand All @@ -154,6 +151,10 @@ class UIDartState : public tonic::DartState {
return {std::move(object), std::move(queue)};
};

UnhandledExceptionCallback unhandled_exception_callback() const {
return unhandled_exception_callback_;
}

protected:
UIDartState(TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
Expand Down
Loading

0 comments on commit d24339f

Please sign in to comment.