Skip to content
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

Better error handling for LoadStrategy #1744

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 8 additions & 0 deletions dwds/lib/src/debugging/location.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
224 changes: 118 additions & 106 deletions dwds/lib/src/injected/client.js

Large diffs are not rendered by default.

31 changes: 17 additions & 14 deletions dwds/lib/src/loaders/build_runner_require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
12 changes: 6 additions & 6 deletions dwds/lib/src/loaders/legacy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand Down
8 changes: 4 additions & 4 deletions dwds/lib/src/loaders/require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions dwds/lib/src/loaders/strategy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
6 changes: 3 additions & 3 deletions webdev/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: