Transaction behavior with faults/errors

[kcbabo]

Tomo and I have been discussing SWITCHYARD-1431 and I wanted to open the discussion up to a wider audience based on the scope of changes.  Here is a summary of the proposed changes:

• TransactionHandler will set active transactions to rollbackOnly when an unexpected fault/error occurs.
• Expected exceptions are documented in the service contract.
• Unexpected exceptions include:
  • RuntimeException
  • SwitchYardException
  • HandlerException wrapping an exception not documented in the service contract.
• Since components are in the best position to determine whether the exception is expected or not, each component is expected to do the following:
  • Unexpected exceptions are thrown as HandlerException
  • Expected exceptions are communicated via sendFault().
• A component involved in a message exchange can "opt out" of this automatic rollback behavior via a context property at Scope.EXCHANGE.

The following changes will be required at a minimum:

• Define a constant for the "opt out" rollback property.  A place in core/api is preferable and Exchange seems like a good spot.
• TransactionHandler needs to be updated to check fault status and the message content to determine if it's RuntimeException, SwitchYardException, or HandlerException.  If it's one of these cases and a transaction is in progress, set it to rollback.
• Components need to be updated to *not* report expected faults using HandlerException.  The bean component is doing the wrong thing for sure.  I think SOAP gateway is doing the right thing based on a cursory review.

Feedback and corrections are welcome.

[objectiser]

Sounds good to me.

Just so that I understand, currently the BPEL component will need to use the "opt out" mechanism, as it returns an expected fault as a BPELFault (derived from HandlerException)? This is the fix that Tomo previously applied.

Originally the BPELFault (and accompanying transformer) was introduced because there was an issue returning a SOAPFault via the sendFault method - do we know if this is still the case, as it would be good if the BPEL component could simply return the SOAPFault using the sendFault call?

Regards

Gary

[igarashitm]

So, we just happened to get back here - https://issues.jboss.org/browse/SWITCHYARD-437

Actually Tom said throwing HandlerException is the right thing rather than sendFault() for the decleared fault at that time. I'll take a look what happen if BPEL exchange handler use sendFault() to send SOAPFault again.

Thanks for letting me remind it

[kcbabo]

Just so that I understand, currently the BPEL component will need to use the "opt out" mechanism, as it returns an expected fault as a BPELFault (derived from HandlerException)? This is the fix that Tomo previously applied.

I think we should change this behavior based on the guidelines presented above.  That said, your understanding is correct that returning BPELFault with the opt out header would not cause a rollback.

Originally the BPELFault (and accompanying transformer) was introduced because there was an issue returning a SOAPFault via the sendFault method - do we know if this is still the case, as it would be good if the BPEL component could simply return the SOAPFault using the sendFault call?

Retuning a SOAPFault via the sendFault() method should work.  If it doesn't work, I would consider it a bug.

[kcbabo]

I'm sure that what Tom said accurately reflected the state of things at the time.  I think we need to take another look at this now that the components and runtime have matured - sendFault() is designed to accept faults, so it should be perfectly legit.  If you could have a look and identify if/where the problem lies, that would be great.

So, we just happened to get back here - https://issues.jboss.org/browse/SWITCHYARD-437

Actually Tom said throwing HandlerException is the right thing rather than sendFault() for the decleared fault at that time. I'll take a look what happen if BPEL exchange handler use sendFault() to send SOAPFault again.

[igarashitm]

OK, fixed as described in above guideline Keith suggested. Whole build (parent~release) was fine on my hand.

https://github.com/igarashitm/core/tree/SWITCHYARD-1431

https://github.com/igarashitm/components/tree/SWITCHYARD-1431

Here is the list of what I changed and some questions for implementation.camel, implementation.bpel, binding.rest:

• implementation
  • bean
    • added sendFault() logic on declared exceptions
    • added throw HandlerException() on undeclared exception for IN_ONLY as well (it used to just log for IN_ONLY and was not throwing any exception)
  • camel
    • added sendFault() logic on declared fault for IN_OUT
      • ? Is there any possibility to get false from camelExchange.isFault() in spite of getException() returning null? currently it's treated as succeeded
  • bpel
    • added sendFault() logic to send SOAPFault directly instead of throwing BPELFault. Now apparently SOAPMessageComposer accepts SOAPFault as a message content. It's extracted here - https://github.com/jboss-switchyard/components/blob/master/soap/src/main/java/org/switchyard/component/soap/composer/SOAPMessageComposer.java#L159-L173
    • ? BPELFault and its transformer is no longer needed? marked as @Deprecated for now
  • clojure
    • added sendFault() logic on declared fault
  • rules/jbpm
    • nothing to do (throws unexpected RuntimeException only)

• gateway
  • camel
    • added sendFault() logic on declared fault for IN_OUT
  • hornetq
    • nothing to do (oneway)
  • soap
    • nothing to do (already doing right)
  • http
    • nothing to do (already doing sendFault() for 4xx and 5xx HTTP status code)
  • jca
    • nothing to do (throws unexpected HandlerException only)
  • resteasy
    • ? nothing to do? (throws unexpected HandlerException only?)

Any comment would be appreciated!

Thanks,

Tomo

[objectiser]

Tomohisa igarashi wrote:

• • bpel
    • added sendFault() logic to send SOAPFault directly instead of throwing BPELFault. Now apparently SOAPMessageComposer accepts SOAPFault as a message content. It's extracted here - https://github.com/jboss-switchyard/components/blob/master/soap/src/main/java/org/switchyard/component/soap/composer/SOAPMessageComposer.java#L159-L173
    • ? BPELFault and its transformer is no longer needed? marked as @Deprecated for now

Thanks for doing this work Tomo. Once we are sure there are no issues, then I'll remove the BPELFault + transformer.

Regards

Gary