OpenStack Image Registry and Delivery Service (Glance)

Glance Client in _do_request does not handle 503 (or other HTTP exceptions) in a manageable way.

Reported by John Postlethwait on 2012-03-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Medium
Jay Pipes

Bug Description

On line 514 (and 512) of glance/common/client.py unknown HTTP status codes are raised with an "Unknown error occurred!" using the base python Exception class. This is not a manageable or catchable way of dealing with glance client errors resulting from HTTP error codes...

Any exceptions raised from doing a request should be some sort of HttpException class or something (anything) other than the base Exception class. This will allow consumers of this client to appropriately catch and deal with these exceptions from Glance in a manner that is specific to the sort of error that occurred. HttpException is a vastly different and less severe exception type than just plain-ol' Exception, which is very vague...

Eoghan Glynn (eglynn) on 2012-03-12
Changed in glance:
assignee: nobody → Eoghan Glynn (eglynn)

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

Changed in glance:
status: New → In Progress
Eoghan Glynn (eglynn) on 2012-03-15
Changed in glance:
milestone: none → essex-rc1
Jay Pipes (jaypipes) on 2012-03-15
Changed in glance:
importance: Undecided → Medium

Reviewed: https://review.openstack.org/5366
Committed: http://github.com/openstack/glance/commit/c5937221b50614b36d46e77827ab5ad1fd815bd6
Submitter: Jenkins
Branch: master

commit c5937221b50614b36d46e77827ab5ad1fd815bd6
Author: Eoghan Glynn <email address hidden>
Date: Wed Mar 14 11:24:59 2012 +0000

    Well-formed exception types for 413 & 503.

    Fixes bug 952618

    Raise well-formed exception types when 413 or 503
    status returned to client.

    Change-Id: I26118ff7ec7ba968b303435287d0eb3ff4bd443f

Changed in glance:
status: In Progress → Fix Committed

This is a partial fix. While there are manageable error types for new types of errors in this fix, this still has "raise Exception" in the code. All errors from this, especially dealing with HTTP communication, should be some type of Glance exception, not just a core "Exception"

Changed in glance:
status: Fix Committed → In Progress
Eoghan Glynn (eglynn) wrote :

John, can you enumerate the actual status codes you're concerned about?

I didn't think there was much point in polluting the code with new exception types and extra conditional branches for status codes that will never be returned in practice, but if I have missed some, please let me know.

In practice any time an exception is raised it should be done so using, at least, the base GlanceException class as a means to catch them and do something with them outside of this code.

The code here would cast any unexpected HTTP status code as a base Exception and is thusly difficult to deal with. We encountered this in Horizon the other day with the 503's and ended up having to deal with the exception based not on the exception class itself, but on the actual string error message which is kludgy and difficult.

I'm not proposing extra conditionals, but using a different error class than Exception is really what this bug is about. You shouldn't need to create a new error class to achieve this, using GlanceException would be much more preferable and manageable than using Exception.

Eoghan Glynn (eglynn) wrote :

John,

The generic GlanceException wouldn't give you much more information, as it stringifies merely as "An unknown exception occurred".

So I've added a new exception type which at least captures the unexpected status code and response body.

Please pitch in to the code review at https://review.openstack.org/5415 if you've any further requirements, so we can this closed off for Essex RC1.

Cheers,
Eoghan

Changed in glance:
assignee: Eoghan Glynn (eglynn) → Jay Pipes (jaypipes)

Reviewed: https://review.openstack.org/5415
Committed: http://github.com/openstack/glance/commit/29b035bcc53a514028e60076692e31a0772b8d5c
Submitter: Jenkins
Branch: master

commit 29b035bcc53a514028e60076692e31a0772b8d5c
Author: Eoghan Glynn <email address hidden>
Date: Thu Mar 15 21:04:56 2012 +0000

    Add new UnexpectedStatus exception.

    Raise UnexpectedStatus as opposed to a generic Exception when
    an unexpected HTTP status is seen by the glance client.

    Further fix for bug 952618

    Change-Id: I222c7553e1a687aec0f6dde8215e4400ea6be2cb

Changed in glance:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2012-03-21
Changed in glance:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2012-04-05
Changed in glance:
milestone: essex-rc1 → 2012.1
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers