XFire

MessageExchange.hasFaultMessage is always true

Details

  • Type: Bug Bug
  • Status: Reopened Reopened
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.2.2
  • Fix Version/s: 1.2.7
  • Component/s: Core
  • Labels:
    None
  • Number of attachments :
    0

Description

(reposted from xfire-dev in the hope that it gets more attention here..

While debugging a surprising problem in Mule/XFireI found that
MessageExchange.hasFaultMessage() unconditionally returns true, even
though the faultMessage field is null. Since the getFaultMessage() getter
is lazy and seems to ignore the hasFault boolean I cannot use that either.
I'm stumped: why is hasFault true by default, never read or properly set
together with the faultMessage reference?
This confusing behaviour causes us quite a bit of trouble since we need to distinguish between a MEX with and without fault, yet there seems to be no way to do that correctly.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Dan Diephouse added a comment -

This should definitely be fixed! Scheduling for 1.2.3

Show
Dan Diephouse added a comment - This should definitely be fixed! Scheduling for 1.2.3
Dan Diephouse made changes -
Field Original Value New Value
Fix Version/s 1.2.3 [ 12951 ]
Hide
Permalink
Dan Diephouse added a comment -

Should be fixed now in SVN and this build:

http://snapshots.repository.codehaus.org/org/codehaus/xfire/xfire-all/1.2-SNAPSHOT/xfire-all-1.2-20061016.171227-27.jar

Just do exchange.setHasFault(false);

Show
Dan Diephouse added a comment - Should be fixed now in SVN and this build: http://snapshots.repository.codehaus.org/org/codehaus/xfire/xfire-all/1.2-SNAPSHOT/xfire-all-1.2-20061016.171227-27.jar Just do exchange.setHasFault(false);
Dan Diephouse made changes -
Resolution Fixed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]
Hide
Permalink
Holger Hoffstätte added a comment -

Cheers for excellent service
But I'm not sure how this helps me - I need to call hasFaultMessage to decide which Stream to read on an incoming MEX so why would I set the flag?
Maybe for clarification a pointer to the code where we need this (the try block):
http://fisheye.codehaus.org/browse/mule/trunk/mule/transports/xfire/src/main/java/org/mule/providers/soap/xfire/transport/MuleLocalChannel.java?r=3484#l345

It seems to me that hasFaultMessage should be false by default unless set either explicitly via setHasFaultMessage or implicitly via setFaultMessage. Maybe setHasFaultMessage should even be removed so that there's no confusion. I hope all this makes some sense?

Show
Holger Hoffstätte added a comment - Cheers for excellent service But I'm not sure how this helps me - I need to call hasFaultMessage to decide which Stream to read on an incoming MEX so why would I set the flag? Maybe for clarification a pointer to the code where we need this (the try block): http://fisheye.codehaus.org/browse/mule/trunk/mule/transports/xfire/src/main/java/org/mule/providers/soap/xfire/transport/MuleLocalChannel.java?r=3484#l345 It seems to me that hasFaultMessage should be false by default unless set either explicitly via setHasFaultMessage or implicitly via setFaultMessage. Maybe setHasFaultMessage should even be removed so that there's no confusion. I hope all this makes some sense?
Hide
Permalink
Dan Diephouse added a comment -

Oh, I see what you're trying to do now - you're trying to figure out if the response is a fault! MessageExchange.hasFaultMessage() isn't really designed for that - its designed to tell if the message exchange pattern could possibly return a fault.

How about checking getFaultMessage().getBody() != null? Or you could add a Handler into transport.getFaultHandlers() which set some property which let you know that you have a fault.

Show
Dan Diephouse added a comment - Oh, I see what you're trying to do now - you're trying to figure out if the response is a fault! MessageExchange.hasFaultMessage() isn't really designed for that - its designed to tell if the message exchange pattern could possibly return a fault. How about checking getFaultMessage().getBody() != null? Or you could add a Handler into transport.getFaultHandlers() which set some property which let you know that you have a fault.
Hide
Permalink
Holger Hoffstätte added a comment -

Dan, thanks for the explanation! I didn't write that code (I just get to debug it but seeing that I'm not the only one who misunderstood the API (especiall yin the absence of javadocs) it seems the method could have a less confusing name like canHaveFault or something. Not sure if that makes sense.

Anyway, reading the faultMessage is exactly what I wanted to avoid since it seemed to create a new Message that is not needed after all (the "lazy" getter in my original comment) - it "somehow seemed wrong".

Would it create a lot of incompatible disturbance in the sauce to rename the existing method to canHave.. and add a simple hasFault that just returns true or false, depending on faultMessage == null? As far as I understand this is exactly what would work without breaking anything. If this is not possible or does not make sense in the great scheme of things I'll try your suggestion.

Show
Holger Hoffstätte added a comment - Dan, thanks for the explanation! I didn't write that code (I just get to debug it but seeing that I'm not the only one who misunderstood the API (especiall yin the absence of javadocs) it seems the method could have a less confusing name like canHaveFault or something. Not sure if that makes sense. Anyway, reading the faultMessage is exactly what I wanted to avoid since it seemed to create a new Message that is not needed after all (the "lazy" getter in my original comment) - it "somehow seemed wrong". Would it create a lot of incompatible disturbance in the sauce to rename the existing method to canHave.. and add a simple hasFault that just returns true or false, depending on faultMessage == null? As far as I understand this is exactly what would work without breaking anything. If this is not possible or does not make sense in the great scheme of things I'll try your suggestion.
Hide
Permalink
Dan Diephouse added a comment -

I think that is probably a good suggestion... Don't have time to look at it yet today, but probably can tomorrow.

Show
Dan Diephouse added a comment - I think that is probably a good suggestion... Don't have time to look at it yet today, but probably can tomorrow.
Dan Diephouse made changes -
Resolution Fixed [ 1 ]
Status Closed [ 6 ] Reopened [ 4 ]
Dan Diephouse made changes -
Fix Version/s 1.2.4 [ 13035 ]
Fix Version/s 1.2.3 [ 12951 ]
Dan Diephouse made changes -
Fix Version/s 1.2.5 [ 13130 ]
Fix Version/s 1.2.4 [ 13035 ]
Dan Diephouse made changes -
Fix Version/s 1.2.5 [ 13130 ]
Fix Version/s 1.2.6 [ 13301 ]
Dan Diephouse made changes -
Fix Version/s 1.2.6 [ 13301 ]
Fix Version/s 1.2.7 [ 13470 ]
Hide
Permalink
Holger Hoffstätte added a comment -

2 beers (on Ross!) that this won't be fixed in 1.2.7. :-D

Show
Holger Hoffstätte added a comment - 2 beers (on Ross!) that this won't be fixed in 1.2.7. :-D
Hide
Permalink
Dan Diephouse added a comment -

sigh - I'm sorry I'm slacking. We absolutely had to cut a release.

Show
Dan Diephouse added a comment - sigh - I'm sorry I'm slacking. We absolutely had to cut a release.

People

  • Assignee:
    Dan Diephouse
    Reporter:
    Holger Hoffstätte
Vote (3)
Watch (2)

Dates

  • Created:
    Updated: