Propagate more errors to the client side

This commit improves error reporting on the client side. It propagates
some errors which were previously only captured on the server side.
In particular, it will now:

- report which property was being resolved, instead of showing
internal `auto.test.resources` properties
- capture errors which are sent by the server but were dropped
because of lack of JSON error handling
- report when a container cannot be started or failed during
startup
- provide clearer message when the test resources service is
down

Fixes #444

gradle/libs.versions.toml

@@ -37,7 +37,7 @@ micronaut-reactor = "3.1.0"
 micronaut-gradle-plugin = "4.1.2"
 micronaut-redis = "6.0.2"
 micronaut-security = "4.1.0"
-micronaut-serde = "2.2.6"
+micronaut-serde = "2.4.0"
 micronaut-sql = "5.2.0"
 micronaut-test = "4.0.0"
 groovy = "4.0.13"

test-resources-client/build.gradle

@@ -9,8 +9,10 @@ and provide their value on demand.
 """
 
 dependencies {
+    annotationProcessor(mnSerde.micronaut.serde.processor)
     api(mn.micronaut.json.core)
-    api(project(':micronaut-test-resources-core'))
-
+    api(projects.micronautTestResourcesCore)
+    compileOnly(mnSerde.micronaut.serde.api)
+    compileOnly(mnSerde.micronaut.serde.jackson)
     testRuntimeOnly(mn.micronaut.http.server.netty)
 }

test-resources-client/src/main/java/io/micronaut/testresources/client/DefaultTestResourcesClient.java

@@ -21,6 +21,7 @@
 import io.micronaut.json.JsonMapper;
 
 import java.io.IOException;
+import java.net.ConnectException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.http.HttpClient;
@@ -29,6 +30,7 @@
 import java.time.Duration;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -51,6 +53,8 @@ public class DefaultTestResourcesClient implements TestResourcesClient {
     private static final Argument<List<String>> LIST_OF_STRING = Argument.LIST_OF_STRING;
     private static final Argument<String> STRING = Argument.STRING;
     private static final Argument<Boolean> BOOLEAN = Argument.BOOLEAN;
+    private static final String INTERNAL_SERVER_ERROR_PREFIX = "Internal Server Error: ";
+    private static final String INTERNAL_SERVER_ERROR = "Internal Server Error";
 
     private final JsonMapper jsonMapper;
     private final String baseUri;
@@ -81,7 +85,8 @@ public List<String> getResolvableProperties(Map<String, Collection<String>> prop
     }
 
     @Override
-    public Optional<String> resolve(String name, Map<String, Object> properties, Map<String, Object> testResourcesConfig) {
+    public Optional<String> resolve(String name, Map<String, Object> properties,
+                                    Map<String, Object> testResourcesConfig) {
         Map<String, Object> params = new HashMap<>();
         params.put("name", name);
         params.put("properties", properties);
@@ -119,7 +124,8 @@ private void GET(HttpRequest.Builder request) {
         request.GET();
     }
 
-    private <T> T request(String path, Argument<T> type, Consumer<? super HttpRequest.Builder> config) {
+    private <T> T request(String path, Argument<T> type,
+                          Consumer<? super HttpRequest.Builder> config) {
         var request = HttpRequest.newBuilder()
             .uri(uri(path))
             .timeout(clientTimeout);
@@ -138,8 +144,15 @@ private <T> T request(String path, Argument<T> type, Consumer<? super HttpReques
                     return (T) body;
                 }
                 return jsonMapper.readValue(body, type);
+            } else if (response.statusCode() == 500) {
+                return handleError(jsonMapper.readValue(body, SimpleJsonErrorModel.class));
+            } else if (response.statusCode() == 404) {
+                return null;
             }
-            return null;
+            throw new TestResourcesException(
+                "Unexpected response code: " + response.statusCode() + " " + body);
+        } catch (ConnectException e) {
+            throw new TestResourcesException("Test resource service is not available at " + baseUri, e);
         } catch (IOException e) {
             throw new TestResourcesException(e);
         } catch (InterruptedException e) {
@@ -148,6 +161,41 @@ private <T> T request(String path, Argument<T> type, Consumer<? super HttpReques
         }
     }
 
+    private <T> T handleError(SimpleJsonErrorModel model) {
+        var allErrors = new LinkedHashSet<String>();
+        collectErrors(model, allErrors);
+        var errorList = allErrors.stream().toList();
+        if (errorList.size() == 1) {
+            throw new TestResourcesException(errorList.get(0));
+        } else {
+            var sb = new StringBuilder();
+            sb.append("Server failed with the following errors:\n");
+            for (String error : errorList) {
+                sb.append(" - ").append(error).append("\n");
+            }
+            throw new TestResourcesException(sb.toString());
+        }
+    }
+
+    private void collectErrors(SimpleJsonErrorModel model, LinkedHashSet<String> allErrors) {
+        sanitizeError(model.message()).ifPresent(allErrors::add);
+        if (model.embedded() != null && model.embedded().errors() != null) {
+            for (SimpleJsonErrorModel error : model.embedded().errors()) {
+                collectErrors(error, allErrors);
+            }
+        }
+    }
+
+    private static Optional<String> sanitizeError(String message) {
+        if (message.equals(INTERNAL_SERVER_ERROR)) {
+            return Optional.empty();
+        }
+        if (message.startsWith(INTERNAL_SERVER_ERROR_PREFIX)) {
+            return Optional.of(message.substring(INTERNAL_SERVER_ERROR_PREFIX.length()));
+        }
+        return Optional.of(message);
+    }
+
     private URI uri(String path) {
         try {
             return new URI(baseUri + path);
@@ -163,4 +211,5 @@ private byte[] writeValueAsBytes(Object o) {
             throw new TestResourcesException(e);
         }
     }
+
 }

test-resources-client/src/main/java/io/micronaut/testresources/client/SimpleJsonErrorModel.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2017-2023 original authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.micronaut.testresources.client;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.micronaut.serde.annotation.Serdeable;
+
+import java.util.List;
+
+@Serdeable
+public record SimpleJsonErrorModel(
+    @JsonProperty("message") String message,
+    @JsonProperty("_embedded") Embedded embedded
+) {
+    @Serdeable
+    public record Embedded(
+        @JsonProperty("errors") List<SimpleJsonErrorModel> errors
+    ) {
+    }
+}

test-resources-client/src/main/java/io/micronaut/testresources/client/TestResourcesClientPropertyExpressionResolver.java

@@ -31,6 +31,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
 
 import static io.micronaut.testresources.core.PropertyResolverSupport.resolveRequiredProperties;
 
@@ -108,7 +109,26 @@ public <T> Optional<T> resolve(PropertyResolver propertyResolver,
         }
 
         private static Optional<String> callClient(String expression, TestResourcesClient client, Map<String, Object> props, Map<String, Object> properties) {
-            return client.resolve(expression, props, properties);
+            return withErrorHandling(
+                () -> client.resolve(expression, props, properties),
+                () -> "Test resources service wasn't able to revolve expression '" + expression + "'"
+            );
+        }
+
+        private static <T> T withErrorHandling(Supplier<T> callable, Supplier<String> errorMessage) {
+            try {
+                return callable.get();
+            } catch (TestResourcesException ex) {
+                var sb = new StringBuilder();
+                sb.append(errorMessage.get()).append(":");
+                var message = ex.getMessage();
+                if (message.contains("\n")) {
+                    sb.append("  ").append(message);
+                } else {
+                    sb.append(" ").append(message);
+                }
+                throw new TestResourcesException(sb.toString());
+            }
         }
 
         @Override

test-resources-client/src/main/java/io/micronaut/testresources/client/TestResourcesException.java

@@ -23,6 +23,10 @@ public TestResourcesException(Throwable cause) {
         super(cause);
     }
 
+    public TestResourcesException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
     public TestResourcesException(String message) {
         super(message);
     }

test-resources-client/src/test/groovy/io/micronaut/testresources/client/NoServerTestResourcesClientTest.groovy

@@ -0,0 +1,35 @@
+package io.micronaut.testresources.client
+
+import io.micronaut.context.ApplicationContext
+import io.micronaut.test.extensions.spock.annotation.MicronautTest
+import spock.lang.Specification
+import spock.util.environment.RestoreSystemProperties
+
+import static io.micronaut.testresources.client.ConfigFinder.systemPropertyNameOf
+
+@MicronautTest
+class NoServerTestResourcesClientTest extends Specification implements ClientCleanup {
+
+    private static final String DUMMY_URL = "https://localhost:666/nope"
+
+    @RestoreSystemProperties
+    def "reasonable error message if the server isn't running"() {
+
+        when:
+        createApplication()
+
+        then:
+        TestResourcesException e = thrown()
+        e.message == "Test resource service is not available at $DUMMY_URL"
+    }
+
+    private ApplicationContext createApplication() {
+        System.setProperty(systemPropertyNameOf(TestResourcesClient.SERVER_URI), DUMMY_URL)
+        def app = ApplicationContext.builder()
+                .properties(['server': 'false'])
+                .start()
+        assert !app.findBean(TestServer).present
+        return app
+    }
+
+}

test-resources-client/src/test/groovy/io/micronaut/testresources/client/TestResourcesClientPropertiesTest.groovy

@@ -1,9 +1,9 @@
 package io.micronaut.testresources.client
 
 import io.micronaut.context.ApplicationContext
-import io.micronaut.context.exceptions.ConfigurationException
 import io.micronaut.runtime.server.EmbeddedServer
 import io.micronaut.test.extensions.spock.annotation.MicronautTest
+import io.micronaut.testresources.core.TestResourcesResolutionException
 import jakarta.inject.Inject
 import spock.lang.Specification
 import spock.util.environment.RestoreSystemProperties
@@ -29,8 +29,8 @@ class TestResourcesClientPropertiesTest extends Specification implements ClientC
         app.getProperty("missing", String).empty
 
         then:
-        ConfigurationException e = thrown()
-        e.message == 'Could not resolve placeholder ${auto.test.resources.missing}'
+        TestResourcesResolutionException e = thrown()
+        e.message == "Test resources doesn't support resolving expression 'missing'"
 
         cleanup:
         System.clearProperty("micronaut.test.resources.server.uri")

test-resources-client/src/test/groovy/io/micronaut/testresources/client/TestResourcesClientTest.groovy

@@ -1,9 +1,9 @@
 package io.micronaut.testresources.client
 
 import io.micronaut.context.ApplicationContext
-import io.micronaut.context.exceptions.ConfigurationException
 import io.micronaut.runtime.server.EmbeddedServer
 import io.micronaut.test.extensions.spock.annotation.MicronautTest
+import io.micronaut.testresources.core.TestResourcesResolutionException
 import jakarta.inject.Inject
 import spock.lang.Specification
 import spock.lang.TempDir
@@ -34,8 +34,20 @@ class TestResourcesClientTest extends Specification implements ClientCleanup {
         app.getProperty("missing", String).empty
 
         then:
-        ConfigurationException e = thrown()
-        e.message == 'Could not resolve placeholder ${auto.test.resources.missing}'
+        TestResourcesResolutionException e = thrown()
+        e.message == "Test resources doesn't support resolving expression 'missing'"
+    }
+
+    @RestoreSystemProperties
+    def "reasonable error message when the server throws an error"() {
+        def app = createApplication()
+
+        when:
+        app.getProperty("throws", String)
+
+        then:
+        TestResourcesException e = thrown()
+        e.message == "Test resources service wasn't able to revolve expression 'throws': Something bad happened"
     }
 
     private ApplicationContext createApplication() {

test-resources-client/src/test/groovy/io/micronaut/testresources/client/TestServer.groovy

@@ -13,7 +13,7 @@ class TestServer implements TestResourcesResolver {
     @Override
     @Post("/list")
     List<String> getResolvableProperties(Map<String, Collection<String>> propertyEntries, Map<String, Object> testResourcesConfig) {
-        ["dummy1", "dummy2", "missing"]
+        ["dummy1", "dummy2", "missing", "throws"]
     }
 
     @Override
@@ -34,6 +34,9 @@ class TestServer implements TestResourcesResolver {
         if ("missing" == name) {
             return Optional.empty()
         }
+        if ("throws" == name) {
+            throw new RuntimeException("Something bad happened")
+        }
         Optional.of("value for $name".toString())
     }
 

test-resources-core/src/main/java/io/micronaut/testresources/core/LazyTestResourcesExpressionResolver.java

@@ -42,7 +42,15 @@ public <T> Optional<T> resolve(PropertyResolver propertyResolver,
                                    Class<T> requiredType) {
         if (expression.startsWith(PLACEHOLDER_PREFIX)) {
             String eagerExpression = expression.substring(PLACEHOLDER_PREFIX.length());
-            return delegate.resolve(propertyResolver, conversionService, eagerExpression, requiredType);
+            var resolved = delegate.resolve(propertyResolver, conversionService, eagerExpression,
+                requiredType);
+            if (resolved.isEmpty()) {
+                // This is the only case where we should throw an exception instead of returning
+                // an empty optional: we assume that if the expression starts with the prefix
+                // then ONLY test resources should resolve it
+                throw new TestResourcesResolutionException("Test resources doesn't support resolving expression '" + expression.substring(PLACEHOLDER_PREFIX.length()) + "'");
+            }
+            return resolved;
         }
         return Optional.empty();
     }

test-resources-core/src/main/java/io/micronaut/testresources/core/TestResourcesResolutionException.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2017-2021 original authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.micronaut.testresources.core;
+
+/**
+ * An exception thrown whenever test resources was expected to resolve
+ * a property but couldn't.
+ */
+public class TestResourcesResolutionException extends RuntimeException {
+    public TestResourcesResolutionException(Throwable cause) {
+        super(cause);
+    }
+
+    public TestResourcesResolutionException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public TestResourcesResolutionException(String message) {
+        super(message);
+    }
+}

test-resources-jdbc/test-resources-jdbc-oracle-xe/src/test/groovy/io/micronaut/testresources/jdbc/xe/OracleATPTest.groovy

@@ -19,7 +19,7 @@ class OracleATPTest extends AbstractJDBCSpec {
 
         then:
         BeanInstantiationException ex = thrown()
-        ex.message.contains("Could not resolve placeholder \${auto.test.resources.datasources.default")
+        ex.message.contains("Test resources doesn't support resolving expression 'datasources.default.password'")
     }
 
     @Override