better logging and rendering of errors
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/01833f20 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/01833f20 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/01833f20 Branch: refs/heads/master Commit: 01833f20621a34bf5437d0a4dff8a775823fcbb8 Parents: 649a08d Author: Alex Heneveld <[email protected]> Authored: Thu Jan 14 11:45:20 2016 +0000 Committer: Alex Heneveld <[email protected]> Committed: Thu Jan 14 14:43:00 2016 +0000 ---------------------------------------------------------------------- .../camp/brooklyn/spi/creation/CampResolver.java | 2 +- .../catalog/internal/BasicBrooklynCatalog.java | 6 ++++-- .../brooklyn/rest/resources/CatalogResource.java | 7 ++++++- .../rest/util/DefaultExceptionMapper.java | 5 ++++- .../brooklyn/util/exceptions/Exceptions.java | 19 ++++++++++++++++++- 5 files changed, 33 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/01833f20/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java b/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java index 5615592..7523343 100644 --- a/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java +++ b/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampResolver.java @@ -139,7 +139,7 @@ class CampResolver { return appSpec; } else { - throw new IllegalStateException("Unable to instantiate YAML; incompatible instantiator "+instantiator+" for "+at); + throw new IllegalStateException("Unable to instantiate YAML; invalid type or parameters in plan:\n"+plan); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/01833f20/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java index 96cf452..4ecc1fd 100644 --- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java +++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java @@ -463,8 +463,10 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { PlanInterpreterGuessingType planInterpreter = new PlanInterpreterGuessingType(null, item, sourceYaml, itemType, libraryBundles, result).reconstruct(); if (!planInterpreter.isResolved()) { throw Exceptions.create("Could not resolve item" - + (Strings.isNonBlank(id) ? " "+id : Strings.isNonBlank(symbolicName) ? " "+symbolicName : Strings.isNonBlank(name) ? name : "") - // better not to show yaml, takes up lots of space, and with multiple plan transformers there might be multiple errors + + (Strings.isNonBlank(id) ? " '"+id+"'" : Strings.isNonBlank(symbolicName) ? " '"+symbolicName+"'" : Strings.isNonBlank(name) ? " '"+name+"'" : "") + // better not to show yaml, takes up lots of space, and with multiple plan transformers there might be multiple errors; + // some of the errors themselves may reproduce it + // (ideally in future we'll be able to return typed errors with caret position of error) // + ":\n"+sourceYaml , planInterpreter.getErrors()); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/01833f20/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index 01bf992..82bac22 100644 --- a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -124,7 +124,12 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat Map<String,Object> result = MutableMap.of(); for (CatalogItem<?,?> item: items) { - result.put(item.getId(), CatalogTransformer.catalogItemSummary(brooklyn(), item)); + try { + result.put(item.getId(), CatalogTransformer.catalogItemSummary(brooklyn(), item)); + } catch (Throwable t) { + log.warn("Error loading catalog item '"+item+"' (rethrowing): "+t); + throw Exceptions.propagate(t); + } } return Response.status(Status.CREATED).entity(result).build(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/01833f20/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java index 38a38eb..978755b 100644 --- a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java +++ b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java @@ -93,9 +93,12 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable> { } } - Builder rb = ApiError.builderFromThrowable(Exceptions.collapse(throwable2)); + Throwable throwable3 = Exceptions.collapse(throwable2); + Builder rb = ApiError.builderFromThrowable(throwable3); if (Strings.isBlank(rb.getMessage())) rb.message("Internal error. Contact server administrator to consult logs for more details."); + if (Exceptions.isPrefixImportant(throwable3)) + rb.message(Exceptions.getPrefixText(throwable3)+": "+rb.getMessage()); return rb.build().asResponse(Status.INTERNAL_SERVER_ERROR, MediaType.APPLICATION_JSON_TYPE); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/01833f20/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java ---------------------------------------------------------------------- diff --git a/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java b/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java index 9643018..314961a 100644 --- a/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java +++ b/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/Exceptions.java @@ -33,6 +33,7 @@ import java.util.concurrent.ExecutionException; import javax.annotation.Nullable; import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.javalang.JavaClassNames; import org.apache.brooklyn.util.text.Strings; import com.google.common.base.Predicate; @@ -324,7 +325,23 @@ public class Exceptions { return new CompoundRuntimeException(prefix, exceptions); } if (Strings.isBlank(prefix)) return new CompoundRuntimeException(exceptions.size()+" errors, including: " + Exceptions.collapseText(exceptions.iterator().next()), exceptions); - return new CompoundRuntimeException(prefix+", "+exceptions.size()+" errors including: " + Exceptions.collapseText(exceptions.iterator().next()), exceptions); + return new CompoundRuntimeException(prefix+"; "+exceptions.size()+" errors including: " + Exceptions.collapseText(exceptions.iterator().next()), exceptions); + } + + /** Some throwables require a prefix for the message to make sense, + * for instance NoClassDefFoundError's message is often just the type. + */ + public static boolean isPrefixImportant(Throwable t) { + if (t instanceof NoClassDefFoundError) return true; + return false; + } + + /** For {@link Throwable} instances where know {@link #isPrefixImportant(Throwable)}, + * this returns a nice message for use as a prefix; otherwise this returns the class name. + * Callers should typically suppress the prefix if {@link #isPrefixBoring(Throwable)} is true. */ + public static String getPrefixText(Throwable t) { + if (t instanceof NoClassDefFoundError) return "Type not found"; + return JavaClassNames.cleanSimpleClassName(t); } }
