Improve error handling in client

When network request fails, the error printing is more robust. Ideally,
the error response should be JSON encoded. In that case it will be
pretty printed.

If JSON parsing or pretty printing fails, user is prompted to file a
bug. The response data is logged only if it was valid JSON, as the only
case of non-JSON responses is a huge HTML dump.

The error is now consistently printed to stderr.

JIRA: PDC-1193

pdc_client/runner.py

@@ -4,6 +4,9 @@
 # Licensed under The MIT License (MIT)
 # http://opensource.org/licenses/MIT
 #
+
+from __future__ import print_function
+
 import sys
 import argparse
 import beanbag
@@ -29,6 +32,7 @@ def autocomplete(*args):
             pass
 
 import pdc_client
+from pdc_client.utils import pretty_print
 
 
 # A list of paths to directories where plugins should be loaded from.
@@ -95,30 +99,30 @@ def run(self, args=None):
             self.args.func(self.args)
         except beanbag.BeanBagException as exc:
             self.print_error_header(exc)
+            json = None
             try:
-                self.print_error_details(exc.response.json())
+                json = exc.response.json()
+                pretty_print(json, file=sys.stderr)
             except ValueError:
-                pass
+                # Response was not JSON
+                print('Failed to parse error response.', file=sys.stderr)
+            except TypeError as e:
+                # Failed to pretty print.
+                print('Failed to correctly display error message. Please file a bug.',
+                      file=sys.stderr)
+                self.logger.info(json, exc_info=e)
             sys.exit(1)
 
     def print_error_header(self, exc):
         if exc.response.status_code > 500:
-            print 'Internal server error. Please consider reporting a bug.'
+            print('Internal server error. Please consider reporting a bug.',
+                  file=sys.stderr)
         else:
             headers = {
                 400: 'bad request data',
                 401: 'unauthorized',
                 404: 'not found',
                 409: 'conflict',
             }
-            print 'Client error: {}.'.format(headers.get(exc.response.status_code, 'unknown'))
-
-    def print_error_details(self, body):
-        self.logger.debug(body)
-        for key, value in body.iteritems():
-            if isinstance(value, basestring):
-                print '{}: {}'.format(key, value)
-            else:
-                print '{}:'.format(key)
-                for error in value:
-                    print ' * {}'.format(error)
+            print('Client error: {}.'.format(headers.get(exc.response.status_code, 'unknown')),
+                  file=sys.stderr)

pdc_client/tests/tests.py

@@ -5,8 +5,10 @@
 # http://opensource.org/licenses/MIT
 #
 import unittest
+from StringIO import StringIO
 
 from .. import plugin_helpers
+from .. import utils
 
 
 class PluginHelperTestCase(unittest.TestCase):
@@ -19,3 +21,26 @@ class Temp(object):
         data = plugin_helpers.extract_arguments(args, prefix='prf__')
         self.assertDictEqual(data,
                              {'foo': {'bar': {'baz': 1, 'quux': 2}}})
+
+
+class PrettyPrinterTestCase(unittest.TestCase):
+    def setUp(self):
+        self.out = StringIO()
+
+    def tearDown(self):
+        self.out.close()
+
+    def test_print_list(self):
+        utils.pretty_print(['foo', 'bar', 'baz'], file=self.out)
+        self.assertEqual(self.out.getvalue(),
+                         '* foo\n* bar\n* baz\n')
+
+    def test_print_dict(self):
+        utils.pretty_print({'foo': 'bar'}, file=self.out)
+        self.assertEqual(self.out.getvalue(),
+                         'foo:\n * bar\n')
+
+    def test_print_nested_dict(self):
+        utils.pretty_print({'foo': {'bar': ['baz', 'quux']}}, file=self.out)
+        self.assertEqual(self.out.getvalue(),
+                         'foo:\n bar:\n  * baz\n  * quux\n')

pdc_client/utils.py

@@ -0,0 +1,39 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (c) 2015 Red Hat
+# Licensed under The MIT License (MIT)
+# http://opensource.org/licenses/MIT
+#
+from __future__ import print_function
+
+import sys
+
+
+def _pprint_str(file, s, indent, lead=''):
+    """Print indented string with optional leading text."""
+    print(' ' * indent + lead + s, file=file)
+
+
+def _pprint_list(file, items, indent):
+    """Print an indented bullet point list."""
+    for item in items:
+        _pprint_str(file, item, indent, lead='* ')
+
+
+def _pprint_dict(file, data, indent):
+    """Print a dict as an indented definition list."""
+    for key, value in data.iteritems():
+        _pprint_str(file, key + ':', indent)
+        pretty_print(value, indent + 1, file)
+
+
+def pretty_print(data, indent=0, file=sys.stdout):
+    """Pretty print a data structure."""
+    if isinstance(data, basestring):
+        _pprint_list(file, [data], indent)
+    elif isinstance(data, list):
+        _pprint_list(file, data, indent)
+    elif isinstance(data, dict):
+        _pprint_dict(file, data, indent)
+    else:
+        raise TypeError('Can not handle {}'.format(type(data)))