From 8a6ed0ae8b9fc41444a182f88d08ee2daeffc6c3 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 10 Jan 2014 15:10:52 -0600 Subject: [PATCH] Introduce new generation of API decorator... Add improved error handling. Introduce error code. Use new decorator with tested methods in histories API. --- lib/galaxy/exceptions/__init__.py | 47 ++++++- lib/galaxy/exceptions/error_codes.py | 33 +++++ lib/galaxy/web/__init__.py | 4 + lib/galaxy/web/framework/__init__.py | 142 ++++++++++++++++++++- lib/galaxy/webapps/galaxy/api/histories.py | 6 +- 5 files changed, 221 insertions(+), 11 deletions(-) create mode 100644 lib/galaxy/exceptions/error_codes.py diff --git a/lib/galaxy/exceptions/__init__.py b/lib/galaxy/exceptions/__init__.py index c3bd4bde319..75c9aba18bd 100644 --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -6,33 +6,66 @@ from galaxy import eggs eggs.require( "Paste" ) from paste import httpexceptions +from ..exceptions import error_codes + class MessageException( Exception ): """ - Exception to make throwing errors from deep in controllers easier + Exception to make throwing errors from deep in controllers easier. """ - def __init__( self, err_msg, type="info" ): - self.err_msg = err_msg + # status code to be set when used with API. + status_code = 400 + # Error code information embedded into API json responses. + err_code = error_codes.UNKNOWN + + def __init__( self, err_msg=None, type="info", **extra_error_info ): + self.err_msg = err_msg or self.err_code.default_error_message self.type = type + self.extra_error_info = extra_error_info + def __str__( self ): return self.err_msg + class ItemDeletionException( MessageException ): pass + class ItemAccessibilityException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_CANNOT_ACCESS_ITEM + class ItemOwnershipException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_DOES_NOT_OWN_ITEM + + +class DuplicatedSlugException( MessageException ): + status_code = 400 + err_code = error_codes.USER_SLUG_DUPLICATE + + +class ObjectAttributeInvalidException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_INVALID + + +class ObjectAttributeMissingException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_MISSING + class ActionInputError( MessageException ): def __init__( self, err_msg, type="error" ): super( ActionInputError, self ).__init__( err_msg, type ) -class ObjectNotFound( Exception ): + +class ObjectNotFound( MessageException ): """ Accessed object was not found """ - pass + status_code = 404 + err_code = error_codes.USER_OBJECT_NOT_FOUND + class ObjectInvalid( Exception ): """ Accessed object store ID is invalid """ diff --git a/lib/galaxy/exceptions/error_codes.py b/lib/galaxy/exceptions/error_codes.py new file mode 100644 index 00000000000..4fe658e6754 --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.py @@ -0,0 +1,33 @@ +# Error codes are provided as a convience to Galaxy API clients, but at this +# time they do represent part of the more stable interface. They can change +# without warning between releases. +UNKNOWN_ERROR_MESSAGE = "Unknown error occurred while processing request." + + +class ErrorCode( object ): + + def __init__( self, code, default_error_message ): + self.code = code + self.default_error_message = default_error_message or UNKNOWN_ERROR_MESSAGE + + def __str__( self ): + return str( self.default_error_message ) + + def __int__( self ): + return int( self.code ) + +# TODO: Guidelines for error message langauge? +UNKNOWN = ErrorCode(0, UNKNOWN_ERROR_MESSAGE) + +USER_CANNOT_RUN_AS = ErrorCode(400001, "User does not have permissions to run jobs as another user.") +USER_INVALID_RUN_AS = ErrorCode(400002, "Invalid run_as request - run_as user does not exist.") +USER_INVALID_JSON = ErrorCode(400003, "Your request did not appear to be valid JSON, please consult the API documentation.") +USER_OBJECT_ATTRIBUTE_INVALID = ErrorCode(400004, "Attempted to create or update object with invalid attribute value.") +USER_OBJECT_ATTRIBUTE_MISSING = ErrorCode(400005, "Attempted to create object without required attribute.") +USER_SLUG_DUPLICATE = ErrorCode(400006, "Slug must be unique per user.") + +USER_NO_API_KEY = ErrorCode(403001, "API Authentication Required for this request") +USER_CANNOT_ACCESS_ITEM = ErrorCode(403002, "User cannot access specified item.") +USER_DOES_NOT_OWN_ITEM = ErrorCode(403003, "User does not own specified item.") + +USER_OBJECT_NOT_FOUND = ErrorCode(404001, "No such object not found.") diff --git a/lib/galaxy/web/__init__.py b/lib/galaxy/web/__init__.py index 107c5002d23..70b016c53e4 100644 --- a/lib/galaxy/web/__init__.py +++ b/lib/galaxy/web/__init__.py @@ -15,3 +15,7 @@ from framework import expose_api_anonymous from framework import expose_api_raw from framework import expose_api_raw_anonymous from framework.base import httpexceptions + +# TODO: Drop and make these the default. +from framework import _future_expose_api +from framework import _future_expose_api_anonymous diff --git a/lib/galaxy/web/framework/__init__.py b/lib/galaxy/web/framework/__init__.py index ab1b48bc296..eecdfb9f83c 100644 --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -10,9 +10,9 @@ import random import socket import string import time - -from functools import wraps +from traceback import format_exc from Cookie import CookieError +from functools import wraps pkg_resources.require( "Cheetah" ) from Cheetah.Template import Template @@ -23,6 +23,7 @@ import helpers from galaxy import util from galaxy.exceptions import MessageException +from galaxy.exceptions import error_codes from galaxy.util import asbool from galaxy.util import safe_str_cmp from galaxy.util.backports.importlib import import_module @@ -212,6 +213,143 @@ def expose_api( func, to_json=True, user_required=True ): decorator.exposed = True return decorator +API_RESPONSE_CONTENT_TYPE = "application/json" + + +def __api_error_message( trans, **kwds ): + exception = kwds.get( "exception", None ) + if exception: + # If we are passed a MessageException use err_msg. + default_error_code = getattr( exception, "err_code", error_codes.UNKNOWN ) + default_error_message = getattr( exception, "err_msg", default_error_code.default_error_message ) + extra_error_info = getattr( exception, 'extra_error_info', {} ) + if not isinstance( extra_error_info, dict ): + extra_error_info = {} + else: + default_error_message = "Error processing API request." + default_error_code = error_codes.UNKNOWN + extra_error_info = {} + traceback_string = kwds.get( "traceback", "No traceback available." ) + err_msg = kwds.get( "err_msg", default_error_message ) + error_code_object = kwds.get( "err_code", default_error_code ) + try: + error_code = error_code_object.code + except AttributeError: + # Some sort of bad error code sent in, logic failure on part of + # Galaxy developer. + error_code = error_codes.UNKNOWN.code + # Would prefer the terminology of error_code and error_message, but + # err_msg used a good number of places already. Might as well not change + # it? + error_response = dict( err_msg=err_msg, err_code=error_code, **extra_error_info ) + if trans.debug: # TODO: Should admins get to see traceback as well? + error_response[ "traceback" ] = traceback_string + return error_response + + +def __api_error_response( trans, **kwds ): + error_dict = __api_error_message( trans, **kwds ) + exception = kwds.get( "exception", None ) + # If we are given an status code directly - use it - otherwise check + # the exception for a status_code attribute. + if "status_code" in kwds: + status_code = int( kwds.get( "status_code" ) ) + elif hasattr( exception, "status_code" ): + status_code = int( exception.status_code ) + else: + status_code = 500 + response = trans.response + if not response.status or str(response.status).startswith("20"): + # Unset status code appears to be string '200 OK', if anything + # non-success (i.e. not 200 or 201) has been set, do not override + # underlying controller. + response.status = status_code + return to_json_string( error_dict ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api_anonymous( func, to_json=True ): + """ + Expose this function via the API but don't require a set user. + """ + return _future_expose_api( func, to_json=to_json, user_required=False ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api( func, to_json=True, user_required=True ): + """ + Expose this function via the API. + """ + @wraps(func) + def decorator( self, trans, *args, **kwargs ): + if trans.error_message: + # TODO: Document this branch, when can this happen, + # I don't understand it. + return __api_error_response( trans, err_msg=trans.error_message ) + if user_required and trans.anonymous: + error_code = error_codes.USER_NO_API_KEY + # Use error codes default error message. + return __api_error_response( trans, err_code=error_code, status_code=403 ) + if trans.request.body: + try: + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) + except ValueError: + error_code = error_codes.USER_INVALID_JSON + return __api_error_response( trans, status_code=400, err_code=error_code ) + + trans.response.set_content_type( API_RESPONSE_CONTENT_TYPE ) + # send 'do not cache' headers to handle IE's caching of ajax get responses + trans.response.headers[ 'Cache-Control' ] = "max-age=0,no-cache,no-store" + # TODO: Refactor next block out into a helper procedure. + # Perform api_run_as processing, possibly changing identity + if 'payload' in kwargs and 'run_as' in kwargs['payload']: + if not trans.user_can_do_run_as(): + error_code = error_codes.USER_CANNOT_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=403 ) + try: + decoded_user_id = trans.security.decode_id( kwargs['payload']['run_as'] ) + except TypeError: + error_message = "Malformed user id ( %s ) specified, unable to decode." % str( kwargs['payload']['run_as'] ) + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, err_msg=error_message, status_code=400) + try: + user = trans.sa_session.query( trans.app.model.User ).get( decoded_user_id ) + trans.api_inherit_admin = trans.user_is_admin() + trans.set_user(user) + except: + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=400 ) + try: + rval = func( self, trans, *args, **kwargs) + if to_json and trans.debug: + rval = to_json_string( rval, indent=4, sort_keys=True ) + elif to_json: + rval = to_json_string( rval ) + return rval + except MessageException as e: + traceback_string = format_exc() + return __api_error_response( trans, exception=e, traceback=traceback_string ) + except paste.httpexceptions.HTTPException: + # TODO: Allow to pass or format for the API??? + raise # handled + except Exception as e: + traceback_string = format_exc() + error_message = 'Uncaught exception in exposed API method:' + log.exception( error_message ) + return __api_error_response( + trans, + status_code=500, + exception=e, + traceback=traceback_string, + err_msg=error_message, + err_code=error_codes.UNKNOWN + ) + if not hasattr(func, '_orig'): + decorator._orig = func + decorator.exposed = True + return decorator + + def require_admin( func ): @wraps(func) def decorator( self, trans, *args, **kwargs ): diff --git a/lib/galaxy/webapps/galaxy/api/histories.py b/lib/galaxy/webapps/galaxy/api/histories.py index 6f58b702f62..254dab8c693 100644 --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -9,6 +9,8 @@ pkg_resources.require( "Paste" ) from paste.httpexceptions import HTTPBadRequest, HTTPForbidden, HTTPInternalServerError, HTTPException from galaxy import web +from galaxy.web import _future_expose_api as expose_api +from galaxy.web import _future_expose_api_anonymous as expose_api_anonymous from galaxy.util import string_as_bool, restore_text from galaxy.util.sanitize_html import sanitize_html from galaxy.web.base.controller import BaseAPIController, UsesHistoryMixin, UsesTagsMixin @@ -20,7 +22,7 @@ log = logging.getLogger( __name__ ) class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): - @web.expose_api_anonymous + @expose_api_anonymous def index( self, trans, deleted='False', **kwd ): """ index( trans, deleted='False' ) @@ -152,7 +154,7 @@ class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): return history_data - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( trans, payload )