Match server paths to source maps paths (#1730)

* Try repro mixed mode ddc bug

* Added logging

* format

* Make module uris match directory structure

* Use debugger module names, add tests

* Cleanup

* Added package_uri_mapper_tests, fixed failures in dart_uri tests

* Fix location test failures

* Addressed CR comments, run dart uri tests with useDebuggerModuleNames flag

* Run dart_uri_file_uri test with frontend server as well

Author: Anna Gringauze

dwds/CHANGELOG.md

@@ -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
 
@@ -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
 
@@ -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.
@@ -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.
 
@@ -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`.
 
@@ -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
 
@@ -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

dwds/lib/asset_reader.dart

@@ -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;

dwds/lib/dwds.dart

@@ -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;

dwds/lib/src/debugging/debugger.dart

@@ -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 ?? [];
@@ -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 {
@@ -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;

dwds/lib/src/debugging/location.dart

@@ -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';
@@ -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) {
@@ -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);
@@ -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,

dwds/lib/src/debugging/metadata/provider.dart

@@ -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:/')) {

dwds/lib/src/debugging/modules.dart

@@ -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;

dwds/lib/src/loaders/build_runner_require.dart

@@ -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;
@@ -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;
   }

dwds/lib/src/loaders/frontend_server_require.dart

@@ -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;
 
@@ -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 {
@@ -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;
   }

dwds/lib/src/loaders/require.dart

@@ -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;