Comment 2 for bug 796386

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote : Re: [Bug 796386] Re: an xml-rpc method can't affect the http status of the response

On Mon, 13 Jun 2011 08:58:59 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Yeah, I considered this too. You will notice that dashboard returns 403
> via Fault.faultCode.

I hadn't noticed that.

> I'm not sure if turning this into a proper HTTP status is the right way
> though

I think you might be right, on further reflection. I guess you could
say: "If you want REST, it's over there -------------->"

> - I see the following possible combinations:
>
> 1) The server raises an appropriate HttpStatus exception without
> producing either Fault or response objects. The client library
> (probably) reports ProtocolError and provides a way for the caller to
> inspect the status. The biggest downside is that it's really not xml-
> rpc-ish. There would have to be two code paths for each method that
> might return those.

Yeah. This doesn't seem great. The client has to be prepared to handle
40x errors anyway though (for example, using the wrong token).

> 2) The server returns a normal response / raises Fault but uses some
> out-of-band data to convey the status code of the request. The client
> (we should check this) follows the normal code path where the
> response/fault is returned as usual but also provides an out-of-band
> channel for inspecting the response status code. The biggest downside
> here is that I don't see anyone using the response code as you already
> have either the Fault (with similar/identical code) or the response
> which you were after.

It's the evening and maybe that's why I'm not thinking straight and
failing to understand this, but this seems over-engineered to no great
benefit.

> 3) The server raises an appropriate HttpStatus exception and the library
> on both sides cooperate to convert this into a standardized Fault. The
> client has only one code path - one that follows Fault processing. There
> is no custom guessing. The biggest advantage is that the caller can now
> assume some semantics of Fault.faultCode between 400 and 500.

Yeah, maybe this makes the most sense.

> What do you think?

Well, I guess it's instructive to think about what the client is going
to do with these errors. I think for the lava-tool use case, all it can
reasonably do is print out the error and exit (although, you could
imagine responding to a 401 with a prompt asking for a token). So maybe
the fact that we can stuff things into the Fault's faultString attribute
is actually the most useful property.

I guess I'm happy to have this bug marked invalid.

Cheers,
mwh