nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Srinivasan <james.sriniva...@gmail.com>
Subject Re: Error handling in @OnScheduled
Date Sun, 26 Aug 2018 20:33:47 GMT
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 <markap14@hotmail.com> 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 <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