Error handling in @OnScheduled

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Error handling in @OnScheduled

James Srinivasan
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
Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

Mike Thomsen
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 <
[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

James Srinivasan
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 <[hidden email]> 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 <
> [hidden email]> 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
> >
Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

Mark Payne
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 <[hidden email]> 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 <[hidden email]> 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 <
>> [hidden email]> 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
>>>

Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

Mike Thomsen
James try it with a throwable like in my example
On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <[hidden email]> 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 <
> [hidden email]> 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 <[hidden email]>
> 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 <
> >> [hidden email]> 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
> >>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

James Srinivasan
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 <[hidden email]> wrote:

>
> James try it with a throwable like in my example
> On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <[hidden email]> 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 <
> > [hidden email]> 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 <[hidden email]>
> > 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 <
> > >> [hidden email]> 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
> > >>>
> >
> >
Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

Mark Payne
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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> wrote:

James try it with a throwable like in my example
On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <[hidden email]<mailto:[hidden email]>> 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 <
[hidden email]<mailto:[hidden email]>> 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 <[hidden email]>
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 <
[hidden email]> 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




Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

James Srinivasan
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 <[hidden email]> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> wrote:
>
> James try it with a throwable like in my example
> On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <[hidden email]<mailto:[hidden email]>> 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 <
> [hidden email]<mailto:[hidden email]>> 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 <[hidden email]>
> 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 <
> [hidden email]> 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
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

Mark Payne
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> wrote:
>>
>> James try it with a throwable like in my example
>> On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <[hidden email]<mailto:[hidden email]>> 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 <
>> [hidden email]<mailto:[hidden email]>> 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 <[hidden email]>
>> 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 <
>> [hidden email]> 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
>>
>>
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: Error handling in @OnScheduled

James Srinivasan
Thanks very much, I'm now able to write a useful unit test to catch
the expected exception. Given the great support I've had from the
list, I'll start my organisation's process to contribute the code back
for this:

https://jira.apache.org/jira/browse/NIFI-5538

On Fri, 24 Aug 2018 at 16:41, Mark Payne <[hidden email]> wrote:

>
> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> wrote:
> >>
> >> James try it with a throwable like in my example
> >> On Thu, Aug 23, 2018 at 10:51 AM Mark Payne <[hidden email]<mailto:[hidden email]>> 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 <
> >> [hidden email]<mailto:[hidden email]>> 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 <[hidden email]>
> >> 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 <
> >> [hidden email]> 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
> >>
> >>
> >>
> >>
>