FlowLogicRefFactoryImpl.createForRpc throws misleading error if no applicable FlowLogic constructors are found

[adamschoenemann]

Hi R3.

I recently encountered the error net.corda.core.flows.IllegalFlowLogicException: A FlowLogicRef cannot be constructed for FlowLogic of type com.deondigital.cordapp.flows.ExecuteOperationsFlow: due to ambiguous match against the constructors: [...]

Turns out that you get this error not when there are in fact multiple matching constructors, but when there are none.
Luckily the extra logging will tell you as much, but the exception itself is misleading.

To see why:

• The control flow starts in FlowLogicRefFactoryImpl.createForRpc
• This calls findContructor in a try/catch
  • If IllegalArgumentException is caught, then it throws the above error

Let's explore in which cases findConstructor might throw IllegalArgumentException.

• findConstructor first tries findConstructorDirectMatch.
• If this throws IllegalArgumentException, a log message is produced, but the exception is swallowed.
  • This allows us to conclude that findConstructor never throws IllegalArgumentException because of findConstructorDirectMatch

Then findConstructorCheckDefaultParams is called. Let's explore when it might throw IllegalArgumentException.
Looking at its definition, it's not immediately clear that it could throw an IllegalArgumentException, (except from a descendant caller, given that IllegalArgumentException is a very broadly used exception).
However, if you look closer you see that it can throw an IllegalFlowLogicException which is a subclass of IllegalArgumentException

Let's consider why and what happens if it throws IllegalFlowLogicException.

• It is thrown if no constructors can be found that matches the argument
• The closest catch clause is in createForRPC which proceeds to ignore the message and throw a new IllegalFlowLogicException with the message that there is an ambiguous match.

However, in fact the only way you could get an IllegalArgumentException here is if there is no match, because any ambiguous match error would have been swallowed in findConstructor.

If/when you attempt to resolve this issue, my recommendation would be to not abuse such general exception classes as IllegalArgumentExcpetion to communicate errors that are related to your domain logic.

In fact, depending on the usage patterns of the functions, you could conceivable throw away some of the exception handling here in favor of returning collections and then only throwing an exception in createForRpc depending on the size of the computed collection of constructors.

[r3jirabot]

Automatically created Jira issue: CORDA-4162

[nargas-ritu]

Logged in the backlog for an internal review