From 9243d0b5326b4c1ffcb31269cad9623a34ee886b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:11:42 -0700 Subject: [PATCH 1/6] Try to map to Dart stack traces --- .../lib/src/framework/app_error_handling.dart | 32 +++++++++++++++++-- packages/devtools_app/pubspec.yaml | 2 ++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/framework/app_error_handling.dart b/packages/devtools_app/lib/src/framework/app_error_handling.dart index 86292c53cf8..151245df7c3 100644 --- a/packages/devtools_app/lib/src/framework/app_error_handling.dart +++ b/packages/devtools_app/lib/src/framework/app_error_handling.dart @@ -3,10 +3,14 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; +import 'package:http/http.dart'; import 'package:logging/logging.dart'; +import 'package:source_map_stack_trace/source_map_stack_trace.dart'; +import 'package:source_maps/source_maps.dart'; import 'package:stack_trace/stack_trace.dart' as stack_trace; import '../shared/analytics/analytics.dart' as ga; @@ -14,6 +18,8 @@ import '../shared/globals.dart'; final _log = Logger('app_error_handling'); +SingleMapping? _sourceMap; + /// Set up error handling for the app. /// /// This method will hook into both the zone error handling and the Flutter @@ -22,7 +28,18 @@ final _log = Logger('app_error_handling'); /// /// [appStartCallback] should be a callback that creates the main Flutter /// application. -void setupErrorHandling(Future Function() appStartCallback) { +Future setupErrorHandling(Future Function() appStartCallback) async { + try { + final sourceMapUri = Uri.parse('main.dart.js.map'); + final sourceMapFile = await get(sourceMapUri); + _sourceMap = SingleMapping.fromJson( + jsonDecode(sourceMapFile.body), + mapUrl: sourceMapUri, + ); + } catch (_) { + // Ignore any errors getting the source map. + } + // First, run all our code in a new zone. unawaited( runZonedGuarded>( @@ -70,7 +87,7 @@ void reportError( bool notifyUser = false, StackTrace? stack, }) { - stack = stack ?? StackTrace.empty; + stack = _maybeMapStackTrace(stack ?? StackTrace.empty); final terseStackTrace = stack_trace.Trace.from(stack).terse.toString(); @@ -86,3 +103,14 @@ void reportError( ); } } + +StackTrace _maybeMapStackTrace(StackTrace stack) { + final sourceMap = _sourceMap; + return sourceMap != null + ? mapStackTrace( + sourceMap, + stack, + minified: true, + ) + : stack; +} diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 278edd699e7..f5c9757120b 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -49,6 +49,8 @@ dependencies: provider: ^6.0.2 # Only used for debug mode logic. shared_preferences: ^2.0.15 + source_map_stack_trace: ^2.1.2 + source_maps: ^0.10.12 sse: ^4.1.2 stack_trace: ^1.10.0 stream_channel: ^2.1.1 From 54b0add23a73ffe8438ae3f37e257699b8376dbd Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 1 Oct 2024 16:15:48 -0700 Subject: [PATCH 2/6] Stack trace mapping works for dart2js and dart2wasm --- flutter-candidate.txt | 2 +- .../lib/src/framework/app_error_handling.dart | 95 ++++++++++++++----- packages/devtools_app/pubspec.yaml | 2 +- tool/build_release.sh | 1 + tool/lib/commands/build.dart | 1 + 5 files changed, 74 insertions(+), 27 deletions(-) diff --git a/flutter-candidate.txt b/flutter-candidate.txt index 9c9b22f0851..0ad61c3c4de 100644 --- a/flutter-candidate.txt +++ b/flutter-candidate.txt @@ -1 +1 @@ -8925e1ffdfe880b06a8bc5877f017083d6652f5b +d59499988ac50c18a3d492f953c71efcc101b1a5 diff --git a/packages/devtools_app/lib/src/framework/app_error_handling.dart b/packages/devtools_app/lib/src/framework/app_error_handling.dart index 151245df7c3..cca200ad6d6 100644 --- a/packages/devtools_app/lib/src/framework/app_error_handling.dart +++ b/packages/devtools_app/lib/src/framework/app_error_handling.dart @@ -18,8 +18,6 @@ import '../shared/globals.dart'; final _log = Logger('app_error_handling'); -SingleMapping? _sourceMap; - /// Set up error handling for the app. /// /// This method will hook into both the zone error handling and the Flutter @@ -28,25 +26,15 @@ SingleMapping? _sourceMap; /// /// [appStartCallback] should be a callback that creates the main Flutter /// application. -Future setupErrorHandling(Future Function() appStartCallback) async { - try { - final sourceMapUri = Uri.parse('main.dart.js.map'); - final sourceMapFile = await get(sourceMapUri); - _sourceMap = SingleMapping.fromJson( - jsonDecode(sourceMapFile.body), - mapUrl: sourceMapUri, - ); - } catch (_) { - // Ignore any errors getting the source map. - } - +void setupErrorHandling(Future Function() appStartCallback) { // First, run all our code in a new zone. unawaited( runZonedGuarded>( - // ignore: avoid-passing-async-when-sync-expected this ignore should be fixed. - () { + () async { WidgetsFlutterBinding.ensureInitialized(); + await _initializeSourceMapping(); + final FlutterExceptionHandler? oldHandler = FlutterError.onError; FlutterError.onError = (FlutterErrorDetails details) { @@ -87,13 +75,29 @@ void reportError( bool notifyUser = false, StackTrace? stack, }) { - stack = _maybeMapStackTrace(stack ?? StackTrace.empty); - - final terseStackTrace = stack_trace.Trace.from(stack).terse.toString(); + unawaited( + _reportError( + error, + errorType: errorType, + notifyUser: notifyUser, + stack: stack, + ).catchError((_) { + // Ignore errors. + }), + ); +} - _log.severe('[$errorType]: ${error.toString()}', error, stack); +Future _reportError( + Object error, { + String errorType = 'DevToolsError', + bool notifyUser = false, + StackTrace? stack, +}) async { + final terseStackTrace = await _mapAndTersify(stack); + final errorMessage = '$error\n$terseStackTrace'; - ga.reportError('$error\n$terseStackTrace'); + _log.severe('[$errorType]: $errorMessage', error, stack); + ga.reportError(errorMessage); // Show error message in a notification pop-up: if (notifyUser) { @@ -104,11 +108,52 @@ void reportError( } } -StackTrace _maybeMapStackTrace(StackTrace stack) { - final sourceMap = _sourceMap; - return sourceMap != null +SingleMapping? _cachedJsSourceMapping; +SingleMapping? _cachedWasmSourceMapping; + +Future _fetchSourceMapping() async { + final cachedSourceMapping = preferences.wasmEnabled.value + ? _cachedWasmSourceMapping + : _cachedJsSourceMapping; + + return cachedSourceMapping ?? (await _initializeSourceMapping()); +} + +Future _initializeSourceMapping() async { + try { + final wasmEnabled = preferences.wasmEnabled.value; + final sourceMapUri = Uri.parse( + 'main.dart.${wasmEnabled ? 'wasm' : 'js'}.map', + ); + final sourceMapFile = await get(sourceMapUri); + + return SingleMapping.fromJson( + jsonDecode(sourceMapFile.body), + mapUrl: sourceMapUri, + ); + } catch (_) { + // Ignore any errors loading the source map. + return Future.value(); + } +} + +Future _mapAndTersify(StackTrace? stack) async { + final originalStackTrace = stack; + if (originalStackTrace == null) return ''; + + final mappedStackTrace = await _maybeMapStackTrace(originalStackTrace); + // If mapping fails, revert back to the original source map: + final stackTrace = mappedStackTrace.toString().isEmpty + ? originalStackTrace + : mappedStackTrace; + return stack_trace.Trace.from(stackTrace).terse.toString(); +} + +Future _maybeMapStackTrace(StackTrace stack) async { + final sourceMapping = await _fetchSourceMapping(); + return sourceMapping != null ? mapStackTrace( - sourceMap, + sourceMapping, stack, minified: true, ) diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index f5c9757120b..b09712e9d19 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -52,7 +52,7 @@ dependencies: source_map_stack_trace: ^2.1.2 source_maps: ^0.10.12 sse: ^4.1.2 - stack_trace: ^1.10.0 + stack_trace: ^1.12.0 stream_channel: ^2.1.1 string_scanner: ^1.1.0 unified_analytics: ^6.1.3 diff --git a/tool/build_release.sh b/tool/build_release.sh index c88203ceef9..4c14b9b9e86 100755 --- a/tool/build_release.sh +++ b/tool/build_release.sh @@ -77,6 +77,7 @@ rm -rf build/web flutter pub get flutter build web \ + --source-maps \ --wasm \ --pwa-strategy=offline-first \ --release \ diff --git a/tool/lib/commands/build.dart b/tool/lib/commands/build.dart index 8668ee2b2d3..d0c8415d7c2 100644 --- a/tool/lib/commands/build.dart +++ b/tool/lib/commands/build.dart @@ -101,6 +101,7 @@ class BuildCommand extends Command { [ 'build', 'web', + '--source-maps', if (useWasm) ...[ BuildCommandArgs.wasm.asArg(), if (noStripWasm) BuildCommandArgs.noStripWasm.asArg(), From 4d967d13fb0b5150bac0abb2e9a07b56a7c371f9 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 2 Oct 2024 09:56:35 -0700 Subject: [PATCH 3/6] Update release notes --- packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index df3400cc6f8..d6df8530e41 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -25,6 +25,9 @@ show. - [#8277](https://github.com/flutter/devtools/pull/8277) * Added support for loading extensions in pub workspaces [8347](https://github.com/flutter/devtools/pull/8347). +* Mapped error stacktraces to use the Dart sourcecode locations so that they are human- + readable. - [#8385](https://github.com/flutter/devtools/pull/8385) + ## Inspector updates - Added a setting to the Flutter Inspector controls that allows users to opt-in to the newly redesigned Flutter Inspector. - [#8342](https://github.com/flutter/devtools/pull/8342) From 5a48068b3959db789e78545aa93b022d7e0cd32b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 2 Oct 2024 09:58:39 -0700 Subject: [PATCH 4/6] Respond to PR comments --- .../lib/src/framework/app_error_handling.dart | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/devtools_app/lib/src/framework/app_error_handling.dart b/packages/devtools_app/lib/src/framework/app_error_handling.dart index cca200ad6d6..35b5eec31f6 100644 --- a/packages/devtools_app/lib/src/framework/app_error_handling.dart +++ b/packages/devtools_app/lib/src/framework/app_error_handling.dart @@ -112,18 +112,16 @@ SingleMapping? _cachedJsSourceMapping; SingleMapping? _cachedWasmSourceMapping; Future _fetchSourceMapping() async { - final cachedSourceMapping = preferences.wasmEnabled.value - ? _cachedWasmSourceMapping - : _cachedJsSourceMapping; + final cachedSourceMapping = + kIsWasm ? _cachedWasmSourceMapping : _cachedJsSourceMapping; return cachedSourceMapping ?? (await _initializeSourceMapping()); } Future _initializeSourceMapping() async { try { - final wasmEnabled = preferences.wasmEnabled.value; final sourceMapUri = Uri.parse( - 'main.dart.${wasmEnabled ? 'wasm' : 'js'}.map', + 'main.dart.${kIsWasm ? 'wasm' : 'js'}.map', ); final sourceMapFile = await get(sourceMapUri); @@ -133,7 +131,7 @@ Future _initializeSourceMapping() async { ); } catch (_) { // Ignore any errors loading the source map. - return Future.value(); + return null; } } From 7727d28735a1e3e474dbadfae2f1237529d39590 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:00:29 -0700 Subject: [PATCH 5/6] typo --- packages/devtools_app/lib/src/framework/app_error_handling.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/src/framework/app_error_handling.dart b/packages/devtools_app/lib/src/framework/app_error_handling.dart index 35b5eec31f6..ed2ceea3832 100644 --- a/packages/devtools_app/lib/src/framework/app_error_handling.dart +++ b/packages/devtools_app/lib/src/framework/app_error_handling.dart @@ -140,7 +140,7 @@ Future _mapAndTersify(StackTrace? stack) async { if (originalStackTrace == null) return ''; final mappedStackTrace = await _maybeMapStackTrace(originalStackTrace); - // If mapping fails, revert back to the original source map: + // If mapping fails, revert back to the original stack trace: final stackTrace = mappedStackTrace.toString().isEmpty ? originalStackTrace : mappedStackTrace; From 8f31a286c352465673e5f8668247efe4096d0d8d Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:01:28 -0700 Subject: [PATCH 6/6] Another typo --- packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index d6df8530e41..bfa84f83075 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -25,7 +25,7 @@ show. - [#8277](https://github.com/flutter/devtools/pull/8277) * Added support for loading extensions in pub workspaces [8347](https://github.com/flutter/devtools/pull/8347). -* Mapped error stacktraces to use the Dart sourcecode locations so that they are human- +* Mapped error stacktraces to use the Dart source code locations so that they are human- readable. - [#8385](https://github.com/flutter/devtools/pull/8385) ## Inspector updates