diff --git a/config/config.exs b/config/config.exs index c9139ea9b..e533809a3 100644 --- a/config/config.exs +++ b/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 diff --git a/config/test.exs b/config/test.exs index cd7b47a50..2151e584f 100644 --- a/config/test.exs +++ b/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, diff --git a/lib/code_corps/github/event.ex b/lib/code_corps/github/event.ex index 2732b2bef..4879665f4 100644 --- a/lib/code_corps/github/event.ex +++ b/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 diff --git a/lib/code_corps/github/sync/sync.ex b/lib/code_corps/github/sync/sync.ex index a606bb3d1..9981a4393 100644 --- a/lib/code_corps/github/sync/sync.ex +++ b/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 diff --git a/lib/code_corps/model/github_event.ex b/lib/code_corps/model/github_event.ex index 43100a017..13b837333 100644 --- a/lib/code_corps/model/github_event.ex +++ b/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 diff --git a/lib/code_corps/sentry/async.ex b/lib/code_corps/sentry/async.ex new file mode 100644 index 000000000..7c8fac25c --- /dev/null +++ b/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 diff --git a/lib/code_corps/sentry/sentry.ex b/lib/code_corps/sentry/sentry.ex new file mode 100644 index 000000000..de3d30e33 --- /dev/null +++ b/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 diff --git a/lib/code_corps/sentry/sync.ex b/lib/code_corps/sentry/sync.ex new file mode 100644 index 000000000..1e8901037 --- /dev/null +++ b/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 diff --git a/lib/code_corps_web/views/github_event_view.ex b/lib/code_corps_web/views/github_event_view.ex index 87e866734..a8e647922 100644 --- a/lib/code_corps_web/views/github_event_view.ex +++ b/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 diff --git a/priv/repo/migrations/20171115225358_add_serialized_error_to_github_events.exs b/priv/repo/migrations/20171115225358_add_serialized_error_to_github_events.exs new file mode 100644 index 000000000..a21301b0d --- /dev/null +++ b/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 diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index 48f229b26..ae231fd31 100644 --- a/priv/repo/structure.sql +++ b/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); diff --git a/test/lib/code_corps/github/event/issue_comment/issue_comment_test.exs b/test/lib/code_corps/github/event/issue_comment/issue_comment_test.exs index a6d46917d..1a15fb3bc 100644 --- a/test/lib/code_corps/github/event/issue_comment/issue_comment_test.exs +++ b/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 diff --git a/test/lib/code_corps/github/event/issues/issues_test.exs b/test/lib/code_corps/github/event/issues/issues_test.exs index e943bc55f..0d24d044c 100644 --- a/test/lib/code_corps/github/event/issues/issues_test.exs +++ b/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 diff --git a/test/lib/code_corps/github/event/pull_request/pull_request_test.exs b/test/lib/code_corps/github/event/pull_request/pull_request_test.exs index 7631e1899..3ec90cdca 100644 --- a/test/lib/code_corps/github/event/pull_request/pull_request_test.exs +++ b/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 diff --git a/test/lib/code_corps/github/event_test.exs b/test/lib/code_corps/github/event_test.exs index 135397207..af7f47c9a 100644 --- a/test/lib/code_corps/github/event_test.exs +++ b/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 diff --git a/test/lib/code_corps/github/sync/sync_test.exs b/test/lib/code_corps/github/sync/sync_test.exs index 57bb58db0..ff45b1a64 100644 --- a/test/lib/code_corps/github/sync/sync_test.exs +++ b/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) diff --git a/test/lib/code_corps/github/webhook/handler_test.exs b/test/lib/code_corps/github/webhook/handler_test.exs index fed2a2410..b7c5ef0d8 100644 --- a/test/lib/code_corps/github/webhook/handler_test.exs +++ b/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"}, diff --git a/test/lib/code_corps_web/views/github_event_view_test.exs b/test/lib/code_corps_web/views/github_event_view_test.exs index 851dd710c..2c6fe7845 100644 --- a/test/lib/code_corps_web/views/github_event_view_test.exs +++ b/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, diff --git a/test/support/test_environment_helper.ex b/test/support/test_environment_helper.ex new file mode 100644 index 000000000..cac9c2493 --- /dev/null +++ b/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