From d9e001b688db83c3c848ab882f3ae6a4a940cc97 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <josephschorr@users.noreply.github.com>
Date: Fri, 16 Oct 2015 13:51:50 -0400
Subject: [PATCH] Better GitHub error messaging

Fixes #612
---
 buildtrigger/githubhandler.py                 | 30 ++++++++++++++-----
 .../js/directives/ui/setup-trigger-dialog.js  |  2 +-
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py
index 1a74e5c2f..2423cbc07 100644
--- a/buildtrigger/githubhandler.py
+++ b/buildtrigger/githubhandler.py
@@ -159,6 +159,13 @@ class GithubBuildTrigger(BuildTriggerHandler):
     source = self.config['build_source']
     return github_trigger.get_public_url(source)
 
+  @staticmethod
+  def _get_error_message(ghe, default_msg):
+    if ghe.data.get('errors') and ghe.data['errors'][0].get('message'):
+      return ghe.data['errors'][0]['message']
+
+    return default_msg
+
   def activate(self, standard_webhook_url):
     config = self.config
     new_build_source = config['build_source']
@@ -179,12 +186,14 @@ class GithubBuildTrigger(BuildTriggerHandler):
         'value': public_key,
       },
     ]
+
     try:
       deploy_key = gh_repo.create_key('%s Builder' % app.config['REGISTRY_TITLE'],
                                       public_key)
       config['deploy_key_id'] = deploy_key.id
-    except GithubException:
-      msg = 'Unable to add deploy key to repository: %s' % new_build_source
+    except GithubException as ghe:
+      default_msg = 'Unable to add deploy key to repository: %s' % new_build_source
+      msg = GithubBuildTrigger._get_error_message(ghe, default_msg)
       raise TriggerActivationException(msg)
 
     # Add the webhook to the GitHub repository.
@@ -192,12 +201,14 @@ class GithubBuildTrigger(BuildTriggerHandler):
       'url': standard_webhook_url,
       'content_type': 'json',
     }
+
     try:
       hook = gh_repo.create_hook('web', webhook_config)
       config['hook_id'] = hook.id
       config['master_branch'] = gh_repo.default_branch
     except GithubException:
-      msg = 'Unable to create webhook on repository: %s' % new_build_source
+      default_msg = 'Unable to create webhook on repository: %s' % new_build_source
+      msg = GithubBuildTrigger._get_error_message(ghe, default_msg)
       raise TriggerActivationException(msg)
 
     return config, {'private_key': private_key}
@@ -224,16 +235,18 @@ class GithubBuildTrigger(BuildTriggerHandler):
     except KeyError:
       # There was no config['deploy_key_id'], thus this is an old trigger without a deploy key.
       pass
-    except GithubException:
-      msg = 'Unable to remove deploy key: %s' % config['deploy_key_id']
+    except GithubException as ghe:
+      default_msg = 'Unable to remove deploy key: %s' % config['deploy_key_id']
+      msg = GithubBuildTrigger._get_error_message(ghe, default_msg)
       raise TriggerDeactivationException(msg)
 
     # Remove the webhook.
     try:
       hook = repo.get_hook(config['hook_id'])
       hook.delete()
-    except GithubException:
-      msg = 'Unable to remove hook: %s' % config['hook_id']
+    except GithubException as ghe:
+      default_msg = 'Unable to remove hook: %s' % config['hook_id']
+      msg = GithubBuildTrigger._get_error_message(ghe, default_msg)
       raise TriggerDeactivationException(msg)
 
     config.pop('hook_id', None)
@@ -435,7 +448,8 @@ class GithubBuildTrigger(BuildTriggerHandler):
       repo = gh_client.get_repo(source)
       default_branch = repo.default_branch
     except GithubException as ghe:
-      raise TriggerStartException(ghe.data['message'])
+      msg = GithubBuildTrigger._get_error_message(ghe, 'Unable to start build trigger')
+      raise TriggerStartException(msg)
 
     def get_branch_sha(branch_name):
       branch = repo.get_branch(branch_name)
diff --git a/static/js/directives/ui/setup-trigger-dialog.js b/static/js/directives/ui/setup-trigger-dialog.js
index 46d0550bb..a9aa70c05 100644
--- a/static/js/directives/ui/setup-trigger-dialog.js
+++ b/static/js/directives/ui/setup-trigger-dialog.js
@@ -119,7 +119,7 @@ angular.module('quay').directive('setupTriggerDialog', function () {
           $scope.canceled({'trigger': $scope.trigger});
 
           return ApiService.getErrorMessage(resp) +
-            '\n\nThis usually means that you do not have admin access on the repository.';
+            '\n\nNote: Errors can occur if you do not have admin access on the repository.';
         });
 
         ApiService.activateBuildTrigger(data, params).then(function(resp) {