an xml-rpc method can't affect the http status of the response

[Michael Hudson-Doyle]

I'm not quite sure what the best approach is, but I think we definitely want to be able to return HTTP 401/403s from xml-rpc method implementations at least, and maybe other codes too. Maybe custom exceptions should be provide translated into appropriate codes, or maybe we should allow methods to just return HttpResponse objects?

[Zygmunt Krynicki (zyga)]

Yeah, I considered this too. You will notice that dashboard returns 403 via Fault.faultCode.

I'm not sure if turning this into a proper HTTP status is the right way though - 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.

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.

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.

What do you think?

[Michael Hudson-Doyle (mwhudson)]

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