[
https://issues.apache.org/jira/browse/NIFI-3142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15722938#comment-15722938
]
ASF GitHub Bot commented on NIFI-3142:
--------------------------------------
Github user mattyb149 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1290#discussion_r90924068
--- Diff:
nifi-nar-bundles/nifi-hl7-bundle/nifi-hl7-processors/src/main/java/org/apache/nifi/processors/hl7/ExtractHL7Attributes.java
---
@@ -196,8 +196,8 @@ public void process(final InputStream in) throws
IOException {
final Map<String, String> attributes = getAttributes(message,
useSegmentNames, parseSegmentFields);
flowFile = session.putAllAttributes(flowFile, attributes);
getLogger().debug("Added the following attributes for {}: {}",
new Object[]{flowFile, attributes});
- } catch (final HL7Exception e) {
- getLogger().error("Failed to extract attributes from {} due to
{}", new Object[]{flowFile, e});
+ } catch (final Throwable t) {
--- End diff --
Where in the code do the NPE and NFE happen? If they are in NiFi code we
should be catching both (possibly throwing a new exception we can
expect/handle).
I see what you're saying about avoiding backups of flow files that could be
processed successfully, but if they can be processed successfully then the
processor should be able to know what is going on in order to make a smart
decision about how to handle the flow files. Catching Throwable means you would
try to handle a flow file as "failed" when it is perfectly fine as input but
something else is going on under the hood. This could keep the flow running
even though something bad is going on having nothing to do with flow files, and
could lead to data loss or non-delivery (which is why the session is being
rolled back on these exceptions, to preserve the data/flow).
I think we should capture non-happy-paths at the business logic level where
possible (such as the NPE and NFE you mention). If you can demonstrate
additional errors that could occur based on the business logic (HL7 library or
NiFi itself), we should catch those too (all could be handled in the same
block). I realize if we miss one then we'll have yet another Jira to fix it,
but I think it's good to try to avoid the catch-everything block.
Probably good to get some more opinions, I'll tag @markap14 and @JPercivall
for additional input here :)
> ExtractHL7Attributes processor does not route to REL_FAILURE for an exception
> other than an HL7Exception
> --------------------------------------------------------------------------------------------------------
>
> Key: NIFI-3142
> URL: https://issues.apache.org/jira/browse/NIFI-3142
> Project: Apache NiFi
> Issue Type: Bug
> Components: Extensions
> Affects Versions: 1.0.0
> Reporter: Kirby Linvill
> Priority: Minor
> Labels: easyfix
>
> The ExtractHL7Attribute processor will not route a flowfile to REL_FAILURE if
> processing the flow file throws an exception that is not an HL7Exception.
> This bug is a result of the try catch block in the onTrigger method. It is
> easily fixed by changing the catch block to catch a throwable object. Change
> {noformat}
> try {
> ...
> } catch (final HL7Exception e) {
> getLogger().error("Failed to extract attributes from {} due to {}", new
> Object[]{flowFile, e}, e);
> session.transfer(flowFile, REL_FAILURE);
> return;
> }
> {noformat}
> to
> {noformat}
> try {
> ...
> } catch (final Throwable t) {
> getLogger().error("Failed to extract attributes from {} due to {}", new
> Object[]{flowFile, t}, t);
> session.transfer(flowFile, REL_FAILURE);
> return;
> }
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
