[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user olegz commented on the pull request: https://github.com/apache/nifi/pull/285#issuecomment-200557606 Also, you may want to amend the commit message "Unepected" unless its a new English word I haven't learned yet ;) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user olegz commented on the pull request: https://github.com/apache/nifi/pull/285#issuecomment-200557146 Ricky Could you please take a look at https://issues.apache.org/jira/browse/NIFI-1447 and see if it's what you are looking for? If I remember correctly we don't really want to log stack traces unless we're in DEBUG mode and that is what the above JIRA addresses. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56422806
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
Sure thing, my battery just died so I will as soon as able. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
GitHub user rickysaltzer opened a pull request: https://github.com/apache/nifi/pull/285 NIFI-1636: Print Stacktrace When Unepected OnTrigger Exception Caught You can merge this pull request into a Git repository by running: $ git pull https://github.com/rickysaltzer/nifi NIFI-1636 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/285.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #285 commit e82464694435ee2e5f27cfbb00c0e6d7b53d89ce Author: ricky Date: 2016-03-16T20:43:05Z NIFI-1636: Print Stacktrace When Unepected OnTrigger Exception Caught --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56420919
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
Right. So i think the short version is in the UI now. But you want the
stacktrace too. We should make that available in the logs. And this class is
what controls that I believe
https://github.com/apache/nifi/blob/master/nifi-commons/nifi-logging-utils/src/main/java/org/apache/nifi/logging/NiFiLog.java
You'll see it has some ifDebugEnabled guards. We should probably just toss
those out. @markap14 what say you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56418107
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
This logic needs to be in the logger and that is its intent. You will note
that the throwable is being passed into it. The stack traces are getting
logged when debug level is engaged for a given logger. This allows the user to
control whether they are generated or not. That said...it is clearly
counterintuitive and we need to improve.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56420228
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
that is actually the current behavior/intent. I think the change needed is
to simply get rid of the 'if debug enabled' stuff we did in the logger and
instead just always log the stacktrace when it is there. This is what Adam
Taft had advocated for previously as I recall. I now see why he was saying it.
So yeah in the UI it should be a short and ideally meaningful message (not
always possible) and if a throwable shows up we should put the whole stack
trace in the logs.
The current idea that a user will go in an do debug enabledjust has
proven to be a failed experiment in my view and as someone who advocated for
that I now think i was wrong.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user rickysaltzer commented on the pull request: https://github.com/apache/nifi/pull/285#issuecomment-197998896 @joewitt posted a new version of the patch. I tested it out on my local machine and it seems to work as expected. That is, the stacktrace is logged to the `nifi-app.log`, while the the shortened version is logged to the UI. The `contrib-check` also checks out ;) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56418891
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
That makes sense @joewitt. Should legitimate unexpected exceptions be
surfaced to `INFO` by default? I feel like these are uncommon enough to warrant
it, but I could be wrong.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56419500
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
I would say generally no. They are uncommon in general but when they
happen they happen in bursts. As a general rule I think we should strive to
make what the user sees be something a lot more friendly than a wild stack
trace. But, the logs should have them. We did discuss this on dev list a
while back and there were some great inputs from various folks. We def need to
do something here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56420502
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
So if I understand correctly, this happens now but only in `DEBUG` mode.
What we would like to see is that behavior but even if we're in `INFO` mode.
That is, shortened version logged to the UI and the stacktrace logged to the
log file?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56421131
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
Yep that would be the easiest way, and I can go ahead and throw this mess I
added in above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56421212
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
I should clarify...i agree with you this is confusing and not quite doing
what we want. We need to fix it. Can you share a screenshot of what you see
for a given error in the UI vs what shows in the logs and then let's agree on
what it should ideally be and then figure out how to get there :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56419796
--- Diff:
nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext
context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
-getLogger().error("{} failed to process due to {}; rolling
back session", new Object[]{this, t});
+StringWriter stacktraceWriter = new StringWriter();
--- End diff --
I wonder if it would make sense to surface the shorthand message to the
user in the UI but log the stacktrace to the log file. I imagine this would
take some re-working.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
