Skip to content

Match server paths to source maps paths #1730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
17 changes: 11 additions & 6 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@
- Migrate `package:dwds` to null safety.
- Make `ChromeProxyService.getStack` wait for the debugger to perform initial
resume operation. This avoids race conditions on isolate start.
- Make server paths match directory structure
- Allows correct relative source map paths resolution.
- Add `PackageUriMapper` class to allow mapping uris to server paths.
- Update the min SDK constraint to 2.18.0.

**Breaking changes**

- Remove no longer used `ExpressionCompilerService.handler`.
- Remove `assetHandler` parameter from `ExpressionCompilerService` constructor.
- Add `packageUriMapper` parameter to the constructor of
`FrontendServerRequireStrategyProvider`.

## 15.0.0

Expand Down Expand Up @@ -141,7 +146,7 @@

## 11.5.1

- Update SDK contraint to `>=2.15.0 <3.0.0`.
- Update SDK constraint to `>=2.15.0 <3.0.0`.

## 11.5.0

Expand Down Expand Up @@ -235,7 +240,7 @@
## 11.1.2

- Return empty library from `ChromeProxyService.getObject` for libraries present
in medatata but not loaded at runtime.
in metadata but not loaded at runtime.
- Log failures to load kernel during expression evaluation.
- Show lowered final fields using their original dart names.
- Limit simultaneous connections to asset server to prevent broken sockets.
Expand Down Expand Up @@ -395,7 +400,7 @@

## 7.0.2

- Depend on the latest `pacakge:sse`.
- Depend on the latest `package:sse`.
- Add more verbose logging around `hotRestart`, `fullReload` and entrypoint
injection.

Expand Down Expand Up @@ -432,7 +437,7 @@
- Change `ExpressionCompiler` to require a new `updateDependencies` method.
- Update a number of `LoadStrategy` APIs to remove heuristics and rely on the
`MetadataProvider`.
- No longer require a `LogWriter` and corresponding `verbose` arguement but
- No longer require a `LogWriter` and corresponding `verbose` argument but
instead properly use `package:logger`.
- `FrontendServerRequireStrategyProvider` now requires a `digestProvider`.

Expand Down Expand Up @@ -503,7 +508,7 @@
- Change the returned errors for the unimplemented `getClassList` and
`reloadSources` methods to -32601 ('method does not exist / is not
available').
- Do not include native JavaScipt objects on stack returned from the debugger.
- Do not include native JavaScript objects on stack returned from the debugger.

## 3.1.0

Expand Down Expand Up @@ -596,7 +601,7 @@
- Expose `middleware` and `handler`.

**Breaking Change:** The `AssetHandler` will not automatically be added the DWDS
handler cascade. You must now also add the `middelware` to your server's
handler cascade. You must now also add the `middleware` to your server's
pipeline.

## 0.8.5
Expand Down
3 changes: 2 additions & 1 deletion dwds/lib/asset_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

export 'src/readers/asset_reader.dart' show AssetReader, UrlEncoder;
export 'src/readers/asset_reader.dart'
show AssetReader, UrlEncoder, PackageUriMapper, stripLeadingSlashes;
3 changes: 2 additions & 1 deletion dwds/lib/dwds.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export 'src/loaders/frontend_server_require.dart'
export 'src/loaders/legacy.dart' show LegacyStrategy;
export 'src/loaders/require.dart' show RequireStrategy;
export 'src/loaders/strategy.dart' show LoadStrategy, ReloadConfiguration;
export 'src/readers/asset_reader.dart' show AssetReader, UrlEncoder;
export 'src/readers/asset_reader.dart'
show AssetReader, UrlEncoder, PackageUriMapper;
export 'src/readers/frontend_server_asset_reader.dart'
show FrontendServerAssetReader;
export 'src/readers/proxy_server_asset_reader.dart' show ProxyServerAssetReader;
Expand Down
11 changes: 6 additions & 5 deletions dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,6 @@ class Debugger extends Domain {
/// Handles pause events coming from the Chrome connection.
Future<void> _pauseHandler(DebuggerPausedEvent e) async {
final isolate = inspector.isolate;

Event event;
final timestamp = DateTime.now().millisecondsSinceEpoch;
final jsBreakpointIds = e.hitBreakpoints ?? [];
Expand Down Expand Up @@ -808,8 +807,9 @@ class _Breakpoints extends Domain {
throw RPCError(
'addBreakpoint',
102,
'The VM is unable to add a breakpoint '
'at the specified line or function');
'The VM is unable to add a breakpoint $id '
'at the specified line or function: ($scriptId:$line:$column): '
' cannot find Dart location.');
}

try {
Expand All @@ -822,8 +822,9 @@ class _Breakpoints extends Domain {
throw RPCError(
'addBreakpoint',
102,
'The VM is unable to add a breakpoint '
'at the specified line or function');
'The VM is unable to add a breakpoint $id '
'at the specified line or function: ($scriptId:$line:$column): '
'cannot set JS breakpoint at $location');
}
_note(jsId: jsBreakpointId, bp: dartBreakpoint);
return dartBreakpoint;
Expand Down
15 changes: 11 additions & 4 deletions dwds/lib/src/debugging/location.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:async/async.dart';
import 'package:dwds/src/loaders/require.dart';
import 'package:logging/logging.dart';
import 'package:path/path.dart' as p;
import 'package:source_maps/parser.dart';
Expand Down Expand Up @@ -162,8 +161,13 @@ class Locations {

/// Returns all [Location] data for a provided JS server path.
Future<Set<Location>> locationsForUrl(String url) async {
final module = await globalLoadStrategy.moduleForServerPath(
_entrypoint, Uri.parse(url).path);
if (url.isEmpty) return {};

final dartUri = DartUri(url, _root);
final serverPath = dartUri.serverPath;
final module =
await globalLoadStrategy.moduleForServerPath(_entrypoint, serverPath);

final cache = _moduleToLocations[module];
if (cache != null) return cache;
if (module != null) {
Expand Down Expand Up @@ -286,7 +290,9 @@ class Locations {
await globalLoadStrategy.sourceMapPathForModule(_entrypoint, module);
final sourceMapContents =
await _assetReader.sourceMapContents(sourceMapPath);
final scriptLocation = p.url.dirname('/${relativizePath(modulePath)}');
final scriptLocation =
p.url.dirname('/${stripLeadingSlashes(modulePath)}');

if (sourceMapContents == null) return result;
// This happens to be a [SingleMapping] today in DDC.
final mapping = parse(sourceMapContents);
Expand All @@ -303,6 +309,7 @@ class Locations {
final relativeSegments = p.split(mapping.urls[index]);
final path = p.url.normalize(
p.url.joinAll([scriptLocation, ...relativeSegments]));

final dartUri = DartUri(path, _root);
result.add(Location.from(
modulePath,
Expand Down
9 changes: 6 additions & 3 deletions dwds/lib/src/debugging/metadata/provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,12 @@ class MetadataProvider {
}

void _addMetadata(ModuleMetadata metadata) {
_moduleToSourceMap[metadata.name] = metadata.sourceMapUri;
_modulePathToModule[metadata.moduleUri] = metadata.name;
_moduleToModulePath[metadata.name] = metadata.moduleUri;
final modulePath = stripLeadingSlashes(metadata.moduleUri);
final sourceMapPath = stripLeadingSlashes(metadata.sourceMapUri);

_moduleToSourceMap[metadata.name] = sourceMapPath;
_modulePathToModule[modulePath] = metadata.name;
_moduleToModulePath[metadata.name] = modulePath;

for (var library in metadata.libraries.values) {
if (library.importUri.startsWith('file:/')) {
Expand Down
1 change: 1 addition & 0 deletions dwds/lib/src/debugging/modules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class Modules {

if (scriptToModule.containsKey(library)) {
final module = scriptToModule[library]!;

_sourceToModule[libraryServerPath] = module;
_sourceToLibrary[libraryServerPath] = Uri.parse(library);
_libraryToModule[library] = module;
Expand Down
12 changes: 8 additions & 4 deletions dwds/lib/src/loaders/build_runner_require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class BuildRunnerRequireStrategyProvider {
Future<String?> _moduleForServerPath(
MetadataProvider metadataProvider, String serverPath) async {
final modulePathToModule = await metadataProvider.modulePathToModule;
final relativePath = relativizePath(serverPath);
final relativePath = stripLeadingSlashes(serverPath);
for (var e in modulePathToModule.entries) {
if (stripTopLevelDirectory(e.key) == relativePath) {
return e.value;
Expand All @@ -97,10 +97,14 @@ class BuildRunnerRequireStrategyProvider {
return stripTopLevelDirectory(path);
}

String? _serverPathForAppUri(String appUri) {
if (appUri.startsWith('org-dartlang-app:')) {
String? _serverPathForAppUri(String appUrl) {
final appUri = Uri.parse(appUrl);
if (appUri.isScheme('org-dartlang-app')) {
// We skip the root from which we are serving.
return Uri.parse(appUri).pathSegments.skip(1).join('/');
return appUri.pathSegments.skip(1).join('/');
}
if (appUri.isScheme('package')) {
return '/packages/${appUri.path}';
}
return null;
}
Expand Down
35 changes: 24 additions & 11 deletions dwds/lib/src/loaders/frontend_server_require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import 'require.dart';
class FrontendServerRequireStrategyProvider {
final ReloadConfiguration _configuration;
final AssetReader _assetReader;
final PackageUriMapper _packageUriMapper;
final Future<Map<String, String>> Function() _digestsProvider;
final String _basePath;

Expand All @@ -29,26 +30,31 @@ class FrontendServerRequireStrategyProvider {
_assetReader,
);

FrontendServerRequireStrategyProvider(this._configuration, this._assetReader,
this._digestsProvider, this._basePath);
FrontendServerRequireStrategyProvider(
this._configuration,
this._assetReader,
this._packageUriMapper,
this._digestsProvider,
this._basePath,
);

RequireStrategy get strategy => _requireStrategy;

String _removeBasePath(String path) {
if (_basePath.isEmpty) return path;
// If path is a server path it might start with a '/'.
final base = path.startsWith('/') ? '/$_basePath' : _basePath;
return path.startsWith(base) ? path.substring(base.length) : path;

final stripped = stripLeadingSlashes(path);
return stripLeadingSlashes(stripped.substring(_basePath.length));
}

String _addBasePath(String serverPath) => _basePath.isEmpty
? relativizePath(serverPath)
: '$_basePath/${relativizePath(serverPath)}';
? stripLeadingSlashes(serverPath)
: '$_basePath/${stripLeadingSlashes(serverPath)}';

Future<Map<String, String>> _moduleProvider(
MetadataProvider metadataProvider) async =>
(await metadataProvider.moduleToModulePath).map((key, value) =>
MapEntry(key, relativizePath(removeJsExtension(value))));
MapEntry(key, stripLeadingSlashes(removeJsExtension(value))));

Future<String?> _moduleForServerPath(
MetadataProvider metadataProvider, String serverPath) async {
Expand All @@ -65,9 +71,16 @@ class FrontendServerRequireStrategyProvider {
MetadataProvider metadataProvider, String module) async =>
_addBasePath((await metadataProvider.moduleToSourceMap)[module] ?? '');

String? _serverPathForAppUri(String appUri) {
if (appUri.startsWith('org-dartlang-app:')) {
return _addBasePath(Uri.parse(appUri).path);
String? _serverPathForAppUri(String appUrl) {
final appUri = Uri.parse(appUrl);
if (appUri.isScheme('org-dartlang-app')) {
return _addBasePath(appUri.path);
}
if (appUri.isScheme('package')) {
final resolved = _packageUriMapper.packageUriToServerPath(appUri);
if (resolved != null) {
return resolved;
}
}
return null;
}
Expand Down
3 changes: 0 additions & 3 deletions dwds/lib/src/loaders/require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ String basePathForServerUri(String url) {
return base = base.startsWith('/') ? base.substring(1) : base;
}

String relativizePath(String path) =>
path.startsWith('/') ? path.substring(1) : path;

String removeJsExtension(String path) =>
path.endsWith('.js') ? p.withoutExtension(path) : path;

Expand Down
86 changes: 86 additions & 0 deletions dwds/lib/src/readers/asset_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:collection/collection.dart';
import 'package:file/file.dart';
import 'package:logging/logging.dart';
import 'package:package_config/package_config.dart';

typedef UrlEncoder = Future<String> Function(String url);

/// A reader for Dart sources and related source maps.
Expand All @@ -21,3 +26,84 @@ abstract class AssetReader {
/// Closes connections
Future<void> close();
}

class PackageUriMapper {
final _logger = Logger('PackageUriMapper');
final PackageConfig packageConfig;
final bool useDebuggerModuleNames;

static Future<PackageUriMapper> create(
FileSystem fileSystem, Uri packageConfigFile,
{bool useDebuggerModuleNames = false}) async {
final packageConfig =
await loadPackageConfig(fileSystem.file(packageConfigFile));
return PackageUriMapper(packageConfig,
useDebuggerModuleNames: useDebuggerModuleNames);
}

PackageUriMapper(this.packageConfig, {this.useDebuggerModuleNames = false});

/// Compute server path for package uri.
///
/// Note: needs to match `urlForComponentUri` in javascript_bundle.dart
/// in SDK code.
String? packageUriToServerPath(Uri packageUri) {
final defaultServerPath = '/packages/${packageUri.path}';
if (packageUri.isScheme('package')) {
if (!useDebuggerModuleNames) {
return defaultServerPath;
}
final Uri? resolvedUri = packageConfig.resolve(packageUri);
if (resolvedUri == null) {
_logger.severe('Cannot resolve package uri $packageUri');
return defaultServerPath;
}
final Package? package = packageConfig.packageOf(resolvedUri);
if (package == null) {
_logger.severe('Cannot find package for package uri $packageUri');
return defaultServerPath;
}
final Uri root = package.root;
final String relativeUrl =
resolvedUri.toString().replaceFirst('$root', '');
final String? relativeRoot = _getRelativeRoot(root);
final String ret = relativeRoot == null
? 'packages/$relativeUrl'
: 'packages/$relativeRoot/$relativeUrl';
return ret;
}
_logger.severe('Expected package uri, but found $packageUri');
return null;
}

/// Compute resolved file uri for a server path.
Uri? serverPathToResolvedUri(String serverPath) {
serverPath = stripLeadingSlashes(serverPath);
final segments = serverPath.split('/');
if (segments.first == 'packages') {
if (!useDebuggerModuleNames) {
return packageConfig
.resolve(Uri(scheme: 'package', pathSegments: segments.skip(1)));
}
final String relativeRoot = segments.skip(1).first;
final String relativeUrl = segments.skip(2).join('/');
final Package package = packageConfig.packages
.firstWhere((Package p) => _getRelativeRoot(p.root) == relativeRoot);
final Uri resolvedUri = package.root.resolve(relativeUrl);

return resolvedUri;
}
_logger.severe('Expected "packages/" path, but found $serverPath');
return null;
}
}

String stripLeadingSlashes(String path) {
while (path.startsWith('/')) {
path = path.substring(1);
}
return path;
}

String? _getRelativeRoot(Uri root) =>
root.pathSegments.lastWhereOrNull((segment) => segment.isNotEmpty);
Loading