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 :)
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
