From e27979870db1358da1477c6d13b05c510c2045d8 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 2 Oct 2015 12:18:50 -0400 Subject: [PATCH] Add/improve yang parser error reporting If a yang file fails to load/parse in karaf, it fails silently. The first problem is that YangTextSchemaContextResolver#getSchemaContext logs the SchemaResolutionException to debug as it retries with getResolvedSources. But if getResolvedSources is empty, it ends up silently returning an empty SchemaContext. Another issue is that the underlying exceptions that are really relevant are added as suppressed to the aggregating exception. This would be fine if the underlying logger in karaf printed suppressed exceptions as Throwable#printStackTrace does but it doesn't. The suppressed exceptions do appear from maven (that underlying logger uses printStackTrace). My first thought was to log the SchemaResolutionException if SchemaResolutionException was empty but use printStackTrace to get the suppressed exceptions. However there are 3 levels of exceptions which is kind of ugly - one needs to sift thru a lot of noise and long stack traces to find the relevant error. So. to make it more user-friendly, I instead modified BundleGlobalContext to log each source exception's suppressed exceptions as these contain the actual parsing failures. I also only printed the exception message normally to avoid the stack trace noise. The full trace can be seen with debug enabled. Change-Id: Ie603bab3caa4fabc421ffbd106fddd5176743d2b Signed-off-by: Tom Pantelis Signed-off-by: Robert Varga --- .../plugin/YangToSourcesProcessor.java | 4 +- .../repo/YangTextSchemaContextResolver.java | 2 +- .../stmt/reactor/BuildGlobalContext.java | 43 +++++++++++++++++-- .../stmt/reactor/SourceSpecificContext.java | 22 +++++++--- .../stmt/rfc6020/YangStatementSourceImpl.java | 5 +++ .../stmt/retest/YangParserNegativeTest.java | 15 ++++--- 6 files changed, 73 insertions(+), 18 deletions(-) diff --git a/yang/yang-maven-plugin/src/main/java/org/opendaylight/yangtools/yang2sources/plugin/YangToSourcesProcessor.java b/yang/yang-maven-plugin/src/main/java/org/opendaylight/yangtools/yang2sources/plugin/YangToSourcesProcessor.java index ccbbc3c4d7..195c7f39ba 100644 --- a/yang/yang-maven-plugin/src/main/java/org/opendaylight/yangtools/yang2sources/plugin/YangToSourcesProcessor.java +++ b/yang/yang-maven-plugin/src/main/java/org/opendaylight/yangtools/yang2sources/plugin/YangToSourcesProcessor.java @@ -11,6 +11,7 @@ import java.util.HashSet; import org.opendaylight.yangtools.yang.parser.stmt.reactor.CrossSourceStatementReactor; import org.opendaylight.yangtools.yang.parser.stmt.rfc6020.YangInferencePipeline; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Throwables; import com.google.common.collect.Maps; import java.io.Closeable; import java.io.File; @@ -185,8 +186,9 @@ class YangToSourcesProcessor { // MojoExecutionException is thrown since execution cannot continue } catch (Exception e) { LOG.error("{} Unable to parse {} files from {}", LOG_PREFIX, Util.YANG_SUFFIX, yangFilesRootDir, e); + Throwable rootCause = Throwables.getRootCause(e); throw new MojoExecutionException(LOG_PREFIX + " Unable to parse " + Util.YANG_SUFFIX + " files from " + - yangFilesRootDir, e); + yangFilesRootDir, rootCause); } } diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/YangTextSchemaContextResolver.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/YangTextSchemaContextResolver.java index 32cf1529a0..7c0c9763ce 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/YangTextSchemaContextResolver.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/YangTextSchemaContextResolver.java @@ -203,7 +203,7 @@ public final class YangTextSchemaContextResolver implements AutoCloseable, Schem sc = Optional.of(f.checkedGet()); break; } catch (SchemaResolutionException e) { - LOG.debug("Failed to fully assemble schema context for {}", sources, e); + LOG.info("Failed to fully assemble schema context for {}", sources, e); sources = e.getResolvedSources(); } } diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java index 86307f2c82..14d450d567 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java @@ -43,8 +43,11 @@ import org.opendaylight.yangtools.yang.parser.spi.validation.ValidationBundlesNa import org.opendaylight.yangtools.yang.parser.spi.validation.ValidationBundlesNamespace.ValidationBundleType; import org.opendaylight.yangtools.yang.parser.stmt.reactor.SourceSpecificContext.PhaseCompletionProgress; import org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective.EffectiveSchemaContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBehaviour.Registry { + private static final Logger LOG = LoggerFactory.getLogger(BuildGlobalContext.class); private static final List PHASE_EXECUTION_ORDER = ImmutableList.builder() .add(ModelProcessingPhase.SOURCE_LINKAGE) @@ -125,7 +128,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh @SuppressWarnings({"unchecked", "rawtypes"}) private > NamespaceBehaviourWithListeners createNamespaceContext( - NamespaceBehaviour potentialRaw) { + final NamespaceBehaviour potentialRaw) { if (potentialRaw instanceof DerivedNamespaceBehaviour) { VirtualNamespaceContext derivedContext = new VirtualNamespaceContext((DerivedNamespaceBehaviour) potentialRaw); @@ -211,9 +214,41 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private SomeModifiersUnresolvedException addSourceExceptions(final SomeModifiersUnresolvedException buildFailure, final List sourcesToProgress) { - for(SourceSpecificContext failedSource : sourcesToProgress) { - SourceException sourceEx = failedSource.failModifiers(currentPhase); - buildFailure.addSuppressed(sourceEx); + boolean addedCause = false; + for (SourceSpecificContext failedSource : sourcesToProgress) { + final SourceException sourceEx = failedSource.failModifiers(currentPhase); + + // Workaround for broken logging implementations which ignore suppressed exceptions + Throwable cause = sourceEx.getCause() != null ? sourceEx.getCause() : sourceEx; + if (LOG.isDebugEnabled()) { + LOG.error("Failed to parse YANG from source {}", failedSource, sourceEx); + } else { + LOG.error("Failed to parse YANG from source {}: {}", failedSource, cause.getMessage()); + } + + final Throwable[] suppressed = sourceEx.getSuppressed(); + if (suppressed.length > 0) { + LOG.error("{} additional errors reported:", suppressed.length); + + int i = 1; + for (Throwable t : suppressed) { + // FIXME: this should be configured in the appender, really + if (LOG.isDebugEnabled()) { + LOG.error("Error {}: {}", i, t.getMessage(), t); + } else { + LOG.error("Error {}: {}", i, t.getMessage()); + } + + i++; + } + } + + if(!addedCause) { + addedCause = true; + buildFailure.initCause(sourceEx); + } else { + buildFailure.addSuppressed(sourceEx); + } } return buildFailure; } diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java index 34959de19a..86ff1afa87 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java @@ -14,6 +14,7 @@ import com.google.common.collect.Multimap; import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Objects; import javax.annotation.Nullable; @@ -276,17 +277,28 @@ public class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeh } SourceException failModifiers(final ModelProcessingPhase identifier) { - InferenceException sourceEx = new InferenceException("Fail to infer source relationships", root.getStatementSourceReference()); - - + final List exceptions = new ArrayList<>(); for (ModifierImpl mod : modifiers.get(identifier)) { try { mod.failModifier(); } catch (SourceException e) { - sourceEx.addSuppressed(e); + exceptions.add(e); } } - return sourceEx; + + final String message = String.format("Yang model processing phase %s failed", identifier); + if (exceptions.isEmpty()) { + return new InferenceException(message, root.getStatementSourceReference()); + } + + final InferenceException e = new InferenceException(message, root.getStatementSourceReference(), + exceptions.get(0)); + final Iterator it = exceptions.listIterator(1); + while (it.hasNext()) { + e.addSuppressed(it.next()); + } + + return e; } void loadStatements() throws SourceException { diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/YangStatementSourceImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/YangStatementSourceImpl.java index 8a5519f678..7910044ba3 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/YangStatementSourceImpl.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/YangStatementSourceImpl.java @@ -141,6 +141,11 @@ public final class YangStatementSourceImpl implements StatementStreamSource { return statementContext; } + @Override + public String toString() { + return sourceName; + } + // public InputStream getSourceStream() { // return sourceStream; // } diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/retest/YangParserNegativeTest.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/retest/YangParserNegativeTest.java index bfaea689bc..9d82d20d8f 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/retest/YangParserNegativeTest.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/retest/YangParserNegativeTest.java @@ -24,6 +24,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedEx import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; import org.opendaylight.yangtools.yang.parser.util.YangParseException; import org.opendaylight.yangtools.yang.parser.util.YangValidationException; +import com.google.common.base.Throwables; public class YangParserNegativeTest { @@ -36,10 +37,10 @@ public class YangParserNegativeTest { fail("SomeModifiersUnresolvedException should be thrown"); } } catch (SomeModifiersUnresolvedException e) { - final Throwable suppressed2levelsDown = e.getSuppressed()[0].getSuppressed()[0]; - assertTrue(suppressed2levelsDown instanceof InferenceException); - assertTrue(suppressed2levelsDown.getMessage().startsWith("Imported module")); - assertTrue(suppressed2levelsDown.getMessage().endsWith("was not found.")); + Throwable rootCause = Throwables.getRootCause(e); + assertTrue(rootCause instanceof InferenceException); + assertTrue(rootCause.getMessage().startsWith("Imported module")); + assertTrue(rootCause.getMessage().endsWith("was not found.")); } } @@ -73,11 +74,11 @@ public class YangParserNegativeTest { } } } catch (SomeModifiersUnresolvedException e) { - final Throwable suppressed2levelsDown = e.getSuppressed()[0].getSuppressed()[0]; - assertTrue(suppressed2levelsDown instanceof InferenceException); + final Throwable rootCause = Throwables.getRootCause(e); + assertTrue(rootCause instanceof InferenceException); assertEquals( "Augment target not found: Absolute{path=[(urn:simple.container.demo?revision=1970-01-01)unknown]}", - suppressed2levelsDown.getMessage()); + rootCause.getMessage()); } } -- 2.36.6