XFire
  1. XFire
  2. XFIRE-710

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

        Hide
        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
        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
        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
        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
        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
        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
        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
        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
          • Votes:
            3 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated: