Add json response handler for server internal errors

Fixes #3035

Signed-off-by: Sebastian Wojciechowski <[email protected]>
(cherry picked from commit 2d30986)

bodhi/server/static/js/forms.js

@@ -70,15 +70,31 @@ Form.prototype.success = function(data) {
 Form.prototype.error = function(data) {
     var self = this;
     self.finish();
-    // Here is where we handle those error messages on the response by cornice.
-    $.each(data.responseJSON.errors, function (i, error) {
-        msg = self.messenger.post({
-            message: error.name.capitalize() + ' : ' + error.description,
-            type: "error",
-            hideAfter: false,
-            showCloseButton: true,
+    if (data.status >= 500) {
+        // In case of Internal Server Error show error modal
+        var msg = '';
+
+        $.each(data.responseJSON.errors, function (i, error) {
+            if (msg != '') {
+                msg += '; ';
+            }
+            msg += error.name.capitalize() + ' : ' + error.description;
         });
-    });
+
+        $('#alertModalDescription').text(msg);
+        $('#alertModal').modal('show');
+    }
+    else {
+        // Here is where we handle those error messages on the response by cornice.
+        $.each(data.responseJSON.errors, function (i, error) {
+            msg = self.messenger.post({
+                message: error.name.capitalize() + ' : ' + error.description,
+                type: "error",
+                hideAfter: false,
+                showCloseButton: true,
+            });
+        });
+    }
 }
 
 Form.prototype.data = function() {

bodhi/server/templates/master.html

@@ -224,6 +224,28 @@
       </div>
     </div>
 
+    <!-- Alert Modal -->
+    <div class="modal fade" id="alertModal" tabindex="-1" role="dialog" aria-labelledby="alertModalLabel" aria-hidden="true" data-backdrop="static" data-keyboard="false">
+      <div class="modal-dialog" role="document">
+	<div class="modal-content">
+          <div class="modal-header">
+            <h5 class="modal-title" id="alertModalLabel">Internal Server Error!</h5>
+          </div>
+          <div class="modal-body" id="alertModalDescription">
+	     The server encountered an internal error
+          </div>
+          <div class="modal-footer">
+            <button type="button" class="btn btn-secondary" data-dismiss="modal">Close</button>
+          </div>
+        </div>
+      </div>
+    </div>
+    <script>
+      $('#alertModal').on('hidden.bs.modal', function (e) {
+          document.location.href = document.baseURI;
+      })
+    </script>
+
     <!-- Placed at the end of the document so the pages load faster -->
     <script src="${request.static_url('bodhi:server/static/jquery.flot.js')}"></script>
     <script src="${request.static_url('bodhi:server/static/jquery.flot.stack.js')}"></script>

bodhi/server/views/generic.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Copyright © 2014-2017 Red Hat, Inc. and others
+# Copyright © 2014-2019 Red Hat, Inc. and others
 #
 # This file is part of Bodhi.
 #
@@ -428,14 +428,14 @@ def notfound_view(context, request):
         bodhi.server.services.errors.html_handler: A pyramid.httpexceptions.HTTPError to be rendered
             to the user for the 404.
     """
-    return exception_view(context, request)
+    return exception_html_view(context, request)
 
 
-@view_config(context=HTTPForbidden)
-@view_config(context=Exception)
-def exception_view(exc, request):
+@view_config(context=HTTPForbidden, accept='text/html')
+@view_config(context=Exception, accept='text/html')
+def exception_html_view(exc, request):
     """
-    Return an error response upon generic errors (404s, 403s, 500s, etc..).
+    Return a html error response upon generic errors (404s, 403s, 500s, etc..).
 
     This is here to catch everything that isn't caught by our cornice error
     handlers.  When we do catch something, we transform it into a cornice
@@ -464,3 +464,39 @@ def exception_view(exc, request):
         errors.add('body', description=description)
 
     return bodhi.server.services.errors.html_handler(errors, request)
+
+
+@view_config(context=HTTPForbidden, accept='application/json')
+@view_config(context=Exception, accept='application/json')
+def exception_json_view(exc, request):
+    """
+    Return a json error response upon generic errors (404s, 403s, 500s, etc..).
+
+    This is here to catch everything that isn't caught by our cornice error
+    handlers.  When we do catch something, we transform it into a cornice
+    Errors object and pass it to our nice cornice error handler.  That way, all
+    the exception presentation and rendering we can keep in one place.
+
+    Args:
+        exc (Exception): The unhandled exception.
+        request (pyramid.util.Request): The current request.
+    Returns:
+        bodhi.server.services.errors.html_handler: A pyramid.httpexceptions.HTTPError to be rendered
+            to the user for the given exception.
+    """
+    errors = getattr(request, 'errors', [])
+    status = getattr(exc, 'status_code', 500)
+
+    if status not in (404, 403):
+        log.exception("Error caught.  Handling JSON response.")
+    else:
+        log.warning(str(exc))
+
+    if not len(errors):
+        description = getattr(exc, 'explanation', None) or str(exc)
+
+        errors = cornice.errors.Errors(status=status)
+        errors.add('body', description=description, name=exc.__class__.__name__)
+        request.errors = errors
+
+    return bodhi.server.services.errors.json_handler(request)

bodhi/tests/server/functional/test_admin.py

@@ -37,12 +37,13 @@ def test_admin(self):
 
     def test_admin_unauthed(self):
         """Test that an unauthed user cannot see the admin endpoint"""
+        headers = {'Accept': 'text/html'}
         anonymous_settings = copy.copy(self.app_settings)
         anonymous_settings.update({
             'authtkt.secret': 'whatever',
             'authtkt.secure': True,
         })
         app = webtest.TestApp(main({}, session=self.db, **anonymous_settings))
-        res = app.get('/admin/', status=403)
+        res = app.get('/admin/', status=403, headers=headers)
         self.assertIn('<h1>403 <small>Forbidden</small></h1>', res)
         self.assertIn('<p class="lead">Access was denied to this resource.</p>', res)

bodhi/tests/server/functional/test_stacks.py

@@ -277,12 +277,13 @@ def test_new_stack_form_unauthed(self):
         """
         Assert we get a 403 if the user is not logged in
         """
+        headers = {'Accept': 'text/html'}
         anonymous_settings = copy.copy(self.app_settings)
         anonymous_settings.update({
             'authtkt.secret': 'whatever',
             'authtkt.secure': True,
         })
         app = webtest.TestApp(main({}, session=self.db, **anonymous_settings))
-        res = app.get('/stacks/new', status=403)
+        res = app.get('/stacks/new', status=403, headers=headers)
         self.assertIn('<h1>403 <small>Forbidden</small></h1>', res)
         self.assertIn('<p class="lead">Access was denied to this resource.</p>', res)

bodhi/tests/server/views/test_generic.py

@@ -36,14 +36,34 @@ class TestExceptionView(base.BaseTestCase):
     @mock.patch('bodhi.server.views.generic.log.exception')
     @mock.patch('bodhi.server.views.metrics.compute_ticks_and_data',
                 mock.MagicMock(side_effect=RuntimeError("BOOM")))
-    def test_status_500(self, exception):
+    def test_status_500_html(self, exception):
         """Assert that a status 500 code causes the exception to get logged."""
-        res = self.app.get('/metrics', status=500)
+        headers = {'Accept': 'text/html'}
+        res = self.app.get('/metrics', status=500, headers=headers)
 
+        self.assertEqual('text/html', res.content_type)
         self.assertIn('Server Error', res)
         self.assertIn('BOOM', res)
         exception.assert_called_once_with("Error caught.  Handling HTML response.")
 
+    @mock.patch('bodhi.server.views.generic.log.exception')
+    @mock.patch('bodhi.server.views.metrics.compute_ticks_and_data',
+                mock.MagicMock(side_effect=RuntimeError("BOOM")))
+    def test_status_500_json(self, exception):
+        """Assert that a status 500 code causes the exception to get logged."""
+        headers = {'Content-Type': 'application/json'}
+        res = self.app.get('/metrics', status=500, headers=headers)
+
+        self.assertEqual('application/json', res.content_type)
+        self.assertEqual(
+            res.json_body,
+            {
+                "status": "error",
+                "errors": [{"location": "body", "name": "RuntimeError", "description": "BOOM"}]
+            },
+        )
+        exception.assert_called_once_with("Error caught.  Handling JSON response.")
+
 
 class TestGenericViews(base.BaseTestCase):
 
@@ -348,15 +368,18 @@ def test_masher_status(self):
 
     def test_popup_toggle(self):
         """Check that the toggling of pop-up notifications works"""
+
+        headers = {'Accept': 'text/html'}
+
         # first we check that popups are enabled by default
-        res = self.app.get('/')
+        res = self.app.get('/', headers=headers)
         self.assertIn('Disable popups', res)
 
         # toggle popups off
         self.app.post('/popup_toggle')
 
         # now check popups are off
-        res = self.app.get('/')
+        res = self.app.get('/', headers=headers)
         self.assertIn('Enable popups', res)
 
         # test that the unlogged in user cannot toggle popups
@@ -366,15 +389,17 @@ def test_popup_toggle(self):
             'authtkt.secure': True,
         })
         app = webtest.TestApp(main({}, session=self.db, **anonymous_settings))
-        res = app.post('/popup_toggle', status=403)
+        res = app.post('/popup_toggle', status=403, headers=headers)
         self.assertIn('<h1>403 <small>Forbidden</small></h1>', res)
         self.assertIn('<p class="lead">Access was denied to this resource.</p>', res)
 
     def test_new_override_form(self):
         """Test the New Override form page"""
 
+        headers = {'Accept': 'text/html'}
+
         # Test that the New Override form shows when logged in
-        res = self.app.get('/overrides/new')
+        res = self.app.get('/overrides/new', headers=headers)
         self.assertIn('<span>New Buildroot Override Form Requires JavaScript</span>', res)
 
         # Test that the unlogged in user cannot see the New Override form
@@ -384,15 +409,17 @@ def test_new_override_form(self):
             'authtkt.secure': True,
         })
         app = webtest.TestApp(main({}, session=self.db, **anonymous_settings))
-        res = app.get('/overrides/new', status=403)
+        res = app.get('/overrides/new', status=403, headers=headers)
         self.assertIn('<h1>403 <small>Forbidden</small></h1>', res)
         self.assertIn('<p class="lead">Access was denied to this resource.</p>', res)
 
     def test_new_update_form(self):
         """Test the new update Form page"""
 
+        headers = {'Accept': 'text/html'}
+
         # Test that a logged in user sees the New Update form
-        res = self.app.get('/updates/new')
+        res = self.app.get('/updates/new', headers=headers)
         self.assertIn('Creating a new update requires JavaScript', res)
         # Make sure that unspecified comes first, as it should be the default.
         regex = r''
@@ -407,7 +434,7 @@ def test_new_update_form(self):
             'authtkt.secure': True,
         })
         app = webtest.TestApp(main({}, session=self.db, **anonymous_settings))
-        res = app.get('/updates/new', status=403)
+        res = app.get('/updates/new', status=403, headers=headers)
         self.assertIn('<h1>403 <small>Forbidden</small></h1>', res)
         self.assertIn('<p class="lead">Access was denied to this resource.</p>', res)