Github user JPercivall commented on the issue:

    https://github.com/apache/nifi/pull/2113
  
    Thanks @MikeThomsen, looks like that fixed the NPE. Still needs the License 
and Notice in the client service nar.
    
    Also, it's a bit of an anti-pattern to have the logging and error handling 
in the Controller Service for methods called by the processor. This is because 
any time there is a problem with the search, the bulletin is logged at the 
Controller Service. This means that not only will a user not know which 
processor is the cause of the error (since it could be shared) but also when 
looking at the map the processor appears to be working without issue. Lastly, 
since the error isn't bubbled back to the processor, it never yields so if 
there is something wrong with the query it will continually error and log 
without pause. 
    
    Related, there's also some weirdness with error handling when there is an 
incoming connection. Currently, input is only sent to Failure instead of 
Original when an error is caught at the top level. This means that if a query 
is messed up and is caught as a JsonProcessingException in the Controller 
service, the input will be routed to original and no other outputs. As a user, 
I would expect that flowfile to be sent to Failure.
    
    So in summary, the CS should let the calling processor decide how to handle 
any exceptions encountered during processing. This allows the processor to more 
granularlly decide what to do under different situations. For example, was it a 
JSON parsing exception and it's acting as a source processor? well we better 
yield the whole processor since it's probably just going to continually fail. 
Is there an incoming connection but some network error happened when searching? 
again, later FlowFiles will probably fail too, so we should yield. If you have 
any questions on when to yield the processor vs penalizing the FlowFile, feel 
free to ask.
    
    Lastly, it's preferred if you don't squash your commits every time. If you 
don't, that allows the reviewer to more easily see exactly what changed since 
they last reviewed it. Also allows reviewers to see how the PR evolved over 
time in response to different comments.


---

Reply via email to