From 6987c9fb8248e8e8ae85eeeb11f084fce007dcd7 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Tue, 12 Oct 2021 13:48:03 -0700 Subject: [PATCH] Migrate crash_reporting and github_template (#91516) --- packages/flutter_tools/lib/src/globals.dart | 2 - .../lib/src/globals_null_migrated.dart | 2 + .../lib/src/reporting/crash_reporting.dart | 41 ++++---- .../lib/src/reporting/github_template.dart | 22 ++--- .../general.shard/crash_reporting_test.dart | 93 ++++++++----------- .../general.shard/runner/runner_test.dart | 2 +- 6 files changed, 73 insertions(+), 89 deletions(-) diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index 764cddb8165..27d410db3c5 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -8,11 +8,9 @@ import 'base/context.dart'; import 'doctor.dart'; import 'ios/simulators.dart'; import 'macos/xcdevice.dart'; -import 'reporting/crash_reporting.dart'; export 'globals_null_migrated.dart'; -CrashReporter get crashReporter => context.get(); Doctor get doctor => context.get(); IOSSimulatorUtils get iosSimulatorUtils => context.get(); diff --git a/packages/flutter_tools/lib/src/globals_null_migrated.dart b/packages/flutter_tools/lib/src/globals_null_migrated.dart index 89c91ab4447..85c83d88008 100644 --- a/packages/flutter_tools/lib/src/globals_null_migrated.dart +++ b/packages/flutter_tools/lib/src/globals_null_migrated.dart @@ -37,6 +37,7 @@ import 'macos/cocoapods_validator.dart'; import 'macos/xcode.dart'; import 'persistent_tool_state.dart'; import 'project.dart'; +import 'reporting/crash_reporting.dart'; import 'reporting/reporting.dart'; import 'runner/local_engine.dart'; import 'version.dart'; @@ -49,6 +50,7 @@ BuildSystem? get buildSystem => context.get(); Cache get cache => context.get()!; CocoaPodsValidator? get cocoapodsValidator => context.get(); Config get config => context.get()!; +CrashReporter? get crashReporter => context.get(); DeviceManager? get deviceManager => context.get(); HttpClientFactory? get httpClientFactory => context.get(); Logger get logger => context.get()!; diff --git a/packages/flutter_tools/lib/src/reporting/crash_reporting.dart b/packages/flutter_tools/lib/src/reporting/crash_reporting.dart index 732c03280b2..d6f4f94ecea 100644 --- a/packages/flutter_tools/lib/src/reporting/crash_reporting.dart +++ b/packages/flutter_tools/lib/src/reporting/crash_reporting.dart @@ -2,13 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.8 - import 'dart:async'; import 'package:file/file.dart'; import 'package:http/http.dart' as http; -import 'package:meta/meta.dart'; import '../base/file_system.dart'; import '../base/io.dart'; @@ -43,14 +40,14 @@ const String _kStackTraceFilename = 'stacktrace_file'; class CrashDetails { CrashDetails({ - @required this.command, - @required this.error, - @required this.stackTrace, - @required this.doctorText, + required this.command, + required this.error, + required this.stackTrace, + required this.doctorText, }); final String command; - final dynamic error; + final Object error; final StackTrace stackTrace; final String doctorText; } @@ -58,10 +55,10 @@ class CrashDetails { /// Reports information about the crash to the user. class CrashReporter { CrashReporter({ - @required FileSystem fileSystem, - @required Logger logger, - @required FlutterProjectFactory flutterProjectFactory, - @required HttpClient client, + required FileSystem fileSystem, + required Logger logger, + required FlutterProjectFactory flutterProjectFactory, + required HttpClient client, }) : _fileSystem = fileSystem, _logger = logger, _flutterProjectFactory = flutterProjectFactory, @@ -110,11 +107,11 @@ class CrashReporter { /// wish to use your own server for collecting crash reports from Flutter Tools. class CrashReportSender { CrashReportSender({ - http.Client client, - @required Usage usage, - @required Platform platform, - @required Logger logger, - @required OperatingSystemUtils operatingSystemUtils, + http.Client? client, + required Usage usage, + required Platform platform, + required Logger logger, + required OperatingSystemUtils operatingSystemUtils, }) : _client = client ?? http.Client(), _usage = usage, _platform = platform, @@ -130,7 +127,7 @@ class CrashReportSender { bool _crashReportSent = false; Uri get _baseUrl { - final String overrideUrl = _platform.environment['FLUTTER_CRASH_SERVER_BASE_URL']; + final String? overrideUrl = _platform.environment['FLUTTER_CRASH_SERVER_BASE_URL']; if (overrideUrl != null) { return Uri.parse(overrideUrl); @@ -147,10 +144,10 @@ class CrashReportSender { /// /// The report is populated from data in [error] and [stackTrace]. Future sendReport({ - @required dynamic error, - @required StackTrace stackTrace, - @required String Function() getFlutterVersion, - @required String command, + required Object error, + required StackTrace stackTrace, + required String Function() getFlutterVersion, + required String command, }) async { // Only send one crash report per run. if (_crashReportSent) { diff --git a/packages/flutter_tools/lib/src/reporting/github_template.dart b/packages/flutter_tools/lib/src/reporting/github_template.dart index 31ff0378c74..f98d1b038e7 100644 --- a/packages/flutter_tools/lib/src/reporting/github_template.dart +++ b/packages/flutter_tools/lib/src/reporting/github_template.dart @@ -2,13 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.8 - import 'dart:async'; import 'package:file/file.dart'; import 'package:intl/intl.dart'; -import 'package:meta/meta.dart'; import '../base/file_system.dart'; import '../base/io.dart'; @@ -25,10 +22,10 @@ import '../version.dart'; /// Provide suggested GitHub issue templates to user when Flutter encounters an error. class GitHubTemplateCreator { GitHubTemplateCreator({ - @required FileSystem fileSystem, - @required Logger logger, - @required FlutterProjectFactory flutterProjectFactory, - @required HttpClient client, + required FileSystem fileSystem, + required Logger logger, + required FlutterProjectFactory flutterProjectFactory, + required HttpClient client, }) : _fileSystem = fileSystem, _logger = logger, _flutterProjectFactory = flutterProjectFactory, @@ -44,7 +41,7 @@ class GitHubTemplateCreator { } /// Restricts exception object strings to contain only information about tool internals. - static String sanitizedCrashException(dynamic error) { + static String sanitizedCrashException(Object error) { if (error is ProcessException) { // Suppress args. return 'ProcessException: ${error.message} Command: ${error.executable}, OS error code: ${error.errorCode}'; @@ -83,7 +80,7 @@ class GitHubTemplateCreator { /// Shorten the URL, if possible. Future toolCrashIssueTemplateGitHubURL( String command, - dynamic error, + Object error, StackTrace stackTrace, String doctorText ) async { @@ -131,13 +128,14 @@ ${_projectMetadataInformation()} return exception.toString(); } try { - final FlutterManifest manifest = project?.manifest; + final FlutterManifest manifest = project.manifest; if (project == null || manifest == null || manifest.isEmpty) { return 'No pubspec in working directory.'; } final FlutterProjectMetadata metadata = FlutterProjectMetadata(project.metadataFile, _logger); + final FlutterProjectType? projectType = metadata.projectType; final StringBuffer description = StringBuffer() - ..writeln('**Type**: ${flutterProjectTypeToString(metadata.projectType)}') + ..writeln('**Type**: ${projectType == null ? 'malformed' : flutterProjectTypeToString(projectType)}') ..writeln('**Version**: ${manifest.appVersion}') ..writeln('**Material**: ${manifest.usesMaterialDesign}') ..writeln('**Android X**: ${manifest.usesAndroidX}') @@ -175,7 +173,7 @@ ${_projectMetadataInformation()} /// /// See https://github.blog/2011-11-10-git-io-github-url-shortener. Future _shortURL(String fullURL) async { - String url; + String? url; try { _logger.printTrace('Attempting git.io shortener: $fullURL'); final List bodyBytes = utf8.encode('url=${Uri.encodeQueryComponent(fullURL)}'); diff --git a/packages/flutter_tools/test/general.shard/crash_reporting_test.dart b/packages/flutter_tools/test/general.shard/crash_reporting_test.dart index 1ad00b61cd2..d0c893be219 100644 --- a/packages/flutter_tools/test/general.shard/crash_reporting_test.dart +++ b/packages/flutter_tools/test/general.shard/crash_reporting_test.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.8 - import 'dart:convert'; import 'package:file/file.dart'; @@ -12,8 +10,6 @@ import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/base/platform.dart'; -import 'package:flutter_tools/src/doctor.dart'; -import 'package:flutter_tools/src/doctor_validator.dart'; import 'package:flutter_tools/src/project.dart'; import 'package:flutter_tools/src/reporting/crash_reporting.dart'; import 'package:flutter_tools/src/reporting/reporting.dart'; @@ -25,11 +21,12 @@ import '../src/fake_http_client.dart'; import '../src/fake_process_manager.dart'; void main() { - BufferLogger logger; - FileSystem fs; - TestUsage testUsage; - Platform platform; - OperatingSystemUtils operatingSystemUtils; + late BufferLogger logger; + late FileSystem fs; + late TestUsage testUsage; + late Platform platform; + late OperatingSystemUtils operatingSystemUtils; + late StackTrace stackTrace; setUp(() async { logger = BufferLogger.test(); @@ -45,6 +42,9 @@ void main() { ); MockCrashReportSender.sendCalls = 0; + stackTrace = StackTrace.fromString(''' +#0 _File.open. (dart:io/file_impl.dart:366:9) +#1 _rootRunUnary (dart:async/zone.dart:1141:38)'''); }); Future verifyCrashReportSent(RequestInfo crashInfo, { @@ -62,15 +62,15 @@ void main() { 'version': 'test-version', }, )); - expect(crashInfo.fields['uuid'], testUsage.clientId); - expect(crashInfo.fields['product'], 'Flutter_Tools'); - expect(crashInfo.fields['version'], 'test-version'); - expect(crashInfo.fields['osName'], 'linux'); - expect(crashInfo.fields['osVersion'], 'Linux'); - expect(crashInfo.fields['type'], 'DartError'); - expect(crashInfo.fields['error_runtime_type'], 'StateError'); - expect(crashInfo.fields['error_message'], 'Bad state: Test bad state error'); - expect(crashInfo.fields['comments'], 'crash'); + expect(crashInfo.fields?['uuid'], testUsage.clientId); + expect(crashInfo.fields?['product'], 'Flutter_Tools'); + expect(crashInfo.fields?['version'], 'test-version'); + expect(crashInfo.fields?['osName'], 'linux'); + expect(crashInfo.fields?['osVersion'], 'Linux'); + expect(crashInfo.fields?['type'], 'DartError'); + expect(crashInfo.fields?['error_runtime_type'], 'StateError'); + expect(crashInfo.fields?['error_message'], 'Bad state: Test bad state error'); + expect(crashInfo.fields?['comments'], 'crash'); expect(logger.traceText, contains('Sending crash report to Google.')); expect(logger.traceText, contains('Crash report sent (report ID: test-report-id)')); @@ -112,7 +112,7 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -138,7 +138,7 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -157,7 +157,7 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -176,7 +176,7 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -195,7 +195,7 @@ void main() { await crashReportSender.sendReport( error: ClientException('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -216,28 +216,28 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -247,8 +247,8 @@ void main() { }); testWithoutContext('should not send a crash report if on a user-branch', () async { - String method; - Uri uri; + String? method; + Uri? uri; final MockClient mockClient = MockClient((Request request) async { method = request.method; @@ -270,7 +270,7 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => '[user-branch]/v1.2.3', command: 'crash', ); @@ -283,7 +283,7 @@ void main() { }); testWithoutContext('can override base URL', () async { - Uri uri; + Uri? uri; final MockClient mockClient = MockClient((Request request) async { uri = request.url; return Response('test-report-id', 200); @@ -307,7 +307,7 @@ void main() { await crashReportSender.sendReport( error: StateError('Test bad state error'), - stackTrace: null, + stackTrace: stackTrace, getFlutterVersion: () => 'test-version', command: 'crash', ); @@ -329,9 +329,9 @@ void main() { } class RequestInfo { - String method; - Uri uri; - Map fields; + String? method; + Uri? uri; + Map? fields; } class MockCrashReportSender extends MockClient { @@ -341,21 +341,20 @@ class MockCrashReportSender extends MockClient { crashInfo.uri = request.url; // A very ad-hoc multipart request parser. Good enough for this test. - String boundary = request.headers['Content-Type']; - boundary = boundary.substring(boundary.indexOf('boundary=') + 9); + String? boundary = request.headers['Content-Type']; + boundary = boundary?.substring(boundary.indexOf('boundary=') + 9); crashInfo.fields = Map.fromIterable( utf8.decode(request.bodyBytes) .split('--$boundary') - .map>((String part) { - final Match nameMatch = RegExp(r'name="(.*)"').firstMatch(part); + .map?>((String part) { + final Match? nameMatch = RegExp(r'name="(.*)"').firstMatch(part); if (nameMatch == null) { return null; } - final String name = nameMatch[1]; + final String name = nameMatch[1]!; final String value = part.split('\n').skip(2).join('\n').trim(); return [name, value]; - }) - .where((List pair) => pair != null), + }).whereType>(), key: (dynamic key) { final List pair = key as List; return pair[0]; @@ -380,13 +379,3 @@ class CrashingCrashReportSender extends MockClient { throw exception; }); } - -/// A DoctorValidatorsProvider that overrides the default validators without -/// overriding the doctor. -class FakeDoctorValidatorsProvider implements DoctorValidatorsProvider { - @override - List get validators => []; - - @override - List get workflows => []; -} diff --git a/packages/flutter_tools/test/general.shard/runner/runner_test.dart b/packages/flutter_tools/test/general.shard/runner/runner_test.dart index 752ffd13d01..457edc36f73 100644 --- a/packages/flutter_tools/test/general.shard/runner/runner_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/runner_test.dart @@ -15,7 +15,7 @@ import 'package:flutter_tools/src/base/io.dart' as io; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/user_messages.dart'; import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/globals.dart' as globals; +import 'package:flutter_tools/src/globals_null_migrated.dart' as globals; import 'package:flutter_tools/src/reporting/crash_reporting.dart'; import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart';