Merge pull request #636 from jzelinskie/timeouts

raise a 520 for any GitLab timeouts

buildtrigger/gitlabhandler.py

@@ -1,5 +1,7 @@
 import logging
 
+from functools import wraps
+
 from app import app
 
 from jsonschema import validate
@@ -13,8 +15,10 @@ from buildtrigger.basehandler import BuildTriggerHandler
 
 from util.security.ssh import generate_ssh_keypair
 from util.dict_wrappers import JSONPathDict, SafeDictSetter
+from endpoints.api import ExternalServiceTimeout
 
 import gitlab
+import requests
 
 logger = logging.getLogger(__name__)
 
@@ -68,6 +72,18 @@ GITLAB_WEBHOOK_PAYLOAD_SCHEMA = {
   'required': ['ref', 'checkout_sha', 'repository'],
 }
 
+def _catch_timeouts(func):
+  @wraps(func)
+  def wrapper(*args, **kwargs):
+    try:
+      return func(*args, **kwargs)
+    except requests.exceptions.Timeout:
+      msg = 'Request to the GitLab API timed out'
+      logger.exception(msg)
+      raise ExternalServiceTimeout(msg)
+  return wrapper
+
+
 def get_transformed_webhook_payload(gl_payload, default_branch=None, lookup_user=None):
   """ Returns the Gitlab webhook JSON payload transformed into our own payload
       format. If the gl_payload is not valid, returns None.
@@ -118,6 +134,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
   def is_active(self):
     return 'hook_id' in self.config
 
+  @_catch_timeouts
   def activate(self, standard_webhook_url):
     config = self.config
     new_build_source = config['build_source']
@@ -182,6 +199,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
 
     return config
 
+  @_catch_timeouts
   def list_build_sources(self):
     gl_client = self._get_authorized_client()
     current_user = gl_client.currentuser()
@@ -208,6 +226,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
 
     return namespaces.values()
 
+  @_catch_timeouts
   def list_build_subdirs(self):
     config = self.config
     gl_client = self._get_authorized_client()
@@ -238,6 +257,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
 
     return []
 
+  @_catch_timeouts
   def load_dockerfile_contents(self):
     gl_client = self._get_authorized_client()
     path = self.get_dockerfile_path()
@@ -261,6 +281,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
 
     return contents
 
+  @_catch_timeouts
   def list_field_values(self, field_name, limit=None):
     if field_name == 'refs':
       branches = self.list_field_values('branch_name')
@@ -299,6 +320,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
   def get_repository_url(self):
     return 'https://gitlab.com/%s' % self.config['build_source']
 
+  @_catch_timeouts
   def lookup_user(self, email):
     gl_client = self._get_authorized_client()
     try:
@@ -312,6 +334,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
     except ValueError:
       return None
 
+  @_catch_timeouts
   def get_metadata_for_commit(self, commit_sha, ref, repo):
     gl_client = self._get_authorized_client()
     commit = gl_client.getrepositorycommit(repo['id'], commit_sha)
@@ -352,6 +375,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
 
     return metadata
 
+  @_catch_timeouts
   def manual_start(self, run_parameters=None):
     gl_client = self._get_authorized_client()
 
@@ -384,6 +408,7 @@ class GitLabBuildTrigger(BuildTriggerHandler):
     metadata = self.get_metadata_for_commit(commit_sha, ref, repo)
     return self.prepare_build(metadata, is_manual=True)
 
+  @_catch_timeouts
   def handle_trigger_request(self, request):
     payload = request.get_json()
     if not payload:

endpoints/api/__init__.py

@@ -49,10 +49,16 @@ class ApiException(Exception):
     return rv
 
 
+class ExternalServiceTimeout(ApiException):
+  def __init__(self, error_description, payload=None):
+    ApiException.__init__(self, 'external_service_timeout', 520, error_description, payload)
+
+
 class InvalidRequest(ApiException):
   def __init__(self, error_description, payload=None):
     ApiException.__init__(self, 'invalid_request', 400, error_description, payload)
 
+
 class InvalidResponse(ApiException):
   def __init__(self, error_description, payload=None):
     ApiException.__init__(self, 'invalid_response', 400, error_description, payload)