On Thu, Jan 8, 2009 at 9:21 AM, Simon Laws <[email protected]>wrote:
> > > On Wed, Jan 7, 2009 at 9:53 PM, Raymond Feng <[email protected]> wrote: > >> I'm fine to take the workaround as your checked in for 1.4 release. But >> we definitely need to have a more consistent story in 2.x. >> >> Thanks, >> Raymond >> >> *From:* ant elder <[email protected]> >> *Sent:* Wednesday, January 07, 2009 1:41 PM >> *To:* [email protected] >> *Subject:* Re: [1.4] Change in Exception handling behaviour for >> binding.jms >> >> But that spec extract says nothing about how to handle exceptions, the key >> bit being line 227 which explicitly leaves this out, as does the current >> OASIS versions of the spec. I did ask the spec guys about this once a while >> back (not on any mailing list so I can't provide a link) the answer was that >> as JMS doesn't have any standard definition of fault messages so the SCA JMS >> spec doesn't define anything here either and how to handle this is SCA >> runtime specific. (as a slight aside, line 227 already makes the JMS spec >> inconsistent with JAXWS as a return value is unwrapped XML not wrapped, not >> sure our current wireformat is doing that correctly). I'm missing the reason >> this could be considered a "backdoor", its not like doing this would open up >> any vulnerabilities that i can see so a backdoor to where? >> >> Anyway, what to do for 1.4? We did support non-JAXWS exceptions in >> previous releases so wouldn't it be better to continue doing that in 1.x and >> perhaps if we really do want to change this then to do it only in 2.x? >> >> ...ant >> >> On Wed, Jan 7, 2009 at 9:14 PM, Raymond Feng <[email protected]> wrote: >> >>> IMO, the wire format controls how business data and/or fault are >>> represented in the JMSMessage. The following is quoted from the OSOA JMS >>> binding spec 1.0: >>> >>> 217 1.5.2 Default Data Binding >>> 218 The default data binding behavior maps between a JMSMessage and the >>> object(s) expected by >>> 219 the component implementation. We encourage component implementers to >>> avoid exposure of >>> 220 JMS APIs to component implementations, however in the case of an >>> existing implementation that >>> 221 expects a JMSMessage, this provides for simple reuse of that as an >>> SCA component. >>> 222 The message body is mapped to the parameters or return value of the >>> target operation as >>> 223 follows: >>> 224 . If there is a single parameter or return value that is a >>> JMSMessage, then the JMSMessage is >>> 225 passed as is. >>> 226 . Otherwise, the JMSMessage must be a JMS text message containing >>> XML. >>> 227 . If there is a single parameter, or for the return value, the JMS >>> text XML payload is the XML >>> 228 serialization of that parameter according to the WSDL schema for the >>> message. >>> 229 . If there are multiple parameters, then they are encoded in XML >>> using the document wrapped >>> 230 style, according to the WSDL schema for the message. >>> >>> And the SCA java spec says: >>> >>> 1715 1.9. WSDL to Java and Java to WSDL >>> 1716 The SCA Client and Implementation Model for Java applies the WSDL to >>> Java and Java to WSDL mapping >>> 1717 rules as defined by the JAX-WS specification [4] for generating >>> remotable Java interfaces from WSDL >>> 1718 portTypes and vice versa. >>> >>> Based on this, my assumption is for text/xml wire format, we follow the >>> Java to WSDL mapping rules defined by JAXWS. It should also apply to fault >>> data. Passing exceptions as Objects in JMSMessage seems to be a backdoor and >>> it is not consistent with the wire format. It requires both client and >>> service side have the same CheckedException class. >>> >>> Thanks, >>> Raymond >>> >>> From: ant elder >>> Sent: Wednesday, January 07, 2009 12:02 PM >>> >>> To: [email protected] >>> Subject: Re: [1.4] Change in Exception handling behaviour for binding.jms >>> >>> >>> Ok, looks like we are getting to the bottom of this, those two points are >>> exactly what and why it worked in previous releases ;) >>> >>> The JMS spec doesn't define how to handle exceptions so there's nothing >>> that says the exception needs to follow the JAXWS pattern and ideally it >>> shouldn't need to. The TransportServiceInterceptor creates a >>> JMSObjectMessage to return the exception as in the previous releases thats >>> how this was able to work - it didn't delegate everything to the databinding >>> framework so the on the client side the invoke spotted the response was an >>> exception and didn't use the databinding framework to do any transformations >>> but just returned the received exception. >>> >>> As we know the exception response wont be XML so wont need a databinding >>> framework transformation could we prevent the transformation happening by >>> changing the WireFormatJMSTextXMLReferenceProvider constructor around line >>> 82 to not reset the datatype of the fault types to OMElement? >>> >>> ...ant >>> >>> >>> On Wed, Jan 7, 2009 at 7:37 PM, Raymond Feng <[email protected]> >>> wrote: >>> >>> Hi, >>> >>> 1) The org.apache.tuscany.sca.binding.jms.ExceptionService interface is >>> remotable but the CheckedException is a plain java exception and it doesn't >>> follow the JAXWS pattern to represent a fault. In this case, the >>> construction of the ChjeckedException is heuristic. >>> >>> 2) I further debugged the root cause of the null faultInfo. On the >>> following stack, it seems that WireFormatJMSTextXMLServiceInterceptor is not >>> doing its job to correctly marshal the fault data into the response JMS >>> message. Even though the wireFormat is textXML, the >>> TransportServiceInterceptor catches the exception, creates a >>> JMSObjectMessage, sets the exception as an Object and sends it back as a >>> response. The WireFormatJMSTextXMLServiceInterceptor doesn't have a chance >>> to format the fault JMSMessage as textXML. >>> >>> Thanks, >>> Raymond >>> >>> Thread [ActiveMQ Session Task] (Suspended (breakpoint at line 25 in >>> ExceptionServiceImpl)) >>> ExceptionServiceImpl.throwChecked() line: 25 >>> NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not >>> available [native method] >>> NativeMethodAccessorImpl.invoke(Object, Object[]) line: 79 >>> DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43 >>> Method.invoke(Object, Object...) line: 618 >>> JavaImplementationInvoker.invoke(Message) line: 132 >>> DataTransformationInterceptor.invoke(Message) line: 78 >>> >>> RuntimeWireInvoker.invoke(InvocationChain, Message, RuntimeWire) line: >>> 129 >>> RuntimeWireInvoker.invoke(RuntimeWire, Operation, Message) line: 104 >>> RuntimeWireInvoker.invoke(Operation, Message) line: 98 >>> RuntimeWireInvoker.invoke(Message) line: 86 >>> WireFormatJMSTextXMLServiceInterceptor.invoke(Message) line: 68 >>> OperationSelectorJMSDefaultServiceInterceptor.invoke(Message) line: 78 >>> TransportServiceInterceptor.invoke(Message) line: 77 >>> RuntimeWireImpl.invoke(Message) line: 149 >>> RRBJMSBindingListener.invokeService(Message) line: 100 >>> RRBJMSBindingListener.onMessage(Message) line: 76 >>> ActiveMQMessageConsumer.dispatch(MessageDispatch) line: 1021 >>> ActiveMQSessionExecutor.dispatch(MessageDispatch) line: 122 >>> ActiveMQSessionExecutor.iterate() line: 192 >>> PooledTaskRunner.runTask() line: 122 >>> PooledTaskRunner$1.run() line: 43 >>> >>> ThreadPoolExecutor$Worker.runTask(Runnable) line: 665 >>> ThreadPoolExecutor$Worker.run() line: 690 >>> Thread.run() line: 810 >>> >>> >>> >>> From: ant elder >>> Sent: Wednesday, January 07, 2009 10:30 AM >>> To: [email protected] >>> Subject: Re: [1.4] Change in Exception handling behaviour for binding.jms >>> >>> >>> To show what I mean i've committed some fixes in r732415 and r732416 >>> which get the testcase working, this isn't a finished fix it just to show >>> where the code is going wrong. I haven't looked at the >>> JAXWSFaultExceptionMapper code in any detail, should it be dealing with a >>> null faultInfo is is the bug that faultInfo isn't getting setup correctly? >>> >>> ...ant >>> >>> >>> On Wed, Jan 7, 2009 at 6:16 PM, ant elder <[email protected]> wrote: >>> >>> It turns out its a bit more complicated that that, i think it is the >>> FaultException that needs to be returned up the chain so that the >>> databinding can transform that back into the user exception, and there is a >>> bug in AbstractMessageProcessor either the createFaultMessage or >>> extractPayloadMessage needs to pull the FaultException out of the >>> InvocationTargetException, but then fixing that the >>> JAXWSFaultExceptionMapper gets an NPE on line 129 as faultInfo is null. >>> >>> Does that all sound plausible? >>> >>> ...ant >>> >>> >>> On Wed, Jan 7, 2009 at 5:39 PM, Raymond Feng <[email protected]> >>> wrote: >>> >>> IIRC, we intentionally return a FaultException to wrap the user exception >>> from the transformation to indicate there is a user fault. The >>> extractPayloadMessage should extract the user exception from the >>> FaultException. >>> >>> Thanks, >>> Raymond >>> >>> >>> From: ant elder >>> Sent: Wednesday, January 07, 2009 9:33 AM >>> To: Raymond Feng >>> Cc: [email protected] >>> Subject: Re: [1.4] Change in Exception handling behaviour for binding.jms >>> >>> >>> OK that helps, but also debugging there you see its too late at that >>> point as the service response message sent back from the service already >>> contains the wrong exception. Debugging on the service side this is the >>> stack from where it goes wrong: >>> >>> Thread [ActiveMQ Session Task] (Suspended) >>> DataTransformationInterceptor.invoke(Message) line: 159 >>> RuntimeWireInvoker.invoke(InvocationChain, Message, RuntimeWire) line: >>> 129 >>> RuntimeWireInvoker.invoke(RuntimeWire, Operation, Message) line: 104 >>> RuntimeWireInvoker.invoke(Operation, Message) line: 98 >>> RuntimeWireInvoker.invoke(Message) line: 86 >>> WireFormatJMSTextXMLServiceInterceptor.invoke(Message) line: 68 >>> OperationSelectorJMSDefaultServiceInterceptor.invoke(Message) line: 78 >>> TransportServiceInterceptor.invoke(Message) line: 77 >>> RuntimeWireImpl.invoke(Message) line: 149 >>> RRBJMSBindingListener.invokeService(Message) line: 100 >>> RRBJMSBindingListener.onMessage(Message) line: 76 >>> ActiveMQMessageConsumer.dispatch(MessageDispatch) line: 1021 >>> ActiveMQSessionExecutor.dispatch(MessageDispatch) line: 122 >>> ActiveMQSessionExecutor.iterate() line: 192 >>> PooledTaskRunner.runTask() line: 122 >>> PooledTaskRunner$1.run() line: 43 >>> ThreadPoolExecutor$Worker.runTask(Runnable) line: not available >>> ThreadPoolExecutor$Worker.run() line: not available >>> Thread.run() line: not available >>> >>> Before DataTransformationInterceptor.invoke(Message) line: 159 is run the >>> "result" variable contains the correct original application exception and >>> thats what we'd like gets sent back to the clinet, but after >>> transformException is called at line 160 "newResult" is a >>> org.apache.tuscany.sca.interfacedef.util.FaultException instance and that is >>> whats returned to the client. >>> >>> Is this related to the value of sourceFaultType which is: >>> >>> sourceFaultType DataTypeImpl<L> (id=265) >>> dataBinding "org.apache.axiom.om.OMElement" (id=282) >>> genericType Class<T> (java.lang.Object) (id=9) >>> logical XMLType (id=298) >>> metaDataMap null >>> physical Class<T> (java.lang.Object) (id=9) >>> >>> perhaps thats not correct with the way all the wireformats and >>> databindings are working now for JMS where really is just wanting to send >>> back the application exception in the response message? >>> >>> ...ant >>> >>> >>> >>> On Wed, Jan 7, 2009 at 4:40 PM, Raymond Feng <[email protected]> >>> wrote: >>> >>> Hi, >>> >>> I debugged the test case and found the code on the following stack is >>> problematic: >>> >>> org.osoa.sca.ServiceRuntimeException: remote service exception, see >>> nested exception >>> at >>> org.apache.tuscany.sca.binding.jms.provider.AbstractMessageProcessor.extractPayloadFromJMSMessage(AbstractMessageProcessor.java:92) >>> at >>> org.apache.tuscany.sca.binding.jms.wireformat.jmstextxml.runtime.WireFormatJMSTextXMLReferenceInterceptor.invokeResponse(WireFormatJMSTextXMLReferenceInterceptor.java:107) >>> at >>> org.apache.tuscany.sca.binding.jms.wireformat.jmstextxml.runtime.WireFormatJMSTextXMLReferenceInterceptor.invoke(WireFormatJMSTextXMLReferenceInterceptor.java:82) >>> at >>> org.apache.tuscany.sca.binding.jms.provider.RRBJMSBindingInvoker.invoke(RRBJMSBindingInvoker.java:201) >>> at >>> org.apache.tuscany.sca.core.databinding.wire.DataTransformationInterceptor.invoke(DataTransformationInterceptor.java:78) >>> at >>> org.apache.tuscany.sca.core.invocation.JDKInvocationHandler.invoke(JDKInvocationHandler.java:287) >>> at >>> org.apache.tuscany.sca.core.invocation.JDKInvocationHandler.invoke(JDKInvocationHandler.java:154) >>> at $Proxy21.throwChecked(Unknown Source) >>> at >>> org.apache.tuscany.sca.binding.jms.ExceptionServiceClient.throwChecked(ExceptionServiceClient.java:38) >>> >>> When the JMSMessage contains a fault (user exception), the >>> AbstractMessageProcessor.extractPayloadFromJMSMessage() method should not >>> throw a ServiceRuntimeException, instead it should call Message.setFaultBody >>> with the user exception extracted from the FaultException. >>> >>> public Object extractPayloadFromJMSMessage(Message msg) { >>> try { >>> if (msg.getBooleanProperty(JMSBindingConstants.FAULT_PROPERTY)) { >>> // ----------------- [rfeng] This is wrong >>> ---------------------- >>> throw new ServiceRuntimeException("remote service exception, >>> see nested exception", (Throwable)((ObjectMessage)msg).getObject()); >>> } >>> } catch (JMSException e) { >>> throw new JMSBindingException(e); >>> } >>> return extractPayload(msg); >>> } >>> >>> Thanks, >>> Raymond >>> >>> >>> From: ant elder >>> Sent: Wednesday, January 07, 2009 4:24 AM >>> To: [email protected] >>> Subject: Re: [1.4] Change in Exception handling behaviour for binding.jms >>> >>> >>> >>> >>> >>> >>> On Wed, Jan 7, 2009 at 11:12 AM, Dave Sowerby <[email protected]> >>> wrote: >>> >>> Hey guys, >>> >>> I'm trying to upgrade an application from Tuscany 1.3.2 to 1.4 RC4, >>> the application uses binding.jms and I've noticed a marked change in >>> behaviour for Exception handling - could someone tell me if this is >>> expected and if there is any means of getting a handle on the original >>> Exception? >>> >>> Basically, the service throws a UserException with a message, but on >>> the client side I get the following Exception hierarchy: >>> >>> org.osoa.sca.ServiceRuntimeException >>> -> org.osoa.sca.ServiceRuntimeException >>> -> java.lang.reflect.InvocationTargetException >>> -> org.apache.tuscany.sca.interfacedef.util.FaultException >>> >>> Where the FaultException contains the original message from the >>> UserException. >>> >>> Back in 1.3.2, for Exceptions the client would have a reconsituted >>> equivilant of the original Exception thrown. >>> >>> Any guidance would be greatly appreciated, >>> >>> Cheers, >>> >>> Dave. >>> >>> >>> That sounds like http://issues.apache.org/jira/browse/TUSCANY-2593 which >>> sadly although marked as a blocker didn't get fixed in the 1.4 RC. >>> >>> The problem is due to the way the DataTransformationInterceptor handles >>> the exception is different now so the way the JMS binding returns the >>> service exception doesn't get returned to the client as before. Tracing >>> through the code i can get it to DataTransformationInterceptor line 147 >>> where the sourceDataType gets set to >>> org.apache.tuscany.sca.interfacedef.util.FaultException which is what the >>> transform on line 159 produces so the original exception from the remote >>> service is lost. >>> >>> There's quite a few FIXME comments around here in the >>> DataTransformationInterceptor so i'm not sure how this is supposed to work, >>> you'd think it must be common across all bindings - does any one know is >>> there a fixed way a binding can return an application checked exception >>> object and have the data binding framework just return that to the client? >>> >>> ...ant >>> >> >> I still think we could get rid of the extra ServiceRuntimeException > wrapping (in the RuntimeWireInvoker) on the service side and have the > response exception be generated by the wire formatter (the code is already > there to do that). At the moment the exception is thrown directly to the > transport interceptor on the service side but does pass through the wire > formatter on the client side. > > Simon > Ok that is done too in r732679 and r732680. I've merged all this into the 1.4 branch in r732687 and will start building a 1.4 RC5, that will take a little while to get done and uploaded but should be able to start a vote later today so all going well we could get a 1.4 out next Monday or Tuesday. In the mean time the fixed module jars are at http://people.apache.org/~antelder/tuscany/TUSCANY-2593/ if anyone could test more it works ok that would be great, or any quick reviews of the changes would be great as these type of last minute changes have a tendency to go wrong... ...ant
