Services return the HTTP call's status

In order to properly record the data for ServiceEvents in codeclimate
proper, we need to have the calls to the external services return
something useful.

The way this works is by moving all of the actual HTTP POSTing into a
dedicated method that wraps the response. Failures are handled by the
`WithErrorHandling` invocation middleware as normal. A successful
request will be parsed for standard things like status code, the params
we sent, the url we hit, and whether it worked or not, and a message.
Failures will have a message explaining the error. `receive_test`
methods will include a user-friendly error message. Any other error will
forego HTTP information. This is all tested and common to each service, so
they don't need their own specific tests for it.

A successful request can be modified by adding extra attributes into the
hash via a block. The response body is passed to the block to be parsed
inside it (since we get both JSON and XML from various services).
Relevant values can be extracted in this block. Parse errors will
trigger the generic error handling and return a "this failed" message to
the caller. The uniquely-added attributes are tested on a per-service
basis.

Some services make multiple requests. The first one to fail is what we
return an error message for. If none fail, it's all good.

Author: Jon Yurek

lib/cc/service/http.rb

@@ -15,7 +15,23 @@ def default_http_options
     end
   end
 
-  def http_get(url = nil, params = nil, headers = nil)
+  def get(url = nil, body = nil, headers = nil, &block)
+    raw_get(url, body, headers, &block)
+  end
+
+  def post(url, body = nil, headers = nil, &block)
+    block ||= lambda{|*args| Hash.new }
+    response = raw_post(url, body, headers)
+    {
+      ok: response.success?,
+      params: body.as_json,
+      endpoint_url: url,
+      status: response.status,
+      message: "Success"
+    }.merge(block.call(response))
+  end
+
+  def raw_get(url = nil, params = nil, headers = nil)
     http.get do |req|
       req.url(url)                if url
       req.params.update(params)   if params
@@ -24,7 +40,7 @@ def http_get(url = nil, params = nil, headers = nil)
     end
   end
 
-  def http_post(url = nil, body = nil, headers = nil)
+  def raw_post(url = nil, body = nil, headers = nil)
     block = Proc.new if block_given?
     http_method :post, url, body, headers, &block
   end

lib/cc/service/invocation/with_error_handling.rb

@@ -8,24 +8,35 @@ def initialize(invocation, logger, prefix = nil)
 
     def call
       @invocation.call
-    rescue => ex
-      @logger.error(error_message(ex))
-
-      nil
+    rescue CC::Service::HTTPError => e
+      @logger.error(error_message(e))
+      {
+        ok: false,
+        params: e.params,
+        status: e.status,
+        endpoint_url: e.endpoint_url,
+        message: error_message(e)
+      }
+    rescue => e
+      @logger.error(error_message(e))
+      {
+        ok: false,
+        message: error_message(e)
+      }
     end
 
     private
 
-    def error_message(ex)
-      if ex.respond_to?(:response_body)
-        response_body = ". Response: <#{ex.response_body.inspect}>"
+    def error_message(e)
+      if e.respond_to?(:response_body)
+        response_body = ". Response: <#{e.response_body.inspect}>"
       else
         response_body = ""
       end
 
       message  = "Exception invoking service:"
       message << " [#{@prefix}]" if @prefix
-      message << " (#{ex.class}) #{ex.message}"
+      message << " (#{e.class}) #{e.message}"
       message << response_body
     end
   end

lib/cc/service/response_check.rb

@@ -1,10 +1,12 @@
 class CC::Service
   class HTTPError < StandardError
-    attr_reader :response_body, :status
+    attr_reader :response_body, :status, :params, :endpoint_url
 
     def initialize(message, env)
       @response_body = env[:body]
       @status        = env[:status]
+      @params        = env[:params]
+      @endpoint_url  = env[:url]
 
       super(message)
     end

lib/cc/services/asana.rb

@@ -14,21 +14,20 @@ class Config < CC::Service::Config
     validates :workspace_id, presence: true
   end
 
+  ENDPOINT = "https://app.asana.com/api/1.0/tasks"
+
   self.title = "Asana"
   self.description = "Create tasks in Asana"
   self.issue_tracker = true
 
   def receive_test
     result = create_task("Test task from Code Climate")
-
-    {
-      ok: true,
-      message: "Ticked <a href='#{result[:url]}'>#{result[:id]}</a> created."
-    }
-  rescue => ex
-    { ok: false, message: ex.message }
+    result.merge(
+      message: "Ticket <a href='#{result[:url]}'>#{result[:id]}</a> created."
+    )
   end
 
+
   def receive_quality
     title = "Refactor #{constant_name} from #{rating} on Code Climate"
 
@@ -45,6 +44,18 @@ def receive_vulnerability
 private
 
   def create_task(name)
+    params = generate_params(name)
+    authenticate_http
+    http.headers["Content-Type"] = "application/json"
+    post(ENDPOINT, params.to_json) do |response|
+      body = JSON.parse(response.body)
+      id = body['data']['id']
+      url = "https://app.asana.com/0/#{config.workspace_id}/#{id}"
+      { id: id, url: url }
+    end
+  end
+
+  def generate_params(name)
     params = {
       data: { workspace: config.workspace_id, name: name }
     }
@@ -58,18 +69,11 @@ def create_task(name)
       params[:data][:assignee] = config.assignee
     end
 
-    http.headers["Content-Type"] = "application/json"
-    http.basic_auth(config.api_key, "")
-
-    url = "https://app.asana.com/api/1.0/tasks"
-    res = http_post(url, params.to_json)
-
-    body = JSON.parse(res.body)
-
-    id = body['data']['id']
-    url = "https://app.asana.com/0/#{config.workspace_id}/#{id}"
+    params
+  end
 
-    { id: id, url: url }
+  def authenticate_http
+    http.basic_auth(config.api_key, "")
   end
 
 end

lib/cc/services/campfire.rb

@@ -15,11 +15,9 @@ class Config < CC::Service::Config
   self.description = "Send messages to a Campfire chat room"
 
   def receive_test
-    speak(formatter.format_test)
-
-    { ok: true, message: "Test message sent" }
-  rescue => ex
-    { ok: false, message: ex.message }
+    speak(formatter.format_test).merge(
+      message: "Test message sent"
+    )
   end
 
   def receive_coverage
@@ -42,10 +40,10 @@ def formatter
 
   def speak(line)
     http.headers['Content-Type']  = 'application/json'
-    body = { message: { body: line } }
+    params = { message: { body: line } }
 
     http.basic_auth(config.token, "X")
-    http_post(speak_uri, body.to_json)
+    post(speak_uri, params.to_json)
   end
 
   def speak_uri

lib/cc/services/flowdock.rb

@@ -13,11 +13,9 @@ class Config < CC::Service::Config
   self.description = "Send messages to a Flowdock inbox"
 
   def receive_test
-    notify("Test", repo_name, formatter.format_test)
-
-    { ok: true, message: "Test message sent" }
-  rescue => ex
-    { ok: false, message: ex.message }
+    notify("Test", repo_name, formatter.format_test).merge(
+      message: "Test message sent"
+    )
   end
 
   def receive_coverage
@@ -56,7 +54,9 @@ def notify(subject, project, content)
     }
 
     url = "#{BASE_URL}/messages/team_inbox/#{config.api_token}"
+    http.headers['Content-Type']  = 'application/json'
     http.headers["User-Agent"] = "Code Climate"
-    http_post(url, params)
+
+    post(url, params)
   end
 end

lib/cc/services/github_issues.rb

@@ -21,13 +21,9 @@ class Config < CC::Service::Config
 
   def receive_test
     result = create_issue("Test ticket from Code Climate", "")
-
-    {
-      ok: true,
+    result.merge(
       message: "Issue <a href='#{result[:url]}'>##{result[:number]}</a> created."
-    }
-  rescue => ex
-    { ok: false, message: ex.message }
+    )
   end
 
   def receive_quality
@@ -59,15 +55,14 @@ def create_issue(title, issue_body)
     http.headers["User-Agent"] = "Code Climate"
 
     url = "#{BASE_URL}/repos/#{config.project}/issues"
-    res = http_post(url, params.to_json)
-
-    body = JSON.parse(res.body)
-
-    {
-      id:     body["id"],
-      number: body["number"],
-      url:    body["html_url"]
-    }
+    post(url, params.to_json) do |response|
+      body = JSON.parse(response.body)
+      {
+        id: body["id"],
+        number: body["number"],
+        url: body["html_url"]
+      }
+    end
   end
 
 end

lib/cc/services/github_pull_requests.rb

@@ -13,25 +13,6 @@ class Config < CC::Service::Config
     validates :oauth_token, presence: true
   end
 
-  class ResponseAggregator
-    def initialize(status_response, comment_response)
-      @status_response = status_response
-      @comment_response = comment_response
-    end
-
-    def response
-      return @status_response if @status_response[:ok] && @comment_response[:ok]
-      message = if !@status_response[:ok] && !@comment_response[:ok]
-        "Unable to post comment or update status"
-      elsif !@status_response[:ok]
-        "Unable to update status: #{@status_response[:message]}"
-      elsif !@comment_response[:ok]
-        "Unable to post comment: #{@comment_response[:message]}"
-      end
-      { ok: false, message: message }
-    end
-  end
-
   self.title = "GitHub Pull Requests"
   self.description = "Update pull requests on GitHub"
 
@@ -46,11 +27,14 @@ def receive_test
     setup_http
 
     if config.update_status && config.add_comment
-      ResponseAggregator.new(receive_test_status, receive_test_comment).response
+      receive_test_status
+      receive_test_comment
     elsif config.update_status
       receive_test_status
     elsif config.add_comment
       receive_test_comment
+    else
+      { ok: true, message: "Nothing happened" }
     end
   end
 
@@ -76,14 +60,13 @@ def receive_pull_request
 
   def update_status(state, description)
     if config.update_status
-      body = {
+      params = {
         state:       state,
         description: description,
         target_url:  @payload["details_url"],
         context:     "codeclimate"
-      }.to_json
-
-      http_post(status_url, body)
+      }
+      post(status_url, params.to_json)
     end
   end
 
@@ -93,37 +76,44 @@ def add_comment
         body: COMMENT_BODY % @payload["compare_url"]
       }.to_json
 
-      http_post(comments_url, body)
+      post(comments_url, body) do |response|
+        doc = JSON.parse(response.body)
+        { id: doc["id"] }
+      end
     end
   end
 
   def receive_test_status
-    http_post(base_status_url("0" * 40), "{}")
-
-  rescue HTTPError => ex
-    if ex.status == 422 # response message: "No commit found for SHA"
-      { ok: true, message: "OAuth token is valid" }
-    else ex.status == 401 # response message: "Bad credentials"
-      { ok: false, message: ex.message }
+    url = base_status_url("0" * 40)
+    params = {}
+    raw_post(url, params.to_json)
+  rescue CC::Service::HTTPError => e
+    if e.status == 422
+      {
+        ok: true,
+        params: params.as_json,
+        status: e.status,
+        endpoint_url: url,
+        message: "OAuth token is valid"
+      }
+    else
+      raise
     end
-  rescue => ex
-    { ok: false, message: ex.message }
   end
 
   def receive_test_comment
-    response = http_get(user_url)
+    response = get(user_url)
     if response_includes_repo_scope?(response)
       { ok: true, message: "OAuth token is valid" }
     else
       { ok: false, message: "OAuth token requires 'repo' scope to post comments." }
     end
-
   rescue => ex
     { ok: false, message: ex.message }
   end
 
   def comment_present?
-    response = http_get(comments_url)
+    response = get(comments_url)
     comments = JSON.parse(response.body)
 
     comments.any? { |comment| comment["body"] =~ BODY_REGEX }