From 5e7d4d8762427a152fc26861db753080ce328577 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Thu, 4 May 2023 10:15:54 -0400 Subject: [PATCH] Defer projection failure until after plugins run 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 #1416, but was rolled back in #1429 as a precaution since we had an unrelated issue ocurring at the time. --- .../amazon/smithy/build/SmithyBuildImpl.java | 19 ++++++- .../amazon/smithy/build/SmithyBuildTest.java | 57 +++++++++++++++++++ .../amazon/smithy/build/defers-failure.json | 7 +++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/defers-failure.json diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java b/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java index 6efd2da3c72..6488aafd448 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java @@ -303,7 +303,7 @@ private ProjectionResult applyProjection( ProjectionConfig projection, ValidatedResult baseModel, List resolvedPlugins - ) { + ) throws Throwable { Model resolvedModel = baseModel.unwrap(); LOGGER.fine(() -> String.format("Creating the `%s` projection", projectionName)); @@ -351,6 +351,8 @@ 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) @@ -358,11 +360,24 @@ private ProjectionResult applyProjection( 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(); } diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java index 47398bdfcb7..f140b2d7005 100644 --- a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java +++ b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java @@ -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; @@ -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 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> factory = SmithyBuildPlugin.createServiceFactory(); + Function> 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 manifests = new ArrayList<>(); + Function 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 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, () -> { diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/defers-failure.json b/smithy-build/src/test/resources/software/amazon/smithy/build/defers-failure.json new file mode 100644 index 00000000000..de0651acf12 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/defers-failure.json @@ -0,0 +1,7 @@ +{ + "version": "2.0", + "plugins": { + "broken": {}, + "test1Serial": {} + } +}