nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Payne <marka...@hotmail.com>
Subject Re: Error handling in @OnScheduled
Date Fri, 24 Aug 2018 15:41:36 GMT
James,

Yes, makes perfect sense. I think that's a good balance of logic, as well. Use a validator
to quickly
ensure that the file exists. Then, when trying to use it, go ahead and parse the data and
set your
member variable. You could have the validator parse the file (but not set the member variable)
to
ensure that the format is valid, etc. but I would personally avoid doing that, because the
parsing
may well prove to be quite expensive for validation purposes. I think you're very much on
the right track.

Thanks!
-Mark



> On Aug 24, 2018, at 11:34 AM, James Srinivasan <james.srinivasan@gmail.com> wrote:
> 
> Mark,
> 
> Thanks very much for the detailed answer. In my particular case, I
> have a parameter corresponding to a schema file on disk and there is a
> standard validator to ensure that the file exists. Currently, in my
> @OnScheduled method, I read that schema file, parse it and store the
> parsed results in a member of my class ready for use in my onTrigger
> method. If the file fails to parse, an exception is thrown. I could
> move that code into a validator, but setting a member as a side effect
> of validation didn't quite feel right - does that makes sense?
> 
> James
> On Fri, 24 Aug 2018 at 16:01, Mark Payne <markap14@hotmail.com> wrote:
>> 
>> James,
>> 
>> You can certainly catch Throwable there, or AssertionError, more specifically, but
I'd be very wary
>> of doing that, because at that point you're really kind of working against the framework
(both the
>> nifi mock/test framework as well as the JUnit framework) instead of with it. If your
intent is to test
>> a specific method, I would recommend testing that method explicitly by calling it
yourself.
>> 
>> You don't need to create your own MockProcessContext. You can get the ProcessContext
from
>> the Test Runner. For example:
>> 
>> 
>> final MyProcessor myProcessor = new MyProcessor();
>> final TestRunner runner = TestRunners.newTestRunner(myProcessor);
>> 
>> runner.setProperty(MyProcessor.MY_PROPERTY, "hello");
>> 
>> try {
>>  myProcessor.onScheduled( runner.getProcessContext() );
>>  Assert.fail("Expected SomeException to get thrown from onScheduled method but it
did not.");
>> } catch (final SomeException e) {
>>  // expected.
>> }
>> 
>> Now, this being said, it begs the question whether or not you want to be throwing
an Exception from your @OnScheduled method.
>> I'm sure there are use cases where this makes perfect sense. However, you should
first think about whether or not you are
>> able to prevent the Exception from occurring by applying validation rules (addValidator()
to PropertyDescriptor's or customValidate).
>> The benefit to validators here is that when the user configures the Processor incorrectly,
they get a clear indication immediately that it
>> is not valid and a clear explanation of why it's not valid (as well as being shown
in the Invalid Counts of Process Groups, etc.).
>> If you wait until the user tries to start the Processor and throw an Exception, it
will be less obvious that there's a configuration problem
>> and the error message that they receive is likely not to be as clear.
>> 
>> Thanks
>> -Mark
>> 
>> 
>> On Aug 23, 2018, at 5:25 PM, James Srinivasan <james.srinivasan@gmail.com<mailto:james.srinivasan@gmail.com>>
wrote:
>> 
>> Ah, hadn't spotted that. It's close, but the Throwable I get is a
>> java.lang.AssertionError (Could not invoke methods annotated with
>> @OnScheduled annotation due to:
>> java.lang.reflect.InvocationTargetException) and there doesn't seem to
>> be any way to get the actual underlying exception my code threw in
>> order to properly validate it.
>> 
>> Mark's suggestion of calling the @OnScheduled method directly seems a
>> little tricky when using the TestRunner framework, or should I just
>> replicate the test setup (e.g. create my own MockProcessContext etc.)
>> 
>> Thanks,
>> 
>> James
>> On Thu, 23 Aug 2018 at 21:03, Mike Thomsen <mikerthomsen@gmail.com<mailto:mikerthomsen@gmail.com>>
wrote:
>> 
>> James try it with a throwable like in my example
>> On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <markap14@hotmail.com<mailto:markap14@hotmail.com>>
wrote:
>> 
>> James,
>> 
>> If you are expecting the method to throw an Exception and want to verify
>> that, you should
>> just call the method directly from your unit test and catch the Exception
>> there. The TestRunner
>> expects to run the full lifecycle of the Processor.
>> 
>> Thanks
>> -Mark
>> 
>> 
>> On Aug 23, 2018, at 10:49 AM, James Srinivasan <
>> james.srinivasan@gmail.com<mailto:james.srinivasan@gmail.com>> wrote:
>> 
>> I tried that, but the problem is the exception is caught and the test
>> fails due to this:
>> 
>> try {
>> ReflectionUtils.invokeMethodsWithAnnotation(OnScheduled.class,
>> processor, context);
>> } catch (final Exception e) {
>> e.printStackTrace();
>> Assert.fail("Could not invoke methods annotated with @OnScheduled
>> annotation due to: " + e);
>> }
>> 
>> 
>> https://github.com/apache/nifi/blob/master/nifi-mock/src/main/java/org/apache/nifi/util/StandardProcessorTestRunner.java#L181
>> On Thu, 23 Aug 2018 at 15:41, Mike Thomsen <mikerthomsen@gmail.com>
>> wrote:
>> 
>> For unit tests, if you're doing this to catch a failure scenario, you
>> should be able to wrap the failing call in something like this:
>> 
>> final def msg = "Lorem ipsum..."
>> def error = null
>> try {
>>  runner.run()
>> } catch (Throwable t) {
>>  error = t
>> } finally {
>>  assertNotNull(error)
>>  assertTrue(error.cause instanceof SomeException)
>>  assertTrue(error.cause.message.contains(msg))
>> }
>> 
>> Obviously play around with the finally block, but I've had success with
>> that pattern.
>> 
>> On Thu, Aug 23, 2018 at 10:19 AM James Srinivasan <
>> james.srinivasan@gmail.com> wrote:
>> 
>> What is the best way to handle exceptions which might be thrown in my
>> @OnScheduled method? Right now, I'm logging and propagating the
>> exception which has the desired behaviour in NiFi (bulletin in GUI and
>> processor cannot be started) but when trying to add a unit test, the
>> (expected) exception is caught in StandardProcessorTestRunner.run and
>> failure asserted.
>> 
>> My actual @OnScheduled method builds a non-trivial object based on the
>> Processor's params - maybe I should be building that any time any of
>> the params change instead?
>> 
>> Many thanks,
>> 
>> James
>> 
>> 
>> 
>> 


Mime
View raw message