Skip to content

Commit 131fe7b

Browse files
DanTupCommit Queue
authored andcommitted
[dds/dap] Extract URIs from stack traces with regex instead of package_stack_trace
package:stack_trace only parses stack traces in very specific formats and this has caused us to miss some in the past. Dart-Code/Dart-Code#5360 reports that this is also the case when there is any prefix (including `flutter:`) in the output. Rather than trying to clean up the input to pass to package:stack_trace, this just uses a simple regex to extract the URIs (and optional line/cols) from the line instead. Fixes Dart-Code/Dart-Code#5360 Change-Id: I13192fb201499f6cee6e28e7aa86d882557c0232 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400380 Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Derek Xu <[email protected]> Reviewed-by: Derek Xu <[email protected]>
1 parent 71f18e8 commit 131fe7b

File tree

5 files changed

+306
-40
lines changed

5 files changed

+306
-40
lines changed

pkg/dds/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- Updated `vm_service` constraint to ^14.3.0.
1111
- [DAP] Updated `dap` constraint to ^1.4.0.
1212
- [DAP] Set `supportsANSIStyling` to `true` in debug adapter capabilities to indicate that `Output` events might contain ansi color codes.
13+
- [DAP] Stack traces in more formats will be parsed and have locations attached to `OutputEvents`s.
1314
- Update to be forward compatible with `package:shelf_web_socket` version `3.x`.
1415

1516
# 4.2.7

pkg/dds/lib/src/dap/adapters/dart.dart

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2260,8 +2260,8 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
22602260

22612261
// Extract all the URIs so we can send a batch request for resolving them.
22622262
final lines = message.split('\n');
2263-
final frames = lines.map(parseDartStackFrame).toList();
2264-
final uris = frames.nonNulls.map((f) => f.uri).toList();
2263+
final frameLocations = lines.map(parseDartStackFrame).toList();
2264+
final uris = frameLocations.nonNulls.map((f) => f.uri).toList();
22652265

22662266
// We need an Isolate to resolve package URIs. Since we don't know what
22672267
// isolate printed an error to stderr, we just have to use the first one and
@@ -2294,7 +2294,7 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
22942294
// Convert any URIs to paths and if we successfully get a path, check
22952295
// whether it's inside the users workspace so we can fade out unrelated
22962296
// frames.
2297-
final paths = await Future.wait(frames.map((frame) async {
2297+
final framePaths = await Future.wait(frameLocations.map((frame) async {
22982298
final uri = frame?.uri;
22992299
if (uri == null) return null;
23002300
if (isSupportedFileScheme(uri)) {
@@ -2316,16 +2316,16 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
23162316
final supportsAnsiColors = args.allowAnsiColorOutput ?? false;
23172317
for (var i = 0; i < lines.length; i++) {
23182318
final line = lines[i];
2319-
final frame = frames[i];
2320-
final uri = frame?.uri;
2321-
final pathInfo = paths[i];
2319+
final frameLocation = frameLocations[i];
2320+
final uri = frameLocation?.uri;
2321+
final framePathInfo = framePaths[i];
23222322

23232323
// A file-like URI ('file://' or 'dart-macro+file://').
2324-
final fileLikeUri = pathInfo?.uri;
2324+
final fileLikeUri = framePathInfo?.uri;
23252325

23262326
// Default to true so that if we don't know whether this is user-project
23272327
// then we leave the formatting as-is and don't fade anything out.
2328-
final isUserProject = pathInfo?.isUserCode ?? true;
2328+
final isUserProject = framePathInfo?.isUserCode ?? true;
23292329

23302330
// For the name, we usually use the package URI, but if we only had a file
23312331
// URI to begin with, try to make it relative to cwd so it's not so long.
@@ -2362,8 +2362,8 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
23622362
output: output,
23632363
source:
23642364
clientPath != null ? Source(name: name, path: clientPath) : null,
2365-
line: frame?.line,
2366-
column: frame?.column,
2365+
line: frameLocation?.line,
2366+
column: frameLocation?.column,
23672367
),
23682368
);
23692369
}

pkg/dds/lib/src/dap/utils.dart

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:stack_trace/stack_trace.dart' as stack;
5+
import 'dart:io';
6+
7+
import 'package:path/path.dart' as path;
68

79
/// Returns whether this URI is something that can be resolved to a file-like
810
/// URI via the VM Service.
@@ -27,10 +29,9 @@ bool isResolvableUri(Uri uri) {
2729
///
2830
/// Frames that do not look like real Dart stack frames (such as including path
2931
/// or URIs that look like real Dart libraries) will be filtered out but it
30-
/// should not be assumed that if a [stack.Frame] is returned that the input
31-
/// was necessarily a stack frame or that calling `toString` will return the
32-
/// original input text.
33-
stack.Frame? parseDartStackFrame(String line) {
32+
/// should not be assumed that if a value is returned that the input
33+
/// was necessarily a stack frame.
34+
StackFrameLocation? parseDartStackFrame(String line) {
3435
final frame = _parseStackFrame(line);
3536
final uri = frame?.uri;
3637
return uri != null && _isDartUri(uri) ? frame : null;
@@ -68,35 +69,58 @@ bool _isDartUri(Uri uri) {
6869
return false;
6970
}
7071

72+
/// A [RegExp] for extracting URIs and optional line/columns out of a line from
73+
/// a stack trace.
74+
final _stackFrameLocationPattern =
75+
// Characters we consider part of a path:
76+
//
77+
// - `\w` word characters
78+
// - `-` dash (valid in paths and URI schemes)
79+
// - `:` colons (scheme or drive letters)
80+
// - `/` forward slashes (URIs)
81+
// - `\` black slashes (Windows paths)
82+
// - `%` percent (URL percent encoding)
83+
// - `+` plus (possible URL encoding of space)
84+
//
85+
// To avoid matching too much, we don't allow spaces even though they could
86+
// appear in relative paths. Most output should be URIs where they would be
87+
// encoded.
88+
//
89+
// The whole string must end with the line/col sequence, a non-word
90+
// character or be the end of the line. This avoids matching some strings
91+
// that contain ".dart" but probably aren't valid paths, like ".dart2".
92+
RegExp(r'([\w\-:\/\\%+]+\.dart)(?:(?:(?: +|:)(\d+):(\d+))|\W|$)');
93+
7194
/// Attempts to parse a line as a stack frame in order to read path/line/col
7295
/// information.
7396
///
74-
/// It should not be assumed that if a [stack.Frame] is returned that the input
75-
/// was necessarily a stack frame or that calling `toString` will return the
76-
/// original input text.
77-
stack.Frame? _parseStackFrame(String line) {
78-
// Because we split on \n, on Windows there may be trailing \r which prevents
79-
// package:stack_trace from parsing correctly.
80-
line = line.trim();
81-
82-
/// Helper to try parsing a frame with [parser], returning `null` if it
83-
/// fails to parse.
84-
stack.Frame? tryParseFrame(stack.Frame Function(String) parser) {
85-
final frame = parser(line);
86-
return frame is stack.UnparsedFrame ? null : frame;
97+
/// It should not be assumed that if a value is returned that the input
98+
/// was necessarily a stack frame.
99+
StackFrameLocation? _parseStackFrame(String input) {
100+
var match = _stackFrameLocationPattern.firstMatch(input);
101+
if (match == null) return null;
102+
103+
var uriMatch = match.group(1);
104+
var lineMatch = match.group(2);
105+
var colMatch = match.group(3);
106+
107+
var uri = uriMatch != null ? Uri.tryParse(uriMatch) : null;
108+
var line = lineMatch != null ? int.tryParse(lineMatch) : null;
109+
var col = colMatch != null ? int.tryParse(colMatch) : null;
110+
111+
if (uriMatch == null || uri == null) {
112+
return null;
113+
}
114+
115+
// If the URI has no scheme, assume a relative path from Directory.current.
116+
if (!uri.hasScheme && path.isRelative(uriMatch)) {
117+
var currentDirectoryPath = Directory.current.path;
118+
if (currentDirectoryPath.isNotEmpty) {
119+
uri = Uri.file(path.join(currentDirectoryPath, uriMatch));
120+
}
87121
}
88122

89-
// Try different formats of stack frames.
90-
// pkg:stack_trace does not have a generic Frame.parse() and Trace.parse()
91-
// doesn't work well when the content includes non-stack-frame lines
92-
// (https://github.com/dart-lang/stack_trace/issues/115).
93-
return tryParseFrame((line) => stack.Frame.parseVM(line)) ??
94-
// TODO(dantup): Tidy up when constructor tear-offs are available.
95-
tryParseFrame((line) => stack.Frame.parseV8(line)) ??
96-
tryParseFrame((line) => stack.Frame.parseSafari(line)) ??
97-
tryParseFrame((line) => stack.Frame.parseFirefox(line)) ??
98-
tryParseFrame((line) => stack.Frame.parseIE(line)) ??
99-
tryParseFrame((line) => stack.Frame.parseFriendly(line));
123+
return (uri: uri, line: line, column: col);
100124
}
101125

102126
/// Checks whether [flag] is in [args], allowing for both underscore and
@@ -106,3 +130,5 @@ bool containsVmFlag(List<String> args, String flag) {
106130
final flagDashes = flag.replaceAll('_', '-');
107131
return args.contains(flagUnderscores) || args.contains(flagDashes);
108132
}
133+
134+
typedef StackFrameLocation = ({Uri uri, int? line, int? column});

pkg/dds/pubspec.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ dependencies:
2727
shelf_web_socket: '>=1.0.0 <3.0.0'
2828
shelf: ^1.0.0
2929
sse: ^4.0.0
30-
stack_trace: ^1.10.0
3130
stream_channel: ^2.0.0
3231
vm_service: ^14.3.0
3332
web_socket_channel: '>=2.0.0 <4.0.0'

0 commit comments

Comments
 (0)