Skip to content

Commit

Permalink
improve propagation of error messages (#2738)
Browse files Browse the repository at this point in the history
  • Loading branch information
devoncarew committed Nov 30, 2023
1 parent 43a4cb3 commit 7d62c04
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 97 deletions.
6 changes: 2 additions & 4 deletions pkgs/dart_pad/lib/sharing/editor_ui.dart
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,10 @@ abstract class EditorUi {
return !hasErrors && !hasWarnings;
} catch (e) {
if (e is! TimeoutException) {
final message = e is ApiRequestError ? e.message : '$e';
displayIssues([
AnalysisIssue(
kind: 'error',
message: message,
message: '$e',
// set invalid line number, so NO line # will be displayed
location: Location(line: -1),
)
Expand Down Expand Up @@ -250,10 +249,9 @@ abstract class EditorUi {
return true;
} catch (e) {
ga.sendException('${e.runtimeType}');
final message = e is ApiRequestError ? e.message : '$e';
showSnackbar('Error compiling to JavaScript');
clearOutput();
showOutput('Error compiling to JavaScript:\n$message', error: true);
showOutput('Error compiling to JavaScript:\n$e', error: true);
return false;
} finally {
runButton.disabled = false;
Expand Down
10 changes: 8 additions & 2 deletions pkgs/dart_services/lib/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,14 @@ Middleware exceptionResponse() {
return (Request request) async {
try {
return await handler(request);
} catch (e) {
return Response.badRequest(body: e is BadRequest ? e.message : '$e');
} catch (e, st) {
if (e is BadRequest) {
return Response.badRequest(body: e.message);
}

_logger.severe('${request.requestedUri.path} $e', null, st);

return Response.badRequest(body: '$e');
}
};
};
Expand Down
25 changes: 12 additions & 13 deletions pkgs/dart_services/lib/src/analysis.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,26 @@ class Analyzer {
return analysisServer.analyze(source, imports: imports);
}

Future<api.CompleteResponse> completeV3(String source, int offset) async {
Future<api.CompleteResponse> complete(String source, int offset) async {
await _checkPackageReferences(getAllImportsFor(source));

return analysisServer.completeV3(source, offset);
return analysisServer.complete(source, offset);
}

Future<api.FixesResponse> fixesV3(String source, int offset) async {
Future<api.FixesResponse> fixes(String source, int offset) async {
await _checkPackageReferences(getAllImportsFor(source));

return analysisServer.fixesV3(source, offset);
return analysisServer.fixes(source, offset);
}

Future<api.FormatResponse> format(String source, int? offset) async {
await _checkPackageReferences(getAllImportsFor(source));

return analysisServer.format(source, offset);
}

Future<api.DocumentResponse> dartdocV3(String source, int offset) async {
Future<api.DocumentResponse> dartdoc(String source, int offset) async {
await _checkPackageReferences(getAllImportsFor(source));

return analysisServer.dartdocV3(source, offset);
return analysisServer.dartdoc(source, offset);
}

/// Check that the set of packages referenced is valid.
Expand All @@ -83,8 +81,9 @@ class Analyzer {

if (unsupportedImports.isNotEmpty) {
final unsupportedUris =
unsupportedImports.map((import) => import.uri.stringValue);
throw BadRequest('Unsupported import(s): $unsupportedUris');
unsupportedImports.map((import) => import.uri.stringValue).toList();
final plural = unsupportedUris.length == 1 ? 'import' : 'imports';
throw BadRequest('unsupported $plural ${unsupportedUris.join(', ')}');
}
}

Expand Down Expand Up @@ -149,7 +148,7 @@ class AnalysisServerWrapper {
});
}

Future<api.CompleteResponse> completeV3(String source, int offset) async {
Future<api.CompleteResponse> complete(String source, int offset) async {
final results = await _completeImpl(
{kMainDart: source},
kMainDart,
Expand Down Expand Up @@ -208,7 +207,7 @@ class AnalysisServerWrapper {
);
}

Future<api.FixesResponse> fixesV3(String src, int offset) async {
Future<api.FixesResponse> fixes(String src, int offset) async {
final mainFile = _getPathFromName(kMainDart);

await _loadSources({mainFile: src});
Expand Down Expand Up @@ -260,7 +259,7 @@ class AnalysisServerWrapper {
});
}

Future<api.DocumentResponse> dartdocV3(String src, int offset) async {
Future<api.DocumentResponse> dartdoc(String src, int offset) async {
final sourcepath = _getPathFromName(kMainDart);

await _loadSources(_getOverlayMapWithPaths({kMainDart: src}));
Expand Down
8 changes: 4 additions & 4 deletions pkgs/dart_services/lib/src/common_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class CommonServerApi {
api.SourceRequest.fromJson(await request.readAsJson());

final result = await serialize(() =>
impl.analyzer.completeV3(sourceRequest.source, sourceRequest.offset!));
impl.analyzer.complete(sourceRequest.source, sourceRequest.offset!));

return ok(result.toJson());
}
Expand All @@ -152,8 +152,8 @@ class CommonServerApi {
final sourceRequest =
api.SourceRequest.fromJson(await request.readAsJson());

final result = await serialize(() =>
impl.analyzer.fixesV3(sourceRequest.source, sourceRequest.offset!));
final result = await serialize(
() => impl.analyzer.fixes(sourceRequest.source, sourceRequest.offset!));

return ok(result.toJson());
}
Expand Down Expand Up @@ -183,7 +183,7 @@ class CommonServerApi {
api.SourceRequest.fromJson(await request.readAsJson());

final result = await serialize(() {
return impl.analyzer.dartdocV3(
return impl.analyzer.dartdoc(
sourceRequest.source,
sourceRequest.offset!,
);
Expand Down
45 changes: 25 additions & 20 deletions pkgs/dart_services/lib/src/compiling.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,6 @@ class Compiler {
String source, {
bool returnSourceMap = false,
}) async {
final imports = getAllImportsFor(source);
final unsupportedImports =
getUnsupportedImports(imports, sourceFiles: {kMainDart});
if (unsupportedImports.isNotEmpty) {
return CompilationResults(problems: [
for (final import in unsupportedImports)
CompilationProblem._('unsupported import: ${import.uri.stringValue}'),
]);
}

final temp = Directory.systemTemp.createTempSync('dartpad');
_logger.fine('Temp directory created: ${temp.path}');

Expand Down Expand Up @@ -120,14 +110,6 @@ class Compiler {
/// Compile the given string and return the resulting [DDCCompilationResults].
Future<DDCCompilationResults> compileDDC(String source) async {
final imports = getAllImportsFor(source);
final unsupportedImports =
getUnsupportedImports(imports, sourceFiles: {kMainDart});
if (unsupportedImports.isNotEmpty) {
return DDCCompilationResults.failed([
for (final import in unsupportedImports)
CompilationProblem._('unsupported import: ${import.uri.stringValue}'),
]);
}

final temp = Directory.systemTemp.createTempSync('dartpad');
_logger.fine('Temp directory created: ${temp.path}');
Expand Down Expand Up @@ -183,8 +165,9 @@ class Compiler {
await _ddcDriver.doWork(WorkRequest()..arguments.addAll(arguments));

if (response.exitCode != 0) {
return DDCCompilationResults.failed(
[CompilationProblem._(response.output)]);
return DDCCompilationResults.failed([
CompilationProblem._(_rewritePaths(response.output)),
]);
} else {
// The `--single-out-file` option for dartdevc was removed in v2.7.0. As
// a result, the JS code produced above does *not* provide a name for
Expand Down Expand Up @@ -313,3 +296,25 @@ bool _doNothing(String from, String to) {
}
return false;
}

/// Remove any references to 'bootstrap.dart' and replace with referenced to
/// 'main.dart'.
String _rewritePaths(String output) {
final lines = output.split('\n');

return lines.map((line) {
final token1 = 'lib/bootstrap.dart:';
var index = line.indexOf(token1);
if (index != -1) {
return 'main.dart:${line.substring(index + token1.length)}';
}

final token2 = 'lib/main.dart:';
index = line.indexOf(token2);
if (index != -1) {
return 'main.dart:${line.substring(index + token2.length)}';
}

return line;
}).join('\n');
}
14 changes: 12 additions & 2 deletions pkgs/dart_services/lib/src/logging.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,23 @@ import 'package:logging/logging.dart';

const bool verboseLogging = false;

final _wsRegex = RegExp(r' \s+');

void emitLogsToStdout() {
Logger.root.onRecord.listen((LogRecord record) {
if (verboseLogging || record.level >= Level.INFO) {
print('[${record.level.name.toLowerCase()}] ${record.message}');
var stackTrace = '';
if (record.stackTrace != null) {
print(record.stackTrace);
var lines = record.stackTrace!.toString().split('\n').take(5).join(' ');
lines = lines.replaceAll(_wsRegex, ' ');
stackTrace = ' $lines';
}

print(
'[${record.level.name.toLowerCase()}] '
'${record.message}'
'$stackTrace',
);
}
});
}
2 changes: 1 addition & 1 deletion pkgs/dart_services/lib/src/project_templates.dart
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ String? _packageNameFromPackageUri(String uriString) {
///
/// Note: The filenames in [sourceFiles] were sanitized of any
/// 'package:'/etc syntax as the file set arrives from the endpoint, and
/// before being passed to [getUnsupportedImports].This is done so
/// before being passed to [getUnsupportedImports]. This is done so
/// the list can't be used to bypass unsupported imports.
List<ImportDirective> getUnsupportedImports(
List<ImportDirective> imports, {
Expand Down
19 changes: 8 additions & 11 deletions pkgs/dart_services/lib/src/shared/services.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 8 additions & 10 deletions pkgs/dart_services/test/analysis_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ void defineTests() {

test('simple_completion', () async {
// Just after `i.` on line 3 of [completionCode]
final results = await analysisServer.completeV3(completionCode, 32);
final results = await analysisServer.complete(completionCode, 32);
expect(results.replacementLength, 0);
expect(results.replacementOffset, 32);
final completions = results.suggestions.map((c) => c.completion).toList();
Expand All @@ -48,11 +48,9 @@ void defineTests() {

test('completions polluted on second request (repro #126)', () async {
// https://github.com/dart-lang/dart-services/issues/126
return analysisServer
.completeV3(completionFilterCode, 17)
.then((results) {
return analysisServer.complete(completionFilterCode, 17).then((results) {
return analysisServer
.completeV3(completionFilterCode, 17)
.complete(completionFilterCode, 17)
.then((results) {
expect(results.replacementLength, 2);
expect(results.replacementOffset, 16);
Expand All @@ -66,7 +64,7 @@ void defineTests() {
// We're testing here that we don't have any path imports - we don't want
// to enable browsing the file system.
final testCode = "import '/'; main() { int a = 0; a. }";
final results = await analysisServer.completeV3(testCode, 9);
final results = await analysisServer.complete(testCode, 9);
final completions = results.suggestions;

if (completions.isNotEmpty) {
Expand All @@ -81,7 +79,7 @@ void defineTests() {
// Ensure we can import dart: imports.
final testCode = "import 'dart:c'; main() { int a = 0; a. }";

final results = await analysisServer.completeV3(testCode, 14);
final results = await analysisServer.complete(testCode, 14);
final completions = results.suggestions;

expect(
Expand All @@ -98,13 +96,13 @@ void defineTests() {

test('import_and_other_test', () async {
final testCode = "import '/'; main() { int a = 0; a. }";
final results = await analysisServer.completeV3(testCode, 34);
final results = await analysisServer.complete(testCode, 34);

expect(completionsContains(results, 'abs'), true);
});

test('quickFix simple', () async {
final results = await analysisServer.fixesV3(quickFixesCode, 25);
final results = await analysisServer.fixes(quickFixesCode, 25);
final changes = results.fixes;

expect(changes, isNotEmpty);
Expand Down Expand Up @@ -149,7 +147,7 @@ void defineTests() {
final idx = 61;
expect(completionLargeNamespaces.substring(idx - 1, idx), 'A');
final results =
await analysisServer.completeV3(completionLargeNamespaces, 61);
await analysisServer.complete(completionLargeNamespaces, 61);
expect(completionsContains(results, 'A'), true);
expect(completionsContains(results, 'AB'), true);
expect(completionsContains(results, 'ABC'), true);
Expand Down
Loading

0 comments on commit 7d62c04

Please sign in to comment.