10 Replies Latest reply on Feb 18, 2009 6:56 PM by dmlloyd

Those terrible error messages

dmlloyd Jan 28, 2009 10:22 AM

One of the most off-putting things about JBossMC is the confusing, non-human error messages it produces when a deployment fails. Carlo's thread gave me the notion to try and fix this. Here's the patch to AbstractKernelDeployer which humanizes the error message. What do you think? Should I commit it?

Index: kernel/src/main/java/org/jboss/kernel/plugins/deployment/AbstractKernelDeployer.java
===================================================================
--- kernel/src/main/java/org/jboss/kernel/plugins/deployment/AbstractKernelDeployer.java
+++ kernel/src/main/java/org/jboss/kernel/plugins/deployment/AbstractKernelDeployer.java
@@ -217,22 +217,21 @@
 buffer.append("Incompletely deployed:\n");
 if (errors.size() != 0)
 {
- buffer.append("\n*** DEPLOYMENTS IN ERROR: Name -> Error\n");
+ buffer.append("\nDEPLOYMENTS IN ERROR:\n");
 for (ControllerContext ctx : errors)
 {
- buffer.append(ctx.getName()).append(" -> ").append(ctx.getError().toString()).append('\n');
+ buffer.append(String.format(" Deployment \"%s\" is in error due to: %s\n", ctx.getName(), ctx.getError()));
 }
 }
 if (incomplete.size() != 0)
 {
- buffer.append("\n*** DEPLOYMENTS MISSING DEPENDENCIES: Name -> Dependency{Required State:Actual State}\n");
+ buffer.append("\nDEPLOYMENTS MISSING DEPENDENCIES:\n");
 for (ControllerContext ctx : incomplete)
 {
 Object name = ctx.getName();
- buffer.append(name).append(" -> ");
+ buffer.append(String.format(" Deployment \"%s\" is missing the following dependencies:\n", name));
 DependencyInfo dependsInfo = ctx.getDependencyInfo();
 Set<DependencyItem> depends = dependsInfo.getIDependOn(null);
- boolean first = true;
 for (DependencyItem item : depends)
 {
 ControllerState dependentState = item.getDependentState();
@@ -262,29 +261,18 @@

 if (print)
 {
- if (first)
- first = false;
- else
- buffer.append(", ");
-
- buffer.append(iDependOn).append('{').append(dependentState.getStateString());
- buffer.append(':');
- if (iDependOn == null)
- {
- buffer.append("** UNRESOLVED " + item.toHumanReadableString() + " **");
- }
- else
- {
- if (other == null)
- buffer.append("** NOT FOUND **");
- else
- buffer.append(otherState.getStateString());
- }
- buffer.append('}');
+ buffer.append(String.format(" Dependency \"%s\" (should be in state \"%s\", but is actually %s)\n",
+ iDependOn,
+ dependentState.getStateString(),
+ iDependOn == null ?
+ String.format("unresolved (%s)",
+ item.toHumanReadableString()) :
+ other == null ?
+ "not found" :
+ String.format("in state \"%s\"", otherState.getStateString())));
 }
 }
 }
- buffer.append('\n');
 }
 }
 throw new IllegalStateException(buffer.toString());

1. Re: Those terrible error messages

timfox Jan 28, 2009 10:30 AM (in response to dmlloyd)

"david.lloyd@jboss.com" wrote:
One of the most off-putting things about JBossMC is the confusing, non-human error messages it produces when a deployment fails.

+1. I was thinking exactly the same thing just the other day while trying to debug while JBM 2.0 wasn't loading in the MC, and struggling with the alien language.

Anything to humanise it would be winner imho.
Actions
2. Re: Those terrible error messages

alesj Jan 28, 2009 10:43 AM (in response to dmlloyd)

"david.lloyd@jboss.com" wrote:
Should I commit it?

Sure, go ahead.
But this is not the only place this is done.
Search Deployers code for similar error msg aka 'alien language'. ;-)

You must fix them both or none. :-)
Actions
3. Re: Those terrible error messages

alesj Jan 28, 2009 10:44 AM (in response to dmlloyd)

"timfox" wrote:

I was thinking exactly the same thing just the other day while trying to debug while JBM 2.0 wasn't loading in the MC, and struggling with the alien language.

Don't blame MC if JBM doesn't work. :-)
Actions
4. Re: Those terrible error messages

trustin Jan 28, 2009 11:00 AM (in response to dmlloyd)

An off topic - I'd suggest you to replace '\n' with something more platform independent.

For instance, Formatter got '%n'. I think we can't use System.getProperty() though due to the security issue as you already know. Perhaps there was something in JBoss Common? ;-)
Actions

5. Re: Those terrible error messages

dmlloyd Jan 28, 2009 11:10 AM (in response to dmlloyd)

"alesj" wrote:
Sure, go ahead.
But this is not the only place this is done.
Search Deployers code for similar error msg aka 'alien language'. ;-)

You must fix them both or none. :-)

Ales, Ales, Ales. :-)

OK, I found some stuff in jboss-deployers-client-spi which I imagine is it. Here's that patch:

Index: deployers-client-spi/src/main/java/org/jboss/deployers/client/spi/IncompleteDeployments.java
===================================================================
--- deployers-client-spi/src/main/java/org/jboss/deployers/client/spi/IncompleteDeployments.java
+++ deployers-client-spi/src/main/java/org/jboss/deployers/client/spi/IncompleteDeployments.java
@@ -30,6 +30,7 @@
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.Iterator;

 /**
 * IncompleteDeployments.
@@ -261,19 +262,18 @@
 // Display all the missing dependencies
 if (contextsMissingDependencies.isEmpty() == false)
 {
- buffer.append("\n*** CONTEXTS MISSING DEPENDENCIES: Name -> Dependency{Required State:Actual State}\n\n");
+ buffer.append("\nDEPLOYMENTS MISSING DEPENDENCIES:\n");
 for (Map.Entry<String, Set<MissingDependency>> entry : contextsMissingDependencies.entrySet())
 {
 String name = entry.getKey();
- buffer.append(name).append("\n");
+ buffer.append(String.format(" Deployment \"%s\" is missing the following dependencies:\n", name));
 for (MissingDependency dependency : entry.getValue())
 {
- buffer.append(" -> ").append(dependency.getDependency());
- buffer.append('{').append(dependency.getRequiredState());
- buffer.append(':').append(dependency.getActualState()).append("}");
- buffer.append("\n");
+ buffer.append(String.format(" Dependency \"%s\" (should be in state \"%s\", but is actually in state \"%s\")\n",
+ dependency.getDependency(),
+ dependency.getRequiredState(),
+ dependency.getActualState()));
 }
- buffer.append('\n');

 // It is not a root cause if it has missing dependencies
 rootCauses.remove(name);
@@ -286,22 +286,19 @@

 if (rootCauses.isEmpty() == false)
 {
- buffer.append("\n*** CONTEXTS IN ERROR: Name -> Error\n\n");
+ buffer.append("\nDEPLOYMENTS IN ERROR:\n");
 for (String key : rootCauses.keySet())
 {
- buffer.append(key).append(" -> ");
+ buffer.append(String.format(" Deployment \"%s\" is in error due to the following reason(s): ", key));
 Set<String> values = rootCauses.get(key);
- boolean first = true;
- for (String value : values)
- {
- if (first == false)
- buffer.append(" | ");
- else
- first = false;
-
- buffer.append(value);
- }
- buffer.append("\n\n");
+ Iterator<String> it = values.iterator();
+ while (it.hasNext())
+ {
+ buffer.append(it.next());
+ if (it.hasNext())
+ buffer.append(", ");
+ }
+ buffer.append("\n");
 }
 }
 contextsInErrorInfo = buffer.toString();

6. Re: Those terrible error messages

dmlloyd Jan 28, 2009 11:11 AM (in response to dmlloyd)

"trustin" wrote:
An off topic - I'd suggest you to replace '\n' with something more platform independent.

For instance, Formatter got '%n'. I think we can't use System.getProperty() though due to the security issue as you already know. Perhaps there was something in JBoss Common? ;-)

I believe that the logging framework should be taking care of this. Anyway we use '\n' very, very pervasively and I'm not going to try to fix every usage of it. :-)
Actions
7. Re: Those terrible error messages

dmlloyd Jan 28, 2009 11:14 AM (in response to dmlloyd)

In any case, let me know if I shouldn't commit this, otherwise I will... today. :-)
Actions
8. Re: Those terrible error messages

alesj Jan 28, 2009 11:21 AM (in response to dmlloyd)

"david.lloyd@jboss.com" wrote:
In any case, let me know if I shouldn't commit this, otherwise I will... today. :-)

DML, DML, DML ... commit them both. :-)
Actions
9. Re: Those terrible error messages

dmlloyd Jan 28, 2009 11:31 AM (in response to dmlloyd)

Committed.
Actions

10. Re: Those terrible error messages

dmlloyd Feb 18, 2009 6:56 PM (in response to dmlloyd)

Doing some testing in JBAS trunk, I triggered the new error message:

Failed to load profile: Summary of incomplete deployments (SEE PREVIOUS ERRORS FOR DETAILS):

DEPLOYMENTS MISSING DEPENDENCIES:
 Deployment "TestTCPServer" is missing the following dependencies:
 Dependency "XnioProvider" (should be in state "Instantiated", but is actually in state "Described")
 Deployment "XnioProvider" is missing the following dependencies:
 Dependency "XnioSelectorThreadFactory" (should be in state "Instantiated", but is actually in state "Described")
 Deployment "XnioSelectorThreadFactory" is missing the following dependencies:
 Dependency "XnioSelectorThreadGroup" (should be in state "Instantiated", but is actually in state "Described")
 Dependency "XnioThreadInterruptHandler" (should be in state "Instantiated", but is actually in state "** NOT FOUND Depends on 'XnioThreadInterruptHandler' **")
 Deployment "XnioSelectorThreadGroup" is missing the following dependencies:
 Dependency "xnio" (should be in state "Instantiated", but is actually in state "** NOT FOUND Depends on 'xnio' **")

DEPLOYMENTS IN ERROR:
 Deployment "xnio" is in error due to the following reason(s): ** NOT FOUND Depends on 'xnio' **
 Deployment "XnioThreadInterruptHandler" is in error due to the following reason(s): ** NOT FOUND Depends on 'XnioThreadInterruptHandler' **

It looks pretty nice I think. One thing that doesn't quite make sense though is the "** NOT FOUND" part... it says "Depends on XXX" but it's listing its own bean there. If I get a minute, I'll try to find where this is coming from and turn it into a nicer "not found" message...

Go to original post