NIFI-6201 Suppressed stacktraces for expected error conditions (conne…

[alopresto]

…ction timeout, missing component bundle, etc.) in application startup when the stacktrace provides no additional helpful data.

The easiest way to test this PR is to create a flow that has a Registry Client pointing to a NiFi Registry instance and has components (controller service & processor) from a custom NAR (i.e. https://github.com/influxdata/nifi-influxdb-bundle) then stop NiFi, remove the custom NAR, stop the Registry, and then start NiFi and tail -f logs/nifi-app.log to look for stacktraces (they should be absent).

Thank you for submitting a contribution to Apache NiFi.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

• Is there a JIRA ticket associated with this PR? Is it referenced
  in the commit message?

• Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

• Has your PR been rebased against the latest commit within the target branch (typically master)?

• Is your initial contribution a single, squashed commit?

For code changes:

• Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
• Have you written or updated unit tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
• If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
• If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

• Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

[alopresto]

I recommend turning whitespace diff off for this PR as the checkstyle & coding style formatter applied a lot of changes.

[joewitt]

What this PR aims to do is really good which is avoid an exception stack dump showing up for the user on a very expected/normal condition that when it happens we just want to tell the user "Hey the bundle you're trying to load isn't available". We should definitely fix this. Given that as implemented it is based on exception strings it could be brittle. If the string changes this would break but breaking means it would just behave like it does today. In that sense this PR as-is is probably +1-able since it does no harm but for the likely case will help.

However, is there a non exception parsing path we can take here to get the details we want so this fix can be more permanent. @bbende is this something you'll have time to consider soon?

If not I'll merge this tomorrow assuming there are no strong disagreements.

[alopresto]

I acknowledge the current approach is brittle but did not want to change to generate more specific exceptions because there may be existing dependencies on the current exception ecosystem and I did not perform exhaustive analysis to see where side-effects would occur if this changed.

[joewitt]

yep. lets see what bryan (or anyone else) says. We can do this approach now and a cleaner approach at a later date.

[bbende]

I don't have too strong of a preference, but for the bundle case could we just append the message of the exception to the errorMessage string, and then have a debug option to log the whole stracktrace? This would be kind of similar to how our exception mappers work for the REST API.

Something like:

logger.error("Could not create Processor of type " + type + " for ID " + identifier + " due to: " + pie.getMessage() + "; creating \"Ghost\" implementation");
if (logger.isDebugEnabled()) {
  logger.debug(e.getMessage(), e);
}

[alopresto]

I can add that in. I didn't think the stacktrace provided additional relevant information, but it's easy to put in the debug block.

[bbende]

That is probably true... I guess the main question was can we just take pie.getMessage() rather than searching through the caused-by and using substring? or are there cases where its more than one level deep? The example in the JIRA was just one IllegalStateException.

[alopresto]

I think there may be cases where it is deeper, but in every case I encountered, it was just the one. I'll switch to the cleaner construction you offered above.

[alopresto]

Thanks @bbende . I simplified the output like you asked for.

[kevdoran]

Hi @alopresto, I was able to verify the changes in response to @bbende's request. +1, will merge