Merge pull request #1198 from code-corps/1192-save-changeset-errors

Save changeset errors and data for GitHub webhooks

Author: joshsmith

config/config.exs

@@ -82,6 +82,8 @@ config :sentry,
   included_environments: ~w(prod staging)a,
   use_error_logger: true
 
+config :code_corps, :sentry, CodeCorps.Sentry.Async
+
 config :code_corps, :processor, CodeCorps.Processor.Async
 
 # Import environment specific config. This must remain at the bottom

config/test.exs

@@ -57,6 +57,8 @@ config :code_corps,
 config :sentry,
   environment_name: Mix.env || :test
 
+config :code_corps, :sentry, CodeCorps.Sentry.Sync
+
 config :code_corps, :processor, CodeCorps.Processor.Sync
 
 config :code_corps, CodeCorps.Mailer,

lib/code_corps/github/event.ex

@@ -7,6 +7,16 @@ defmodule CodeCorps.GitHub.Event do
   alias CodeCorps.{GithubEvent, Repo}
   alias Ecto.Changeset
 
+  defmodule GitHubEventError do
+    defexception [:reason]
+
+    def exception(reason),
+      do: %__MODULE__{reason: reason}
+
+    def message(%__MODULE__{reason: reason}),
+      do: reason
+  end
+
   @type error :: atom | Changeset.t
 
   @doc ~S"""
@@ -33,11 +43,24 @@ defmodule CodeCorps.GitHub.Event do
     |> Changeset.change(%{status: "processed"})
     |> Repo.update()
   end
-  def stop_processing({:error, reason}, %GithubEvent{} = event) do
-    changes = %{status: "errored", failure_reason: reason |> Atom.to_string}
+  def stop_processing({:error, reason, error}, %GithubEvent{} = event) do
+    %GitHubEventError{reason: error}
+    |> CodeCorps.Sentry.capture_exception([stacktrace: System.stacktrace()])
+
+    changes = %{
+      status: "errored",
+      data: error |> format_data_if_exists(),
+      error: error |> Kernel.inspect(pretty: true),
+      failure_reason: reason |> Atom.to_string()
+    }
 
     event
     |> Changeset.change(changes)
     |> Repo.update()
   end
+
+  defp format_data_if_exists(%Ecto.Changeset{data: data}) do
+    data |> Kernel.inspect(pretty: true)
+  end
+  defp format_data_if_exists(_error), do: nil
 end

lib/code_corps/github/sync/sync.ex

@@ -20,7 +20,7 @@ defmodule CodeCorps.GitHub.Sync do
   @type outcome :: {:ok, list(Comment.t)}
                  | {:ok, GithubPullRequest.t}
                  | {:ok, list(CodeCorps.Task.t)}
-                 | {:error, :repo_not_found}
+                 | {:error, :repo_not_found, %{}}
                  | {:error, :fetching_issue}
                  | {:error, :fetching_pull_request}
                  | {:error, :multiple_issue_users_match}
@@ -232,18 +232,17 @@ defmodule CodeCorps.GitHub.Sync do
   defp marshall_result({:ok, %{github_pull_request: _, github_issue: _}} = result), do: result
   defp marshall_result({:ok, %{github_pull_request: pull_request}}), do: {:ok, pull_request}
   defp marshall_result({:ok, %{task: task}}), do: {:ok, task}
-  defp marshall_result({:error, :repo, :unmatched_project, _steps}), do: {:ok, []}
-  defp marshall_result({:error, :repo, :unmatched_repository, _steps}), do: {:error, :repo_not_found}
-  defp marshall_result({:error, :fetch_issue, _, _steps}), do: {:error, :fetching_issue}
-  defp marshall_result({:error, :fetch_pull_request, _, _steps}), do: {:error, :fetching_pull_request}
-  defp marshall_result({:error, :github_pull_request, %Ecto.Changeset{}, _steps}), do: {:error, :validating_github_pull_request}
-  defp marshall_result({:error, :github_issue, %Ecto.Changeset{}, _steps}), do: {:error, :validating_github_issue}
-  defp marshall_result({:error, :github_comment, %Ecto.Changeset{}, _steps}), do: {:error, :validating_github_comment}
-  defp marshall_result({:error, :comment_user, %Ecto.Changeset{}, _steps}), do: {:error, :validating_user}
-  defp marshall_result({:error, :comment_user, :multiple_users, _steps}), do: {:error, :multiple_comment_users_match}
-  defp marshall_result({:error, :issue_user, %Ecto.Changeset{}, _steps}), do: {:error, :validating_user}
-  defp marshall_result({:error, :issue_user, :multiple_users, _steps}), do: {:error, :multiple_issue_users_match}
-  defp marshall_result({:error, :comment, {_comments, _errors}, _steps}), do: {:error, :validating_comment}
-  defp marshall_result({:error, :task, %Ecto.Changeset{}, _steps}), do: {:error, :validating_task}
-  defp marshall_result({:error, _errored_step, _error_response, _steps}), do: {:error, :unexpected_transaction_outcome}
+  defp marshall_result({:error, :repo, :unmatched_repository, _steps}), do: {:error, :repo_not_found, %{}}
+  defp marshall_result({:error, :fetch_issue, _, _steps}), do: {:error, :fetching_issue, %{}}
+  defp marshall_result({:error, :fetch_pull_request, _, _steps}), do: {:error, :fetching_pull_request, %{}}
+  defp marshall_result({:error, :github_pull_request, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_github_pull_request, changeset}
+  defp marshall_result({:error, :github_issue, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_github_issue, changeset}
+  defp marshall_result({:error, :github_comment, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_github_comment, changeset}
+  defp marshall_result({:error, :comment_user, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_user, changeset}
+  defp marshall_result({:error, :comment_user, :multiple_users, _steps}), do: {:error, :multiple_comment_users_match, %{}}
+  defp marshall_result({:error, :issue_user, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_user, changeset}
+  defp marshall_result({:error, :issue_user, :multiple_users, _steps}), do: {:error, :multiple_issue_users_match, %{}}
+  defp marshall_result({:error, :comment, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_comment, changeset}
+  defp marshall_result({:error, :task, %Ecto.Changeset{} = changeset, _steps}), do: {:error, :validating_task, changeset}
+  defp marshall_result({:error, _errored_step, error_response, _steps}), do: {:error, :unexpected_transaction_outcome, error_response}
 end

lib/code_corps/model/github_event.ex

@@ -6,9 +6,11 @@ defmodule CodeCorps.GithubEvent do
 
   schema "github_events" do
     field :action, :string
+    field :data, :string
     field :failure_reason, :string
     field :github_delivery_id, :string
     field :payload, :map
+    field :error, :string
     field :status, :string
     field :type, :string
 
@@ -20,7 +22,7 @@ defmodule CodeCorps.GithubEvent do
   """
   def changeset(struct, params \\ %{}) do
     struct
-    |> cast(params, [:action, :github_delivery_id, :payload, :status, :type])
+    |> cast(params, [:action, :data, :github_delivery_id, :payload, :error, :status, :type])
     |> validate_required([:action, :github_delivery_id, :payload, :status, :type])
   end
 end

lib/code_corps/sentry/async.ex

@@ -0,0 +1,6 @@
+defmodule CodeCorps.Sentry.Async do
+  def capture_exception(exception, opts \\ []) do
+    exception
+    |> Sentry.capture_exception(opts)
+  end
+end

lib/code_corps/sentry/sentry.ex

@@ -0,0 +1,7 @@
+defmodule CodeCorps.Sentry do
+  @sentry Application.get_env(:code_corps, :sentry)
+
+  def capture_exception(exception, opts \\ []) do
+    @sentry.capture_exception(exception, opts)
+  end
+end

lib/code_corps/sentry/sync.ex

@@ -0,0 +1,6 @@
+defmodule CodeCorps.Sentry.Sync do
+  def capture_exception(exception, opts \\ []) do
+    exception
+    |> Sentry.capture_exception(opts |> Keyword.put(:result, :sync))
+  end
+end

lib/code_corps_web/views/github_event_view.ex

@@ -4,8 +4,8 @@ defmodule CodeCorpsWeb.GithubEventView do
   use JaSerializer.PhoenixView
 
   attributes [
-    :action, :event_type, :failure_reason, :github_delivery_id, :inserted_at,
-    :payload, :status, :updated_at
+    :action, :data, :event_type, :error, :failure_reason, :github_delivery_id,
+    :inserted_at, :payload, :status, :updated_at
   ]
 
   def event_type(github_event, _conn) do

priv/repo/migrations/20171115225358_add_serialized_error_to_github_events.exs

@@ -0,0 +1,10 @@
+defmodule CodeCorps.Repo.Migrations.AddSerializedErrorToGithubEvents do
+  use Ecto.Migration
+
+  def change do
+    alter table(:github_events) do
+      add :data, :text
+      add :error, :text
+    end
+  end
+end

priv/repo/structure.sql

@@ -284,7 +284,9 @@ CREATE TABLE github_events (
     inserted_at timestamp without time zone NOT NULL,
     updated_at timestamp without time zone NOT NULL,
     payload jsonb,
-    failure_reason character varying(255)
+    failure_reason character varying(255),
+    data text,
+    error text
 );
 
 
@@ -3799,5 +3801,5 @@ ALTER TABLE ONLY users
 -- PostgreSQL database dump complete
 --
 
-INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170505224222), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136), (20170706132431), (20170707213648), (20170711122252), (20170717092127), (20170725060612), (20170727052644), (20170731130121), (20170814131722), (20170913114958), (20170921014405), (20170925214512), (20170925230419), (20170926134646), (20170927100300), (20170928234412), (20171003134956), (20171003225853), (20171006063358), (20171006161407), (20171012215106), (20171012221231), (20171016125229), (20171016125516), (20171016223356), (20171016235656), (20171017235433), (20171019191035), (20171025184225), (20171026010933), (20171027061833), (20171028011642), (20171028173508), (20171030182857), (20171031232023), (20171031234356), (20171101023309), (20171104013543), (20171106045740), (20171106050209), (20171106103153), (20171106200036), (20171109231538), (20171110001134), (20171114010851), (20171114033357), (20171114225214), (20171114225713), (20171114232534), (20171115201624);
+INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170505224222), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136), (20170706132431), (20170707213648), (20170711122252), (20170717092127), (20170725060612), (20170727052644), (20170731130121), (20170814131722), (20170913114958), (20170921014405), (20170925214512), (20170925230419), (20170926134646), (20170927100300), (20170928234412), (20171003134956), (20171003225853), (20171006063358), (20171006161407), (20171012215106), (20171012221231), (20171016125229), (20171016125516), (20171016223356), (20171016235656), (20171017235433), (20171019191035), (20171025184225), (20171026010933), (20171027061833), (20171028011642), (20171028173508), (20171030182857), (20171031232023), (20171031234356), (20171101023309), (20171104013543), (20171106045740), (20171106050209), (20171106103153), (20171106200036), (20171109231538), (20171110001134), (20171114010851), (20171114033357), (20171114225214), (20171114225713), (20171114232534), (20171115201624), (20171115225358);
 

test/lib/code_corps/github/event/issue_comment/issue_comment_test.exs

@@ -35,7 +35,7 @@ defmodule CodeCorps.GitHub.Event.IssueCommentTest do
       end
 
       test "returns error if unmatched repository" do
-        assert IssueComment.handle(@payload) == {:error, :repo_not_found}
+        assert IssueComment.handle(@payload) == {:error, :repo_not_found, %{}}
         refute Repo.one(User)
       end
 

test/lib/code_corps/github/event/issues/issues_test.exs

@@ -35,7 +35,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
       end
 
       test "returns error if unmatched repository" do
-        assert Issues.handle(@payload) == {:error, :repo_not_found}
+        assert Issues.handle(@payload) == {:error, :repo_not_found, %{}}
         refute Repo.one(User)
       end
 

test/lib/code_corps/github/event/pull_request/pull_request_test.exs

@@ -36,7 +36,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
       end
 
       test "returns error if unmatched repository" do
-        assert PullRequest.handle(@payload) == {:error, :repo_not_found}
+        assert PullRequest.handle(@payload) == {:error, :repo_not_found, %{}}
         refute Repo.one(User)
       end
 

test/lib/code_corps/github/event_test.exs

@@ -25,7 +25,7 @@ defmodule CodeCorps.GitHub.EventTest do
 
     test "marks event errored, with failure_reason, if resulting tuple starts with :error" do
       event = insert(:github_event, status: "processing")
-      {:ok, %GithubEvent{} = updated_event} = Event.stop_processing({:error, :bar}, event)
+      {:ok, %GithubEvent{} = updated_event} = Event.stop_processing({:error, :bar, %{}}, event)
       assert updated_event.status == "errored"
       assert updated_event.failure_reason == "bar"
     end

test/lib/code_corps/github/sync/sync_test.exs

@@ -144,7 +144,7 @@ defmodule CodeCorps.GitHub.SyncTest do
       github_repo = setup_coderly_repo()
 
       with_real_api do
-          Sync.sync_repo(github_repo)
+        Sync.sync_repo(github_repo)
       end
 
       repo = Repo.one(GithubRepo)

test/lib/code_corps/github/webhook/handler_test.exs

@@ -4,11 +4,13 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do
   use CodeCorps.DbAccessCase
 
   import CodeCorps.GitHub.TestHelpers
+  import CodeCorps.TestEnvironmentHelper
 
   alias CodeCorps.{
     GithubEvent,
     GitHub.Webhook.Handler,
-    Repo
+    Repo,
+    Task
   }
 
   defp setup_repo(github_repo_id) do
@@ -240,6 +242,50 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do
     end
   end
 
+  describe "handle_supported/3 when there are errors" do
+    test "serializes error output" do
+      %{"repository" => %{"id" => github_repo_id}}
+        = opened_payload = load_event_fixture("issues_opened")
+
+      setup_repo(github_repo_id)
+
+      {:ok, %GithubEvent{}} = Handler.handle_supported("issues", "abc-123", opened_payload)
+
+      edited_payload = load_event_fixture("issues_edited")
+
+      edited_payload =
+        edited_payload
+        |> put_in(["issue", "updated_at"], "2006-05-05T23:40:28Z")
+
+      task = Repo.one(Task)
+      changeset = Task.update_changeset(task, %{title: "New title", updated_from: "codecorps"})
+       Repo.update!(changeset)
+
+      bypass = Bypass.open
+      Bypass.expect bypass, fn conn ->
+        {:ok, body, conn} = Plug.Conn.read_body(conn)
+        assert body =~ "GitHubEventError"
+        assert body =~ "CodeCorps"
+        assert conn.request_path == "/api/1/store/"
+        assert conn.method == "POST"
+        Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>)
+      end
+
+      modify_env(:sentry, environment_name: :prod)
+      modify_env(:sentry, dsn: "http://public:secret@localhost:#{bypass.port}/1")
+
+      {:ok, %GithubEvent{} = event} = Handler.handle_supported("issues", "abc-456", edited_payload)
+
+      assert event.action == "edited"
+      assert event.github_delivery_id == "abc-456"
+      assert event.data == Repo.one(Task) |> Kernel.inspect(pretty: true)
+      assert event.error # This is difficult to test, so just assert presence
+      assert event.payload == edited_payload
+      assert event.status == "errored"
+      assert event.type == "issues"
+    end
+  end
+
   describe "handle_unsupported/3" do
     [
       {"installation", "deleted"},

test/lib/code_corps_web/views/github_event_view_test.exs

@@ -14,7 +14,9 @@ defmodule CodeCorpsWeb.GithubEventViewTest do
         "type" => "github-event",
         "attributes" => %{
           "action" => github_event.action,
+          "data" => github_event.data,
           "event-type" => github_event.type,
+          "error" => github_event.error,
           "failure-reason" => github_event.failure_reason,
           "github-delivery-id" => github_event.github_delivery_id,
           "inserted-at" => github_event.inserted_at,

test/support/test_environment_helper.ex

@@ -0,0 +1,16 @@
+defmodule CodeCorps.TestEnvironmentHelper do
+  def modify_env(app, overrides) do
+    original_env = Application.get_all_env(app)
+    Enum.each(overrides, fn {key, value} -> Application.put_env(app, key, value) end)
+
+    ExUnit.Callbacks.on_exit(fn ->
+      Enum.each overrides, fn {key, _} ->
+        if Keyword.has_key?(original_env, key) do
+          Application.put_env(app, key, Keyword.fetch!(original_env, key))
+        else
+          Application.delete_env(app, key)
+        end
+      end
+    end)
+  end
+end