Skip to content

Commit

Permalink
Defer projection failure until after plugins run (smithy-lang#1762)
Browse files Browse the repository at this point in the history
This changes `SmithyBuildImpl` to continue applying plugins after one fails,
throwing the error (if present) after they have all completed. This is
useful for example when you want to see the serialized output of a model
you know is valid but a plugin is causing the whole build to fail. A new
test case was added to ensure that artifacts produced by valid plugins
are still created despite the build failing.

This was originally implemented in smithy-lang#1416, but was rolled back in smithy-lang#1429
as a precaution since we had an unrelated issue ocurring at the time.
  • Loading branch information
milesziemer authored and Steven Yuan committed Aug 11, 2023
1 parent fa92622 commit 8032616
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ private ProjectionResult applyProjection(
ProjectionConfig projection,
ValidatedResult<Model> baseModel,
List<ResolvedPlugin> resolvedPlugins
) {
) throws Throwable {
Model resolvedModel = baseModel.unwrap();
LOGGER.fine(() -> String.format("Creating the `%s` projection", projectionName));

Expand Down Expand Up @@ -351,18 +351,33 @@ private ProjectionResult applyProjection(
LOGGER.fine(() -> String.format("No transforms to apply for projection %s", projectionName));
}

// Keep track of the first error created by plugins to fail the build after all plugins have run.
Throwable firstPluginError = null;
ProjectionResult.Builder resultBuilder = ProjectionResult.builder()
.projectionName(projectionName)
.model(projectedModel)
.events(modelResult.getValidationEvents());

for (ResolvedPlugin resolvedPlugin : resolvedPlugins) {
if (pluginFilter.test(resolvedPlugin.id.getArtifactName())) {
applyPlugin(projectionName, projection, baseProjectionDir, resolvedPlugin,
try {
applyPlugin(projectionName, projection, baseProjectionDir, resolvedPlugin,
projectedModel, resolvedModel, modelResult, resultBuilder);
} catch (Throwable e) {
if (firstPluginError == null) {
firstPluginError = e;
} else {
// Only log subsequent errors, since the first one is thrown.
LOGGER.severe(String.format("Plugin `%s` failed: %s", resolvedPlugin.id, e));
}
}
}
}

if (firstPluginError != null) {
throw firstPluginError;
}

return resultBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -268,6 +269,62 @@ public void failsByDefaultForUnknownPlugins() throws Exception {
assertThat(e.getMessage(), containsString("Unable to find a plugin for `unknown1`"));
}

@Test
public void defersFailureUntilAfterAllPluginsApplied() throws Exception {
SmithyBuildConfig config = SmithyBuildConfig.builder()
.load(Paths.get(getClass().getResource("defers-failure.json").toURI()))
.outputDirectory(outputDirectory.toString())
.build();

RuntimeException canned = new RuntimeException("broken");

// "broken" will run before "test1Serial" because of natural ordering
Map<String, SmithyBuildPlugin> plugins = new HashMap<>();
plugins.put("broken", new SmithyBuildPlugin() {
@Override
public String getName() {
return "broken";
}
@Override
public void execute(PluginContext context) {
throw canned;
}
});
plugins.put("test1Serial", new Test1SerialPlugin());

Function<String, Optional<SmithyBuildPlugin>> factory = SmithyBuildPlugin.createServiceFactory();
Function<String, Optional<SmithyBuildPlugin>> composed = name -> OptionalUtils.or(
Optional.ofNullable(plugins.get(name)), () -> factory.apply(name));

// Because the build will fail, we need a way to access the file manifests
List<FileManifest> manifests = new ArrayList<>();
Function<Path, FileManifest> fileManifestFactory = pluginBaseDir -> {
FileManifest fileManifest = new MockManifest(pluginBaseDir);
manifests.add(fileManifest);
return fileManifest;
};

SmithyBuild builder = new SmithyBuild()
.pluginFactory(composed)
.fileManifestFactory(fileManifestFactory)
.config(config);

SmithyBuildException e = Assertions.assertThrows(SmithyBuildException.class, builder::build);

// "broken" plugin produces the error that causes the build to fail
assertThat(e.getMessage(), containsString("java.lang.RuntimeException: broken"));
assertThat(e.getSuppressed(), equalTo(new Throwable[]{canned}));

List<Path> files = manifests.stream()
.flatMap(fm -> fm.getFiles().stream())
.collect(Collectors.toList());
assertThat(files, containsInAnyOrder(
outputDirectory.resolve("source/sources/manifest"),
outputDirectory.resolve("source/model/model.json"),
outputDirectory.resolve("source/build-info/smithy-build-info.json"),
outputDirectory.resolve("source/test1Serial/hello1Serial")));
}

@Test
public void cannotSetFiltersOrMappersOnSourceProjection() {
Throwable thrown = Assertions.assertThrows(SmithyBuildException.class, () -> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"version": "2.0",
"plugins": {
"broken": {},
"test1Serial": {}
}
}

0 comments on commit 8032616

Please sign in to comment.