Better error handling for LoadStrategy (#1744)

* Better error handling for LoadStrategy * update version, changelog, and build
dart-lang · Sep 9, 2022 · 9b77ab3 · 9b77ab3
1 parent f480bfc
commit 9b77ab3
Show file tree

Hide file tree

Showing 8 changed files with 163 additions and 135 deletions.
diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 16.0.1-dev
+
+- Allow `LoadStrategy.serverPathForModule` and `LoadStrategy.sourceMapPathForModule`
+  to return `null` and add error handling.
+
 ## 16.0.0
 
 - Fix a hang and report errors on hot reload exceptions from the injected

diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart
@@ -286,8 +286,16 @@ class Locations {
       }
       final modulePath =
           await globalLoadStrategy.serverPathForModule(_entrypoint, module);
+      if (modulePath == null) {
+        _logger.warning('No module path for module: $module');
+        return result;
+      }
       final sourceMapPath =
           await globalLoadStrategy.sourceMapPathForModule(_entrypoint, module);
+      if (sourceMapPath == null) {
+        _logger.warning('No sourceMap path for module: $module');
+        return result;
+      }
       final sourceMapContents =
           await _assetReader.sourceMapContents(sourceMapPath);
       final scriptLocation =

diff --git a/dwds/lib/src/injected/client.js b/dwds/lib/src/injected/client.js
diff --git a/dwds/lib/src/loaders/build_runner_require.dart b/dwds/lib/src/loaders/build_runner_require.dart
@@ -84,17 +84,16 @@ class BuildRunnerRequireStrategyProvider {
     return null;
   }
 
-  Future<String> _serverPathForModule(
+  Future<String?> _serverPathForModule(
       MetadataProvider metadataProvider, String module) async {
-    final result = stripTopLevelDirectory(
-        (await metadataProvider.moduleToModulePath)[module] ?? '');
-    return result;
+    final modulePath = (await metadataProvider.moduleToModulePath)[module];
+    return modulePath == null ? null : stripTopLevelDirectory(modulePath);
   }
 
-  Future<String> _sourceMapPathForModule(
+  Future<String?> _sourceMapPathForModule(
       MetadataProvider metadataProvider, String module) async {
-    final path = (await metadataProvider.moduleToSourceMap)[module] ?? '';
-    return stripTopLevelDirectory(path);
+    final sourceMapPath = (await metadataProvider.moduleToSourceMap)[module];
+    return sourceMapPath == null ? null : stripTopLevelDirectory(sourceMapPath);
   }
 
   String? _serverPathForAppUri(String appUrl) {
@@ -115,13 +114,17 @@ class BuildRunnerRequireStrategyProvider {
     final result = <String, ModuleInfo>{};
     for (var module in modules) {
       final serverPath = await _serverPathForModule(metadataProvider, module);
-      result[module] = ModuleInfo(
-          // TODO: Save locations of full kernel files in ddc metadata.
-          // Issue: https://github.com/dart-lang/sdk/issues/43684
-          // TODO: Change these to URIs instead of paths when the SDK supports
-          // it.
-          p.setExtension(serverPath, '.full.dill'),
-          p.setExtension(serverPath, '.dill'));
+      if (serverPath == null) {
+        _logger.warning('No module info found for module $module');
+      } else {
+        result[module] = ModuleInfo(
+            // TODO: Save locations of full kernel files in ddc metadata.
+            // Issue: https://github.com/dart-lang/sdk/issues/43684
+            // TODO: Change these to URIs instead of paths when the SDK supports
+            // it.
+            p.setExtension(serverPath, '.full.dill'),
+            p.setExtension(serverPath, '.dill'));
+      }
     }
     return result;
   }

diff --git a/dwds/lib/src/loaders/legacy.dart b/dwds/lib/src/loaders/legacy.dart
@@ -20,7 +20,7 @@ class LegacyStrategy extends LoadStrategy {
   ///
   /// /packages/path/path.ddc.js -> packages/path/path
   ///
-  final Future<String> Function(
+  final Future<String?> Function(
           MetadataProvider metadataProvider, String sourcePath)
       _moduleForServerPath;
 
@@ -39,7 +39,7 @@ class LegacyStrategy extends LoadStrategy {
   ///
   ///   web/main -> main.ddc.js
   ///
-  final Future<String> Function(
+  final Future<String?> Function(
       MetadataProvider metadataProvider, String module) _serverPathForModule;
 
   /// Returns the source map path for the provided module.
@@ -48,7 +48,7 @@ class LegacyStrategy extends LoadStrategy {
   ///
   ///   web/main -> main.ddc.js.map
   ///
-  final Future<String> Function(
+  final Future<String?> Function(
       MetadataProvider metadataProvider, String module) _sourceMapPathForModule;
 
   /// Returns the server path for the app uri.
@@ -101,7 +101,7 @@ class LegacyStrategy extends LoadStrategy {
       'window.\$dartLoader.forceLoadModule("$clientScript");\n';
 
   @override
-  Future<String> moduleForServerPath(
+  Future<String?> moduleForServerPath(
           String entrypoint, String serverPath) async =>
       _moduleForServerPath(metadataProviderFor(entrypoint), serverPath);
 
@@ -110,11 +110,11 @@ class LegacyStrategy extends LoadStrategy {
       _moduleInfoForProvider(metadataProviderFor(entrypoint));
 
   @override
-  Future<String> serverPathForModule(String entrypoint, String module) async =>
+  Future<String?> serverPathForModule(String entrypoint, String module) async =>
       _serverPathForModule(metadataProviderFor(entrypoint), module);
 
   @override
-  Future<String> sourceMapPathForModule(
+  Future<String?> sourceMapPathForModule(
           String entrypoint, String module) async =>
       _sourceMapPathForModule(metadataProviderFor(entrypoint), module);
 

diff --git a/dwds/lib/src/loaders/require.dart b/dwds/lib/src/loaders/require.dart
@@ -102,7 +102,7 @@ class RequireStrategy extends LoadStrategy {
   ///
   ///   web/main -> main.ddc.js
   ///
-  final Future<String> Function(MetadataProvider provider, String module)
+  final Future<String?> Function(MetadataProvider provider, String module)
       _serverPathForModule;
 
   /// Returns the source map path for the provided module.
@@ -111,7 +111,7 @@ class RequireStrategy extends LoadStrategy {
   ///
   ///   web/main -> main.ddc.js.map
   ///
-  final Future<String> Function(MetadataProvider provider, String module)
+  final Future<String?> Function(MetadataProvider provider, String module)
       _sourceMapPathForModule;
 
   /// Returns the server path for the app uri.
@@ -276,13 +276,13 @@ if(!window.\$requireLoader) {
   }
 
   @override
-  Future<String> serverPathForModule(String entrypoint, String module) {
+  Future<String?> serverPathForModule(String entrypoint, String module) {
     final metadataProvider = metadataProviderFor(entrypoint);
     return _serverPathForModule(metadataProvider, module);
   }
 
   @override
-  Future<String> sourceMapPathForModule(String entrypoint, String module) {
+  Future<String?> sourceMapPathForModule(String entrypoint, String module) {
     final metadataProvider = metadataProviderFor(entrypoint);
     return _sourceMapPathForModule(metadataProvider, module);
   }

diff --git a/dwds/lib/src/loaders/strategy.dart b/dwds/lib/src/loaders/strategy.dart
@@ -93,15 +93,15 @@ abstract class LoadStrategy {
   ///
   ///   web/main -> main.ddc.js
   ///
-  Future<String> serverPathForModule(String entrypoint, String module);
+  Future<String?> serverPathForModule(String entrypoint, String module);
 
   /// Returns the source map path for the provided module.
   ///
   /// For example:
   ///
   ///   web/main -> main.ddc.js.map
   ///
-  Future<String> sourceMapPathForModule(String entrypoint, String module);
+  Future<String?> sourceMapPathForModule(String entrypoint, String module);
 
   /// Returns a map from module id to module info for the provided entrypoint.
   ///

diff --git a/webdev/pubspec.yaml b/webdev/pubspec.yaml
@@ -49,9 +49,9 @@ dev_dependencies:
   webdriver: ^3.0.0
 
 # Comment out before releasing webdev.
-# dependency_overrides:
-#   dwds:
-#     path: ../dwds
+dependency_overrides:
+  dwds:
+    path: ../dwds
 
 executables:
   webdev: