126210 - SOAP responses are sometimes null after successful

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Web Services, defect, P3)

Description

[Ray Whitmer]

Sometimes, with an unknown or perhaps a badly-behaving service, the xml http 
request layer returns a null document.  The http soap transport returns a null 
response but does not make an error out of it.  It would be clearer if a 
response-missing error were generated, even if we don't know exactly what this 
means.

This same general behavior seems to happen with both synchronous and 
asynchronous requests.

Comment 1

[Ray Whitmer]

Attachment: Test case posted by user.

Here is a test case given me by a user who modified soapcall.html which
produces the problem (at least today it does).

Comment 2

[Ray Whitmer]

*** Bug 123506 has been marked as a duplicate of this bug. ***

Comment 3

[Ray Whitmer]

I started several times to create a patch for this in the transport that checked
the response and generated an exception if there was none.  But since this is
the low-level SOAP API, I decided that it was inappropriate in this layer.  If
we can get a real error returning from the necko layers, then that is good.  But
there may well be SOAP calls which are not request / response.  The higher-level
proxy may also raise an error if the response is missing, if the lower layer
should have returned a response and did not.  But the transport layer must not
raise an exception for a missing response.

Comment 4

[Ray Whitmer]

It should be noted that one way for the lower layers to communicate that an
error has occurred would be to deliver the error event.  The SOAP transport
could respond to this and translate it into an error.  But currently the error
event is not delivered in this case, and the SOAP handler of load and error does
not check to see which happened.

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

harishd is now the SOAP owner, reassigning.

Comment 6

[John Gaunt (redfive)]

taking SOAP bugs

Comment 7

[Stef Walter]

Attachment: Patch which fixes the problem.

I've run into this problem too. According to the documentation in
nsISOAPResponseListener.idl the 'status' argument of the completion callback is
supposed to contain an error code when the call fails at a low level. 

Attached is a patch which fixes the problem fills in the 'status' argument as
is documented. This error code can be used to display an error message to the
user.

Comment 8

[Stef Walter]

My bad, I now see the comments about missing responses. The attached patch
expects a response, and generates an error when that's not the case. I'll take
that part out when I get a chance. 

IE: It should simply check the networking layers for errors when no response is
present, and if no error is present return a null response with no error code.

Comment 9

[Stef Walter]

Attachment: Patch which takes empty resposnes into account

As promised, no accounts for empty responses.

Comment 10

[Stef Walter]

make that "now accounts for empty responses". BTW, any ideas on who should I ask
for review? This is my first patch in this component.

Comment 11

[Doron Rosenberg (IBM)]

Comment on attachment 177231 [details] [diff] [review]
Patch which takes empty resposnes into account

>+
>+    if(mResponse == NULL && NS_SUCCEEDED(rv)) {
>+
>+      // Get ahold of the XMLHttpRequest object
>+      nsCOMPtr<nsIDOMEventTarget> target;
>+      rv = aEvent->GetTarget(getter_AddRefs(target));
>+      if (NS_SUCCEEDED(rv) && target) {
>+        nsCOMPtr<nsIXMLHttpRequest> xrequest = do_QueryInterface(target, &rv);
>+        if (NS_SUCCEEDED(rv) && xrequest) {
>+
>+          // Now dig out the request from the channel
>+          nsCOMPtr<nsIChannel> channel;
>+          rv = xrequest->GetChannel(getter_AddRefs(channel));
>+          if (NS_SUCCEEDED(rv) && channel) {
>+            nsCOMPtr<nsIRequest> request = do_QueryInterface(channel, &rv);
>+            if (NS_SUCCEEDED(rv) && request) 
>+              request->GetStatus(&rv);
>+          }
>+        }
>+      }
>+    }

if(mResponse == NULL && NS_SUCCEEDED(rv)) { and friends:

I think just doing a if (mResponse) check should be enough here, but I might be
wrong :)

Other than that, looks fine.  I assume you testing working web services with
this patch?

Comment 12

[Stef Walter]

Some SOAP calls are allowed to return success without a SOAP response. This
represents a one way function call without a return value. So just checking
mResponse is not enough. 

Yup works wonderfully, errors like 'host not found', or 'connection refused' are
now found in the 'status' argument to nsISOAPResponseListener::handleResponse.

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 177231 [details] [diff] [review]
Patch which takes empty resposnes into account

sr=jst

Comment 14

[Stef Walter]

Doron would you have time to commit the patch and close this bug? I don't have
CVS access. 

Comment 15

[John Gaunt (redfive)]

I'm willing to check this in, if we can find some soap tests that I can test
against. I'm using the text checked in and it doesn't appear that they exist
anymore.

Comment 16

[Mike Shaver (:shaver emeritus)]

We dropped SOAP.