Errors should not print json

[Monty Taylor]

When I try to use glanceclient to upload an image and it does not work, glanceclient should not print json.

mordred@mordred-irc:~/system-config$ glance image-create --name=test-monty-ubuntu --disk-format=vhd --container-format=bare --file=ubuntu.vhd
{
    "overLimit" : {
        "code" : 413,
        "message" : "OverLimit Retry...",
        "details" : "Error Details...",
            "retryAt" : "2014-12-03T13:33:06Z"
    }
} (HTTP 413)

A nice error message would be better.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/148387

[Ian Cordasco (icordasc)]

I'm pretty sure this should also be fixed on glance's end. It would see these are errors that are being raised without an explicit response body so webob uses a pre-filled HTML response body. We should find these and eradicate them as any other client will not be expecting HTML to be returned as a response from glance-api

[Zhi Yan Liu (lzy-dev)]

From the Glance's perspective, I think the response content type should follow the content type http request provided/required, currently glance only support application/json [0] (and that's the reason why only json serializer and de-serializer was implemented there), other project followed this approach as well, e.g. cinder support json and xml request so there are two kinds of serializer and de-serializer. IMO the fault stuff need to follow this rule as well, e.g. cinder one [1], so glance could just return json content.

[0] https://github.com/openstack/glance/blob/master/glance/common/wsgi.py#L535
[2] https://github.com/openstack/cinder/blob/master/cinder/api/openstack/wsgi.py#L1260

But I see the point this bug reported, so from end user's opinion, a human readable with nice error message would be better, so I think we can just format it in client side. And more, I think keep server always return the response with requested content-type is better for non-human client - a code/application handling.

Base on my above thoughts, in sort, IMO, we could fix this in glanceclient directly instead of changing glance server side.

[Ian Cordasco (icordasc)]

So, I agree that formatting the response nicely is for glanceclient, but not returning HTML is still a bug in glance. I tagged glance in this specifically to see what we have to do to format webob exceptions as JSON when they're returned.

[Zhi Yan Liu (lzy-dev)]

So, IMO, we needn't any change on glance side to fix this defect, with the change #148387 we can just extract a well formatted human readable error message from response content, instead of printing json directly any more (that is what result this report asked I consider). Btw, the glanceclient change already covered the formation logic for text/html content of exception response.

I think we might have a few cases which may return pre-defined webob exception html content, as you mentioned case (currently I didn't try to check these cases out, and I think we can fix them be defect-driven/case-by-case), the solution in my now mind is 1). to complete missed context message/info for exception object; 2) to prepare a json serializer to organize content of webob exception/fault response based on these context message/info. However, I think we can use a dedicated change to cover this stuff.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/148387
Committed: https://git.openstack.org/cgit/openstack/python-glanceclient/commit/?id=01caf4e734fb012f0b5b19c24a8ffeaef31a06be
Submitter: Jenkins
Branch: master

commit 01caf4e734fb012f0b5b19c24a8ffeaef31a06be
Author: d34dh0r53 <email address hidden>
Date: Mon Jan 19 18:02:08 2015 -0600

    Strip json and html from error messages

    Error messages were being passed with either JSON or HTML formatting
    leading to an unpleasant user experience. This strips the JSON or HTML
    tags and presents the clean error message to the user.

    Rewrote the lispy function to be more pythonic, added test cases
    and cleaned up some pep8 violations.

    Removed commented out cruft from a previous version of this
    patch.

    Changed the HTML details from a set to a list in order to ensure
    the ordering of the exception details.

    Added code to eliminate the duplicate detail messages from the list
    and reordered the assertion string to match actual output.

    Refactored duplicate elimination code to be more readable and
    reliable.

    Some reworking of the duplication elimination loop to make it
    more readable.

    Removed unneeded conditional to filter out empty elements.

    Change-Id: I79985b3e305cb30328a3c16b025315a8e969243d
    Closes-Bug: 1398838

[Zhi Yan Liu (lzy-dev)]

@Ian, Any thoughts on my above comments? If we are on the same page I'd like to remove glance one for this defect report, and mark this to 'close'.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/218831

[OpenStack Infra (hudson-openstack)]

Change abandoned by Kirill Zaitsev (<email address hidden>) on branch: master
Review: https://review.openstack.org/218831