Skip to content

Commit b7fd24a

Browse files
jamesderlinjamesderlin
authored
[flutter tools] Move _informUserOfCrash into crash_reporting.dart (flutter#55614)
1 parent 0663bf5 commit b7fd24a

File tree

9 files changed

+151
-67
lines changed

9 files changed

+151
-67
lines changed

‎packages/flutter_tools/lib/runner.dart

Lines changed: 18 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import 'src/base/context.dart';
1414
import 'src/base/file_system.dart';
1515
import 'src/base/io.dart';
1616
import 'src/base/logger.dart';
17-
import 'src/base/net.dart';
1817
import 'src/base/process.dart';
1918
import 'src/context_runner.dart';
2019
import 'src/doctor.dart';
@@ -135,20 +134,25 @@ Future<int> _handleToolError(
135134
command: args.join(' '),
136135
);
137136

138-
final String errorString = error.toString();
139-
globals.printError('Oops; flutter has exited unexpectedly: "$errorString".');
137+
globals.printError('Oops; flutter has exited unexpectedly: "$error".');
140138

141139
try {
142-
await _informUserOfCrash(args, error, stackTrace, errorString);
140+
final CrashDetails details = CrashDetails(
141+
command: _crashCommand(args),
142+
error: error,
143+
stackTrace: stackTrace,
144+
doctorText: await _doctorText(),
145+
);
146+
final File file = await _createLocalCrashReport(details);
147+
await globals.crashReporter.informUser(details, file);
143148

144149
return _exit(1);
145150
// This catch catches all exceptions to ensure the message below is printed.
146151
} catch (error) { // ignore: avoid_catches_without_on_clauses
147152
globals.stdio.stderrWrite(
148153
'Unable to generate crash report due to secondary error: $error\n'
149-
'please let us know at https://github.com/flutter/flutter/issues.\n',
150-
);
151-
// Any exception throw here (including one thrown by `_exit()`) will
154+
'${globals.userMessages.flutterToolBugInstructions}\n');
155+
// Any exception thrown here (including one thrown by `_exit()`) will
152156
// get caught by our zone's `onError` handler. In order to avoid an
153157
// infinite error loop, we throw an error that is recognized above
154158
// and will trigger an immediate exit.
@@ -157,42 +161,12 @@ Future<int> _handleToolError(
157161
}
158162
}
159163

160-
Future<void> _informUserOfCrash(List<String> args, dynamic error, StackTrace stackTrace, String errorString) async {
161-
final String doctorText = await _doctorText();
162-
final File file = await _createLocalCrashReport(args, error, stackTrace, doctorText);
163-
164-
globals.printError('A crash report has been written to ${file.path}.');
165-
globals.printStatus('This crash may already be reported. Check GitHub for similar crashes.', emphasis: true);
166-
167-
final HttpClientFactory clientFactory = context.get<HttpClientFactory>();
168-
final GitHubTemplateCreator gitHubTemplateCreator = context.get<GitHubTemplateCreator>() ?? GitHubTemplateCreator(
169-
fileSystem: globals.fs,
170-
logger: globals.logger,
171-
flutterProjectFactory: globals.projectFactory,
172-
client: clientFactory != null ? clientFactory() : HttpClient(),
173-
);
174-
final String similarIssuesURL = GitHubTemplateCreator.toolCrashSimilarIssuesURL(errorString);
175-
globals.printStatus('$similarIssuesURL\n', wrap: false);
176-
globals.printStatus('To report your crash to the Flutter team, first read the guide to filing a bug.', emphasis: true);
177-
globals.printStatus('https://flutter.dev/docs/resources/bug-reports\n', wrap: false);
178-
globals.printStatus('Create a new GitHub issue by pasting this link into your browser and completing the issue template. Thank you!', emphasis: true);
179-
180-
final String command = _crashCommand(args);
181-
final String gitHubTemplateURL = await gitHubTemplateCreator.toolCrashIssueTemplateGitHubURL(
182-
command,
183-
error,
184-
stackTrace,
185-
doctorText
186-
);
187-
globals.printStatus('$gitHubTemplateURL\n', wrap: false);
188-
}
189-
190164
String _crashCommand(List<String> args) => 'flutter ${args.join(' ')}';
191165

192166
String _crashException(dynamic error) => '${error.runtimeType}: $error';
193167

194168
/// Saves the crash report to a local file.
195-
Future<File> _createLocalCrashReport(List<String> args, dynamic error, StackTrace stackTrace, String doctorText) async {
169+
Future<File> _createLocalCrashReport(CrashDetails details) async {
196170
File crashFile = globals.fsUtils.getUniqueFile(
197171
globals.fs.currentDirectory,
198172
'flutter',
@@ -201,17 +175,18 @@ Future<File> _createLocalCrashReport(List<String> args, dynamic error, StackTrac
201175

202176
final StringBuffer buffer = StringBuffer();
203177

204-
buffer.writeln('Flutter crash report; please file at https://github.com/flutter/flutter/issues.\n');
178+
buffer.writeln('Flutter crash report.');
179+
buffer.writeln('${globals.userMessages.flutterToolBugInstructions}\n');
205180

206181
buffer.writeln('## command\n');
207-
buffer.writeln('${_crashCommand(args)}\n');
182+
buffer.writeln('${details.command}\n');
208183

209184
buffer.writeln('## exception\n');
210-
buffer.writeln('${_crashException(error)}\n');
211-
buffer.writeln('```\n$stackTrace```\n');
185+
buffer.writeln('${_crashException(details.error)}\n');
186+
buffer.writeln('```\n${details.stackTrace}```\n');
212187

213188
buffer.writeln('## flutter doctor\n');
214-
buffer.writeln('```\n$doctorText```');
189+
buffer.writeln('```\n${details.doctorText}```');
215190

216191
try {
217192
crashFile.writeAsStringSync(buffer.toString());

‎packages/flutter_tools/lib/src/android/android_device_discovery.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ class AndroidDevices extends PollingDeviceDiscovery {
160160
diagnostics?.add(
161161
'Unexpected failure parsing device information from adb output:\n'
162162
'$line\n'
163-
'Please report a bug at https://github.com/flutter/flutter/issues/new/choose');
163+
'${globals.userMessages.flutterToolBugInstructions}');
164164
}
165165
}
166166
}

‎packages/flutter_tools/lib/src/base/user_messages.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ UserMessages get userMessages => context.get<UserMessages>();
1010

1111
/// Class containing message strings that can be produced by Flutter tools.
1212
class UserMessages {
13+
// Messages used in multiple components.
14+
String get flutterToolBugInstructions =>
15+
'Please report a bug at https://github.com/flutter/flutter/issues.';
16+
1317
// Messages used in FlutterValidator
1418
String flutterStatusInfo(String channel, String version, String os, String locale) =>
1519
'Channel ${channel ?? 'unknown'}, ${version ?? 'Unknown'}, on $os, locale $locale';

‎packages/flutter_tools/lib/src/context_runner.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ Future<T> runInContext<T>(
116116
logger: globals.logger,
117117
platform: globals.platform,
118118
),
119+
CrashReporter: () => CrashReporter(
120+
fileSystem: globals.fs,
121+
logger: globals.logger,
122+
flutterProjectFactory: globals.projectFactory,
123+
client: globals.httpClientFactory?.call() ?? HttpClient(),
124+
),
119125
DevFSConfig: () => DevFSConfig(),
120126
DeviceManager: () => DeviceManager(),
121127
Doctor: () => Doctor(logger: globals.logger),

‎packages/flutter_tools/lib/src/globals.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ Artifacts get artifacts => context.get<Artifacts>();
3939
BuildSystem get buildSystem => context.get<BuildSystem>();
4040
Cache get cache => context.get<Cache>();
4141
Config get config => context.get<Config>();
42+
CrashReporter get crashReporter => context.get<CrashReporter>();
4243
Doctor get doctor => context.get<Doctor>();
44+
HttpClientFactory get httpClientFactory => context.get<HttpClientFactory>();
4345
Logger get logger => context.get<Logger>();
4446
OperatingSystemUtils get os => context.get<OperatingSystemUtils>();
4547
PersistentToolState get persistentToolState => PersistentToolState.instance;

‎packages/flutter_tools/lib/src/reporting/crash_reporting.dart

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,73 @@ const String _kStackTraceFileField = 'DartError';
2626
/// it must be supplied in the request.
2727
const String _kStackTraceFilename = 'stacktrace_file';
2828

29+
class CrashDetails {
30+
CrashDetails({
31+
@required this.command,
32+
@required this.error,
33+
@required this.stackTrace,
34+
@required this.doctorText,
35+
});
36+
37+
final String command;
38+
final dynamic error;
39+
final StackTrace stackTrace;
40+
final String doctorText;
41+
}
42+
43+
/// Reports information about the crash to the user.
44+
class CrashReporter {
45+
CrashReporter({
46+
@required FileSystem fileSystem,
47+
@required Logger logger,
48+
@required FlutterProjectFactory flutterProjectFactory,
49+
@required HttpClient client,
50+
}) : _fileSystem = fileSystem,
51+
_logger = logger,
52+
_flutterProjectFactory = flutterProjectFactory,
53+
_client = client;
54+
55+
final FileSystem _fileSystem;
56+
final Logger _logger;
57+
final FlutterProjectFactory _flutterProjectFactory;
58+
final HttpClient _client;
59+
60+
/// Prints instructions for filing a bug about the crash.
61+
Future<void> informUser(CrashDetails details, File crashFile) async {
62+
_logger.printError('A crash report has been written to ${crashFile.path}.');
63+
_logger.printStatus('This crash may already be reported. Check GitHub for similar crashes.', emphasis: true);
64+
65+
final String similarIssuesURL = GitHubTemplateCreator.toolCrashSimilarIssuesURL(details.error.toString());
66+
_logger.printStatus('$similarIssuesURL\n', wrap: false);
67+
_logger.printStatus('To report your crash to the Flutter team, first read the guide to filing a bug.', emphasis: true);
68+
_logger.printStatus('https://flutter.dev/docs/resources/bug-reports\n', wrap: false);
69+
70+
_logger.printStatus('Create a new GitHub issue by pasting this link into your browser and completing the issue template. Thank you!', emphasis: true);
71+
72+
final GitHubTemplateCreator gitHubTemplateCreator = GitHubTemplateCreator(
73+
fileSystem: _fileSystem,
74+
logger: _logger,
75+
flutterProjectFactory: _flutterProjectFactory,
76+
client: _client,
77+
);
78+
79+
final String gitHubTemplateURL = await gitHubTemplateCreator.toolCrashIssueTemplateGitHubURL(
80+
details.command,
81+
details.error,
82+
details.stackTrace,
83+
details.doctorText,
84+
);
85+
_logger.printStatus('$gitHubTemplateURL\n', wrap: false);
86+
}
87+
}
88+
2989
/// Sends crash reports to Google.
3090
///
31-
/// There are two ways to override the behavior of this class:
32-
///
33-
/// * Define a `FLUTTER_CRASH_SERVER_BASE_URL` environment variable that points
34-
/// to a custom crash reporting server. This is useful if your development
35-
/// environment is behind a firewall and unable to send crash reports to
36-
/// Google, or when you wish to use your own server for collecting crash
37-
/// reports from Flutter Tools.
91+
/// To override the behavior of this class, define a
92+
/// `FLUTTER_CRASH_SERVER_BASE_URL` environment variable that points to a custom
93+
/// crash reporting server. This is useful if your development environment is
94+
/// behind a firewall and unable to send crash reports to Google, or when you
95+
/// wish to use your own server for collecting crash reports from Flutter Tools.
3896
class CrashReportSender {
3997
CrashReportSender({
4098
@required http.Client client,

‎packages/flutter_tools/test/general.shard/crash_reporting_test.dart

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
import 'dart:async';
66
import 'dart:convert';
77

8+
import 'package:file/file.dart';
89
import 'package:file/memory.dart';
910
import 'package:flutter_tools/src/base/io.dart';
1011
import 'package:flutter_tools/src/base/logger.dart';
1112
import 'package:flutter_tools/src/base/os.dart';
1213
import 'package:flutter_tools/src/doctor.dart';
14+
import 'package:flutter_tools/src/project.dart';
1315
import 'package:flutter_tools/src/reporting/reporting.dart';
1416
import 'package:http/http.dart';
1517
import 'package:http/testing.dart';
@@ -18,22 +20,25 @@ import 'package:platform/platform.dart';
1820

1921
import '../src/common.dart';
2022
import '../src/context.dart';
23+
import '../src/testbed.dart';
2124

2225
void main() {
2326
BufferLogger logger;
27+
FileSystem fs;
2428
MockUsage mockUsage;
2529
Platform platform;
2630
OperatingSystemUtils operatingSystemUtils;
2731

2832
setUp(() async {
2933
logger = BufferLogger.test();
34+
fs = MemoryFileSystem.test();
3035

3136
mockUsage = MockUsage();
3237
when(mockUsage.clientId).thenReturn('00000000-0000-4000-0000-000000000000');
3338

3439
platform = FakePlatform(environment: <String, String>{}, operatingSystem: 'linux');
3540
operatingSystemUtils = OperatingSystemUtils(
36-
fileSystem: MemoryFileSystem.test(),
41+
fileSystem: fs,
3742
logger: logger,
3843
platform: platform,
3944
processManager: FakeProcessManager.any(),
@@ -71,6 +76,29 @@ void main() {
7176
expect(logger.traceText, contains('Crash report sent (report ID: test-report-id)'));
7277
}
7378

79+
testWithoutContext('CrashReporter.informUser provides basic instructions', () async {
80+
final CrashReporter crashReporter = CrashReporter(
81+
fileSystem: fs,
82+
logger: logger,
83+
flutterProjectFactory: FlutterProjectFactory(fileSystem: fs, logger: logger),
84+
client: FakeHttpClient(),
85+
);
86+
87+
final File file = fs.file('flutter_00.log');
88+
89+
await crashReporter.informUser(
90+
CrashDetails(
91+
command: 'arg1 arg2 arg3',
92+
error: Exception('Dummy exception'),
93+
stackTrace: StackTrace.current,
94+
doctorText: 'Fake doctor text'),
95+
file,
96+
);
97+
98+
expect(logger.errorText, contains('A crash report has been written to ${file.path}.'));
99+
expect(logger.statusText, contains('https://github.com/flutter/flutter/issues/new'));
100+
});
101+
74102
testWithoutContext('suppress analytics', () async {
75103
when(mockUsage.suppressAnalytics).thenReturn(true);
76104

‎packages/flutter_tools/test/general.shard/runner/runner_test.dart

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:flutter_tools/runner.dart' as runner;
99
import 'package:flutter_tools/src/base/file_system.dart';
1010
import 'package:flutter_tools/src/base/io.dart' as io;
1111
import 'package:flutter_tools/src/base/common.dart';
12+
import 'package:flutter_tools/src/base/user_messages.dart';
1213
import 'package:flutter_tools/src/cache.dart';
1314
import 'package:flutter_tools/src/globals.dart' as globals;
1415
import 'package:flutter_tools/src/reporting/reporting.dart';
@@ -19,11 +20,11 @@ import 'package:platform/platform.dart';
1920
import '../../src/common.dart';
2021
import '../../src/context.dart';
2122

23+
const String kCustomBugInstructions = 'These are instructions to report with a custom bug tracker.';
24+
2225
void main() {
2326
group('runner', () {
24-
MockGitHubTemplateCreator mockGitHubTemplateCreator;
2527
setUp(() {
26-
mockGitHubTemplateCreator = MockGitHubTemplateCreator();
2728
// Instead of exiting with dart:io exit(), this causes an exception to
2829
// be thrown, which we catch with the onError callback in the zone below.
2930
io.setExitFunctionForTests((int _) { throw 'test exit';});
@@ -77,10 +78,7 @@ void main() {
7778
Usage: () => CrashingUsage(),
7879
});
7980

80-
testUsingContext('GitHub issue template', () async {
81-
const String templateURL = 'https://example.com/2';
82-
when(mockGitHubTemplateCreator.toolCrashIssueTemplateGitHubURL(any, any, any, any))
83-
.thenAnswer((_) async => templateURL);
81+
testUsingContext('create local report', () async {
8482
final Completer<void> completer = Completer<void>();
8583
// runner.run() asynchronously calls the exit function set above, so we
8684
// catch it in a zone.
@@ -105,14 +103,22 @@ void main() {
105103
await completer.future;
106104

107105
final String errorText = testLogger.errorText;
108-
expect(errorText, contains('A crash report has been written to /flutter_01.log.'));
109106
expect(errorText, contains('Oops; flutter has exited unexpectedly: "an exception % --".\n'));
110107

111-
final String statusText = testLogger.statusText;
112-
expect(statusText, contains('https://github.com/flutter/flutter/issues?q=is%3Aissue+an+exception+%25+--'));
113-
expect(statusText, contains('https://flutter.dev/docs/resources/bug-reports'));
114-
expect(statusText, contains(templateURL));
115-
108+
final File log = globals.fs.file('/flutter_01.log');
109+
final String logContents = log.readAsStringSync();
110+
expect(logContents, contains(kCustomBugInstructions));
111+
expect(logContents, contains('flutter test'));
112+
expect(logContents, contains('String: an exception % --'));
113+
expect(logContents, contains('CrashingFlutterCommand.runCommand'));
114+
expect(logContents, contains('[✓] Flutter'));
115+
116+
final VerificationResult argVerification = verify(globals.crashReporter.informUser(captureAny, any));
117+
final CrashDetails sentDetails = argVerification.captured.first as CrashDetails;
118+
expect(sentDetails.command, 'flutter test');
119+
expect(sentDetails.error, 'an exception % --');
120+
expect(sentDetails.stackTrace.toString(), contains('CrashingFlutterCommand.runCommand'));
121+
expect(sentDetails.doctorText, contains('[✓] Flutter'));
116122
}, overrides: <Type, Generator>{
117123
Platform: () => FakePlatform(
118124
environment: <String, String>{
@@ -123,7 +129,7 @@ void main() {
123129
),
124130
FileSystem: () => MemoryFileSystem(),
125131
ProcessManager: () => FakeProcessManager.any(),
126-
GitHubTemplateCreator: () => mockGitHubTemplateCreator,
132+
UserMessages: () => CustomBugInstructions(),
127133
});
128134
});
129135
}
@@ -222,3 +228,8 @@ class CrashingUsage implements Usage {
222228
@override
223229
void printWelcome() => _impl.printWelcome();
224230
}
231+
232+
class CustomBugInstructions extends UserMessages {
233+
@override
234+
String get flutterToolBugInstructions => kCustomBugInstructions;
235+
}

0 commit comments

Comments
 (0)