Finesse command line error handling

lovelydinosaur · lovelydinosaur · commit d8ed9766cfbd · 2016-01-20T13:45:12.000Z
diff --git a/coreapi/__init__.py b/coreapi/__init__.py
@@ -7,7 +7,7 @@
 from coreapi.transport import BaseTransport, HTTPTransport
 
 
-__version__ = '1.11.3'
+__version__ = '1.11.4'
 __all__ = [
     'BaseCodec', 'CoreJSONCodec', 'HALCodec', 'HTMLCodec', 'PlainTextCodec', 'PythonCodec',
     'negotiate_encoder', 'negotiate_decoder',
diff --git a/coreapi/commandline.py b/coreapi/commandline.py
@@ -152,21 +152,31 @@ def show(path):
     if path:
         keys = coerce_key_types(doc, path)
         for key in keys:
-            doc = doc[key]
+            try:
+                doc = doc[key]
+            except (KeyError, IndexError):
+                click.echo('Key %s not found.' % repr(key).strip('u'))
+                sys.exit(1)
     click.echo(display(doc))
 
 
 def validate_params(ctx, param, value):
     if any(['=' not in item for item in value]):
         raise click.BadParameter('Parameters need to be in format <field name>=<value>')
+
+    # Convert to dict.
     params = dict([tuple(item.split('=', 1)) for item in value])
+
+    # Gracefully handle non-string values.
+    # Eg treat "-p complete=true" as {"complete": True}
     for key, value in params.items():
         try:
             value = json.loads(value)
         except:
             pass
         else:
             params[key] = value
+
     return params
 
 
@@ -201,6 +211,9 @@ def action(path, param, action, inplace):
     except coreapi.exceptions.ErrorMessage as exc:
         click.echo(display(exc.error))
         sys.exit(1)
+    except coreapi.exceptions.NodeLookupError as exc:
+        click.echo(exc)
+        sys.exit(1)
     history = history.add(doc)
     click.echo(display(doc))
     set_document(doc)
diff --git a/coreapi/exceptions.py b/coreapi/exceptions.py
@@ -31,6 +31,13 @@ class TransportError(Exception):
     pass
 
 
+class NodeLookupError(Exception):
+    """
+    Raised when `.action` fails to index a link in the document.
+    """
+    pass
+
+
 class ErrorMessage(Exception):
     """
     Raised when the transition returns an error message.
diff --git a/coreapi/validation.py b/coreapi/validation.py
@@ -1,5 +1,6 @@
 from coreapi.compat import string_types
 from coreapi.document import Document, Link
+from coreapi.exceptions import NodeLookupError
 import collections
 
 
@@ -36,23 +37,6 @@ def validate_parameters(link, parameters):
 
     Raises a `ValueError` if any parameters do not validate.
     """
-    provided = set(parameters.keys())
-    required = set([
-        field.name for field in link.fields if field.required
-    ])
-
-    # Determine if any required field names not supplied.
-    missing = required - provided
-    missing = ['"' + item + '"' for item in sorted(missing)]
-    if missing:
-        fmt = 'Missing required parameter{plural}: {listing}'
-        plural = 's' if len(missing) > 1 else ''
-        listing = ', '.join(missing)
-        raise ValueError(fmt.format(
-            plural=plural,
-            listing=listing
-        ))
-
     # Ensure all parameter values are valid types.
     for value in parameters.values():
         validate_is_primative(value)
@@ -65,7 +49,7 @@ def validate_keys_to_link(document, keys):
     Returns a two-tuple of (link, link_ancestors).
     """
     if not isinstance(keys, (list, tuple)):
-        msg = "'keys' must be a dot seperated string or a list of strings."
+        msg = "'keys' must be a list of strings or intes."
         raise TypeError(msg)
     if any([
         not isinstance(key, string_types) and not isinstance(key, int)
@@ -79,15 +63,22 @@ def validate_keys_to_link(document, keys):
     node = document
     link_ancestors = [Ancestor(document=document, keys=[])]
     for idx, key in enumerate(keys, start=1):
-        node = node[key]
+        try:
+            node = node[key]
+        except (KeyError, IndexError):
+            index_string = ''.join('[%s]' % repr(key).strip('u') for key in keys)
+            msg = 'Index %s did not reference a link. Key %s was not found.'
+            raise NodeLookupError(msg % (index_string, repr(key).strip('u')))
         if isinstance(node, Document):
             ancestor = Ancestor(document=node, keys=keys[:idx])
             link_ancestors.append(ancestor)
 
     # Ensure that we've correctly indexed into a link.
     if not isinstance(node, Link):
-        raise ValueError(
-            "Can only call 'action' on a Link. Got type '%s'." % type(node)
+        index_string = ''.join('[%s]' % repr(key).strip('u') for key in keys)
+        msg = "Can only call 'action' on a Link. Index %s returned type '%s'."
+        raise NodeLookupError(
+            msg % (index_string, type(node).__name__)
         )
 
     return (node, link_ancestors)
diff --git a/tests/test_document.py b/tests/test_document.py
@@ -1,6 +1,7 @@
 # coding: utf-8
 from coreapi import action
 from coreapi import Array, Document, Object, Link, Error, Field
+from coreapi.exceptions import NodeLookupError
 import pytest
 
 
@@ -451,12 +452,12 @@ def test_keys_should_be_a_list_of_strings_or_ints(doc):
 
 
 def test_keys_should_be_valid_indexes(doc):
-    with pytest.raises(KeyError):
+    with pytest.raises(NodeLookupError):
         action(doc, 'dummy')
 
 
 def test_keys_should_access_a_link(doc):
-    with pytest.raises(ValueError):
+    with pytest.raises(NodeLookupError):
         action(doc, 'dict')
 
 
diff --git a/tests/test_validation.py b/tests/test_validation.py
@@ -21,11 +21,6 @@ def test_link_with_correct_parameters(link):
     validate_parameters(link, {'required': 123, 'optional': 456})
 
 
-def test_link_missing_required_parameter(link):
-    with pytest.raises(ValueError):
-        validate_parameters(link, {'optional': 456})
-
-
 def test_link_with_additional_parameter(link):
     validate_parameters(link, {'required': 123, 'unknown': 123})
 
