Better error messages when using the API, index and registry

endpoints/api.py

@@ -48,6 +48,38 @@ def csrf_protect():
       abort(403)
 
 
+@api.errorhandler(404)
+def endpoint_not_found(e):
+  return jsonify({
+    'error_code': 404,
+    'message': 'Resource not found'
+  })
+
+
+@api.errorhandler(403)
+def endpoint_forbidden(e):
+  return jsonify({
+    'error_code': 403,
+    'message': 'Permission Denied'
+  })
+
+
+@api.errorhandler(400)
+def endpoint_invalid_request(e):
+  return jsonify({
+    'error_code': 400,
+    'message': 'Invalid Request'
+  })
+
+
+def request_error(exception = None, **kwargs):
+  data = kwargs.copy()
+  if exception:
+    data['message'] = ex.message
+
+  return make_response(jsonify(data), 400)
+
+
 def get_route_data():
   global route_data
   if route_data:
@@ -132,7 +164,7 @@ def discovery():
 @api.route('/')
 @internal_api_call
 def welcome():
-  return make_response('welcome', 200)
+  return jsonify({'version': '0.5'})
 
 
 @api.route('/plans/')
@@ -222,20 +254,12 @@ def convert_user_to_organization():
   # Ensure that the new admin user is the not user being converted.
   admin_username = convert_data['adminUser']
   if admin_username == user.username:
-    error_resp = jsonify({
+    return request_error(reason = 'invaliduser', message = 'The admin user is not valid')
-      'reason': 'invaliduser'
-    })
-    error_resp.status_code = 400
-    return error_resp
 
   # Ensure that the sign in credentials work.
   admin_password = convert_data['adminPassword']
   if not model.verify_user(admin_username, admin_password):
-    error_resp = jsonify({
+    return request_error(reason = 'invaliduser', message = 'The admin user credentials are not valid')
-      'reason': 'invaliduser'
-    })
-    error_resp.status_code = 400
-    return error_resp
 
   # Subscribe the organization to the new plan.
   plan = convert_data['plan']
@@ -271,22 +295,14 @@ def change_user_details():
       new_email = user_data['email']
       if model.find_user_by_email(new_email):
         # Email already used.
-        error_resp = jsonify({
+        return request_error(message = 'E-mail address already used')
-            'message': 'E-mail address already used'
-            })
-        error_resp.status_code = 400
-        return error_resp
       
       logger.debug('Sending email to change email address for user: %s', user.username)
       code = model.create_confirm_email_code(user, new_email=new_email)
       send_change_email(user.username, user_data['email'], code.code)
       
   except model.InvalidPasswordException, ex:
-    error_resp = jsonify({
+    return request_error(exception = ex)
-      'message': ex.message,
-    })
-    error_resp.status_code = 400
-    return error_resp
 
   return jsonify(user_view(user))
 
@@ -298,11 +314,7 @@ def create_new_user():
 
   existing_user = model.get_user(user_data['username'])
   if existing_user:
-    error_resp = jsonify({
+    return request_error(message = 'The username already exists')
-      'message': 'The username already exists'
-    })
-    error_resp.status_code = 400
-    return error_resp
 
   try:
     new_user = model.create_user(user_data['username'], user_data['password'],
@@ -311,11 +323,7 @@ def create_new_user():
     send_confirmation_email(new_user.username, new_user.email, code.code)
     return make_response('Created', 201)
   except model.DataModelException as ex:
-    error_resp = jsonify({
+    return request_error(exception = ex)
-      'message': ex.message,
-    })
-    error_resp.status_code = 400
-    return error_resp
 
 
 @api.route('/signin', methods=['POST'])
@@ -336,7 +344,7 @@ def conduct_signin(username_or_email, password):
   verified = model.verify_user(username_or_email, password)
   if verified:
     if common_login(verified):
-      return make_response('Success', 200)
+      return jsonify({'success': True})
     else:
       needs_email_verification = True
 
@@ -357,7 +365,7 @@ def conduct_signin(username_or_email, password):
 def logout():
   logout_user()
   identity_changed.send(app, identity=AnonymousIdentity())
-  return make_response('Success', 200)
+  return jsonify({'success': True})
 
 
 @api.route("/recovery", methods=['POST'])
@@ -459,22 +467,14 @@ def create_organization():
     pass
 
   if existing:
-    error_resp = jsonify({
+    return request_error(message = 'A user or organization with this name already exists')
-      'message': 'A user or organization with this name already exists'
-    })
-    error_resp.status_code = 400
-    return error_resp
 
   try:
     model.create_organization(org_data['name'], org_data['email'],
                               current_user.db_user())
     return make_response('Created', 201)
   except model.DataModelException as ex:
-    error_resp = jsonify({
+    return request_error(exception = ex)
-      'message': ex.message,
-    })
-    error_resp.status_code = 400
-    return error_resp
 
 
 def org_view(o, teams):
@@ -529,12 +529,7 @@ def change_organization_details(orgname):
     if 'email' in org_data and org_data['email'] != org.email:
       new_email = org_data['email']
       if model.find_user_by_email(new_email):
-        # Email already used.
+        return request_error(message = 'E-mail address already used')
-        error_resp = jsonify({
-            'message': 'E-mail address already used'
-            })
-        error_resp.status_code = 400
-        return error_resp
       
       logger.debug('Changing email address for organization: %s', org.username)
       model.update_email(org, new_email)
@@ -637,10 +632,10 @@ def create_organization_prototype_permission(orgname):
                      if delegate_teamname else None)
 
     if activating_username and not activating_user:
-      abort(404)
+      return request_error(message = 'Unknown activating user')
 
     if not delegate_user and not delegate_team:
-      abort(400)
+      return request_error(message = 'Missing delagate user or team')
 
     role_name = details['role']
 
@@ -898,7 +893,7 @@ def update_organization_team_member(orgname, teamname, membername):
     # Find the user.
     user = model.get_user(membername)
     if not user:
-      abort(400)
+      return request_error(message = 'Unknown user')
       
     # Add the user to the team.
     model.add_user_to_team(user, team)
@@ -939,7 +934,7 @@ def create_repo():
 
     existing = model.get_repository(namespace_name, repository_name)
     if existing:
-      return make_response('Repository already exists', 400)
+      return request_error(message = 'Repository already exists')
 
     visibility = req['visibility']
 
@@ -1542,11 +1537,7 @@ def change_user_permissions(namespace, repository, username):
       # This repository is not part of an organization
       pass
     except model.DataModelException as ex:
-      error_resp = jsonify({
+      return request_error(exception = ex)
-          'message': ex.message,
-          })
-      error_resp.status_code = 400
-      return error_resp
       
     log_action('change_repo_permission', namespace,
                {'username': username, 'repo': repository,
@@ -1599,11 +1590,7 @@ def delete_user_permissions(namespace, repository, username):
     try:
       model.delete_user_permission(username, namespace, repository)
     except model.DataModelException as ex:
-      error_resp = jsonify({
+      return request_error(exception = ex)
-          'message': ex.message,
-          })
-      error_resp.status_code = 400
-      return error_resp
 
     log_action('delete_repo_permission', namespace,
                {'username': username, 'repo': repository},
@@ -1859,7 +1846,7 @@ def subscribe(user, plan, token, require_business_plan):
                                   plan_found['price'] == 0):
     logger.warning('Business attempting to subscribe to personal plan: %s',
                    user.username)
-    abort(400)
+    return request_error(message = 'No matching plan found')
 
   private_repos = model.get_private_repo_count(user.username)
 
@@ -2089,7 +2076,7 @@ def delete_user_robot(robot_shortname):
   parent = current_user.db_user()
   model.delete_robot(format_robot_username(parent.username, robot_shortname))
   log_action('delete_robot', parent.username, {'robot': robot_shortname})
-  return make_response('No Content', 204)
+  return make_response('Deleted', 204)
 
 
 @api.route('/organization/<orgname>/robots/<robot_shortname>',
@@ -2101,7 +2088,7 @@ def delete_org_robot(orgname, robot_shortname):
   if permission.can():
     model.delete_robot(format_robot_username(orgname, robot_shortname))
     log_action('delete_robot', orgname, {'robot': robot_shortname})
-    return make_response('No Content', 204)
+    return make_response('Deleted', 204)
 
   abort(403)
 

endpoints/index.py

@@ -52,6 +52,19 @@ def generate_headers(role='read'):
   return decorator_method
 
 
+@index.errorhandler(404)
+def fallback_not_found(e):
+  return make_response('Not Found', 404)
+
+@index.errorhandler(403)
+def fallback_forbidden(e):
+  return make_response('Forbidden', 403)
+
+@index.errorhandler(400)
+def fallback_invalid_request(e):
+  return make_response('Invalid Request', 400)
+
+
 @index.route('/users', methods=['POST'])
 @index.route('/users/', methods=['POST'])
 def create_user():

endpoints/registry.py

@@ -1,7 +1,7 @@
 import logging
 import json
 
-from flask import (make_response, request, session, Response, abort,
+from flask import (make_response, request, session, Response, abort as flask_abort,
                    redirect, Blueprint)
 from functools import wraps
 from datetime import datetime
@@ -10,7 +10,7 @@ from time import time
 from data.queue import image_diff_queue
 
 from app import app
-from auth.auth import process_auth, extract_namespace_repo_from_session
+from auth.auth import process_auth, extract_namespace_repo_from_session, get_authenticated_user, get_validated_token
 from util import checksums, changes
 from auth.permissions import (ReadRepositoryPermission,
                               ModifyRepositoryPermission)
@@ -21,6 +21,38 @@ registry = Blueprint('registry', __name__)
 store = app.config['STORAGE']
 logger = logging.getLogger(__name__)
 
+DEFAULT_MESSAGE = {}
+DEFAULT_MESSAGE[400] = 'Invalid Request'
+DEFAULT_MESSAGE[403] = 'Forbidden'
+DEFAULT_MESSAGE[404] = 'Not Found'
+
+@registry.errorhandler(404)
+def fallback_not_found(e):
+  return make_response('Not Found', 404)
+
+@registry.errorhandler(403)
+def fallback_forbidden(e):
+  return make_response('Forbidden', 403)
+
+@registry.errorhandler(400)
+def fallback_invalid_request(e):
+  return make_response('Invalid Request', 400)
+
+def abort(status_code, message=None, **kwargs):
+  if status_code == 403 and not message:
+    # Create a default error message for auth failure.
+    message = 'Forbidden. '
+    auth_user = get_authenticated_user()
+    auth_token = get_validated_token()
+    if auth_user:
+      message = message + 'Current user: ' + auth_user
+    elif auth_token:
+      message = message + 'Current token: ' + auth_token
+
+  message = message % kwargs if message else DEFAULT_MESSAGE[status_code] 
+  log.error('Error %s: %s' % (status_code, message))
+  flask_abort(make_response(HTTPException(message), status_code, headers))
+  
 
 class SocketReader(object):
   def __init__(self, fp):
@@ -45,8 +77,8 @@ def require_completion(f):
   def wrapper(namespace, repository, *args, **kwargs):
     if store.exists(store.image_mark_path(namespace, repository,
                                           kwargs['image_id'])):
-      logger.warning('Image is already being uploaded: %s', kwargs['image_id'])
+      abort(400, 'Image %(image_id)s is being uploaded, retry later', image_id=kwargs['image_id'])
-      abort(400)  # 'Image is being uploaded, retry later')
+
     return f(namespace, repository, *args, **kwargs)
   return wrapper
 
@@ -90,8 +122,7 @@ def get_image_layer(namespace, repository, image_id, headers):
     try:
       return Response(store.stream_read(path), headers=headers)
     except IOError:
-      logger.warning('Image not found: %s', image_id)
+      abort(404, 'Image %(image_id)s not found', image_id=image_id)
-      abort(404)  # 'Image not found', 404)
   
   abort(403)
 
@@ -108,11 +139,11 @@ def put_image_layer(namespace, repository, image_id):
     json_data = store.get_content(store.image_json_path(namespace, repository,
                                                         image_id))
   except IOError:
-    abort(404)  # 'Image not found', 404)
+    abort(404, 'Image not found')
   layer_path = store.image_layer_path(namespace, repository, image_id)
   mark_path = store.image_mark_path(namespace, repository, image_id)
   if store.exists(layer_path) and not store.exists(mark_path):
-    abort(409)  # 'Image already exists', 409)
+    abort(409, 'Image already exists')
   input_stream = request.stream
   if request.headers.get('transfer-encoding') == 'chunked':
     # Careful, might work only with WSGI servers supporting chunked
@@ -151,7 +182,8 @@ def put_image_layer(namespace, repository, image_id):
   # We check if the checksums provided matches one the one we computed
   if checksum not in csums:
     logger.warning('put_image_layer: Wrong checksum')
-    abort(400)  # 'Checksum mismatch, ignoring the layer')
+    abort(400, 'Checksum mismatch; ignoring the layer')
+
   # Checksum is ok, we remove the marker
   store.remove(mark_path)
 
@@ -177,24 +209,28 @@ def put_image_checksum(namespace, repository, image_id):
 
   checksum = request.headers.get('X-Docker-Checksum')
   if not checksum:
-    logger.warning('Missing Image\'s checksum: %s', image_id)
+    abort(400, "Missing checksum for image %(image_id)s", image_id=image_id)
-    abort(400)  # 'Missing Image\'s checksum')
+
   if not session.get('checksum'):
-    logger.warning('Checksum not found in Cookie for image: %s', image_id)
+    abort(400, 'Checksum not found in Cookie for image %(imaage_id)s', image_id=image_id)
-    abort(400)  # 'Checksum not found in Cookie')
+
   if not store.exists(store.image_json_path(namespace, repository, image_id)):
-    abort(404)  # 'Image not found', 404)
+    abort(404, 'Image not found: %(image_id)s', image_id=image_id)
+
   mark_path = store.image_mark_path(namespace, repository, image_id)
   if not store.exists(mark_path):
-    abort(409)  # 'Cannot set this image checksum', 409)
+    abort(409, 'Cannot set checksum for image %(image_id)s', image_id=image_id)
+
   err = store_checksum(namespace, repository, image_id, checksum)
   if err:
-    abort(err)
+    abort(400, err)
+
   if checksum not in session.get('checksum', []):
     logger.debug('session checksums: %s' % session.get('checksum', []))
     logger.debug('client supplied checksum: %s' % checksum)
     logger.debug('put_image_layer: Wrong checksum')
-    abort(400)  # 'Checksum mismatch')
+    abort(400, 'Checksum mismatch for image: %(image_id)s', image_id=image_id)
+
   # Checksum is ok, we remove the marker
   store.remove(mark_path)
 
@@ -225,16 +261,19 @@ def get_image_json(namespace, repository, image_id, headers):
     data = store.get_content(store.image_json_path(namespace, repository,
                                                    image_id))
   except IOError:
-    abort(404)  # 'Image not found', 404)
+    abort(404, 'Image %(image_id)%s not found', image_id=image_id)
+
   try:
     size = store.get_size(store.image_layer_path(namespace, repository,
                                                  image_id))
     headers['X-Docker-Size'] = str(size)
   except OSError:
     pass
+
   checksum_path = store.image_checksum_path(namespace, repository, image_id)
   if store.exists(checksum_path):
     headers['X-Docker-Checksum'] = store.get_content(checksum_path)
+
   response = make_response(data, 200)
   response.headers.extend(headers)
   return response
@@ -255,7 +294,8 @@ def get_image_ancestry(namespace, repository, image_id, headers):
     data = store.get_content(store.image_ancestry_path(namespace, repository,
                                                        image_id))
   except IOError:
-    abort(404)  # 'Image not found', 404)
+    abort(404, 'Image %(image_id)s not found', image_id=image_id)
+
   response = make_response(json.dumps(json.loads(data)), 200)
   response.headers.extend(headers)
   return response
@@ -280,6 +320,7 @@ def store_checksum(namespace, repository, image_id, checksum):
   checksum_parts = checksum.split(':')
   if len(checksum_parts) != 2:
     return 'Invalid checksum format'
+
   # We store the checksum
   checksum_path = store.image_checksum_path(namespace, repository, image_id)
   store.put_content(checksum_path, checksum)
@@ -298,36 +339,35 @@ def put_image_json(namespace, repository, image_id):
   except json.JSONDecodeError:
     pass
   if not data or not isinstance(data, dict):
-    logger.warning('Invalid JSON for image: %s json: %s', image_id,
+    abort(400, 'Invalid JSON for image: %(image_id)s\nJSON: %(json)s', image_id=image_id, json=request.data)
-                   request.data)
+
-    abort(400)  # 'Invalid JSON')
   if 'id' not in data:
-    logger.warning('Missing key `id\' in JSON for image: %s', image_id)
+    abort(400, 'Missing key `id` in JSON for image: %(image_id)s', image_id=image_id)
-    abort(400)  # 'Missing key `id\' in JSON')
+
   # Read the checksum
   checksum = request.headers.get('X-Docker-Checksum')
   if checksum:
     # Storing the checksum is optional at this stage
     err = store_checksum(namespace, repository, image_id, checksum)
     if err:
-      abort(err)
+      abort(400, err)
+
   else:
     # We cleanup any old checksum in case it's a retry after a fail
     store.remove(store.image_checksum_path(namespace, repository, image_id))
   if image_id != data['id']:
-    logger.warning('JSON data contains invalid id for image: %s', image_id)
+    abort(400, 'JSON data contains invalid id for image: %(image_id)s', image_id=image_id)
-    abort(400)  # 'JSON data contains invalid id')
+
   parent_id = data.get('parent')
-  if parent_id and not store.exists(store.image_json_path(namespace,
+  if parent_id and not store.exists(store.image_json_path(namespace, repository, parent_id)):
-                                                          repository,
+    abort(400, 'Image %(image_id)s depends on non existing parent image %(parent_id)s',
-                                                          data['parent'])):
+          image_id=image_id, parent_id=parent_id)
-    logger.warning('Image depends on a non existing parent image: %s',
+
-                   image_id)
-    abort(400)  # 'Image depends on a non existing parent')
   json_path = store.image_json_path(namespace, repository, image_id)
   mark_path = store.image_mark_path(namespace, repository, image_id)
   if store.exists(json_path) and not store.exists(mark_path):
-    abort(409)  # 'Image already exists', 409)
+    abort(409, 'Image %(image_id)s already exists', image_id=image_id)
+
   # If we reach that point, it means that this is a new image or a retry
   # on a failed push
   # save the metadata

static/js/controllers.js

@@ -1242,7 +1242,7 @@ function NewRepoCtrl($scope, $location, $http, $timeout, UserService, ApiService
       $location.path('/repository/' + created.namespace + '/' + created.name);
     }, function(result) {
       $scope.creating = false;
-      $scope.createError = result.data;
+      $scope.createError = result.data ? result.data.message : 'Cannot create repository';
       $timeout(function() {
         $('#repoName').popover('show');
       });