Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WINDUP-254 Improve error message in RuleSubset. #229

Closed
wants to merge 1 commit into from

Conversation

OndraZizka
Copy link
Contributor

The current message misses the original message from the cause exception.

@mbriskar
Copy link
Contributor

mbriskar commented Sep 1, 2014

+1. That ex.getMessage(); is really needed, the exception itself is not shown in the console output and having only information about which rule failed is not sufficient. As Ondra said, something is not letting exception to get through to output. So this fix is just a workaround, because message itself is shown in the output. The actual fix would be to propagate that exception all the way to console
output.

EDIT: However, after a deeper look inside, not sure if the message would be enough.

@lincolnthree
Copy link
Contributor

When you say the exception is not shown in the console output, what do you mean? Is this the same surefire issue you're talking about?

@lincolnthree
Copy link
Contributor

What about an alternate solution? Simply log the entire message as ERROR: before throwing the new exception? Thoughts?

@OndraZizka
Copy link
Contributor Author

@OndraZizka
Copy link
Contributor Author

"log the entire message as ERROR: before throwing the new exception?"

That was exactly the solution that you and Pete rejected in other PR for the case of XML parsing error.

@OndraZizka
Copy link
Contributor Author

lincolnthree commented 28 days ago

Hiding the root cause basically makes this type of error useless IMO, since you pretty much always need to see the underlying exception to figure out what really happened. IOException "Could not open input stream, etc" is really, "File not found".

#164

@jsight
Copy link
Member

jsight commented Sep 3, 2014

I'd like to see an example of what an error looks like with this formatting. I do think that it is important that we surface the root cause like this, though. I'm generally in favor of this.

(although alternatively, it could just be printed to the forge.log as lincoln suggested)

@lincolnthree
Copy link
Contributor

The log is purely a resignation to help you guys debug. I really don't think it should be necessary, but obviously something is wrong with Surefire, so adding an extra log statement isn't the end of the world. If it helps you guys be more productive, I'd rather do that than have you suffer.

@lincolnthree
Copy link
Contributor

So change this to log instead of changing the exception message and we can merge.

@OndraZizka
Copy link
Contributor Author

Without:

org.jboss.windup.util.exception.WindupException_$$_javassist_489f618e-20fb-4ee3-aa90-4169b4e20304: Error encountered while evaluating rule: .addRule().when(org.jboss.windup.config.query.Query@55b21fe3).perform(org.jboss.windup.config.operation.Iteration@214b9c09) from: org.jboss.windup.reporting.rules.CreateApplicationReportIndexRuleProvider defined in: org.jboss.windup.reporting.rules.CreateApplicationReportIndexRuleProvider.getConfiguration(CreateApplicationReportIndexRuleProvider.java:61)

@OndraZizka
Copy link
Contributor Author

With:

org.jboss.windup.util.exception.WindupException_$$_javassist_5561f66c-decb-427f-ad36-50bda9eea530:
Error, no project found in source-based input directory: /home/ondra/work/Migration/Windup/rules/app/java/src/test/java/org/jboss/windup/rules/java
Error encountered while evaluating rule: .addRule().when(org.jboss.windup.config.query.Query@765d5902).perform(org.jboss.windup.config.operation.Iteration@6a7ecf03)
From: org.jboss.windup.reporting.rules.CreateApplicationReportIndexRuleProvider
Defined in: org.jboss.windup.reporting.rules.CreateApplicationReportIndexRuleProvider.getConfiguration(CreateApplicationReportIndexRuleProvider.java:61)

I dare to say the new format is more readable. Github removes the formatting. See https://issues.jboss.org/browse/WINDUP-254

@jsight
Copy link
Member

jsight commented Sep 8, 2014

Where are you getting that output?

@OndraZizka
Copy link
Contributor Author

The output is in my branch Branch: WindupProcessor-WINDUP-253

@OndraZizka OndraZizka closed this Sep 18, 2014
@OndraZizka OndraZizka deleted the RuleSubset-msg branch September 18, 2014 21:27
PhilipCattanach pushed a commit to PhilipCattanach/windup that referenced this pull request Feb 20, 2023
#close WINDUPRULE-240 WINDUPRULE-239 WINDUPRULE-238
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants