Skip to content

Commit

Permalink
Save changeset errors and data for GitHub webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
joshsmith committed Nov 16, 2017
1 parent f82f9cd commit 7b77518
Show file tree
Hide file tree
Showing 19 changed files with 151 additions and 28 deletions.
2 changes: 2 additions & 0 deletions config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 25 additions & 2 deletions lib/code_corps/github/event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand All @@ -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
29 changes: 14 additions & 15 deletions lib/code_corps/github/sync/sync.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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
4 changes: 3 additions & 1 deletion lib/code_corps/model/github_event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
6 changes: 6 additions & 0 deletions lib/code_corps/sentry/async.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
defmodule CodeCorps.Sentry.Async do
def capture_exception(exception, opts \\ []) do
exception
|> Sentry.capture_exception(opts)
end
end
7 changes: 7 additions & 0 deletions lib/code_corps/sentry/sentry.ex
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions lib/code_corps/sentry/sync.ex
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions lib/code_corps_web/views/github_event_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
6 changes: 4 additions & 2 deletions priv/repo/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
);


Expand Down Expand Up @@ -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);

Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion test/lib/code_corps/github/event/issues/issues_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion test/lib/code_corps/github/event_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/lib/code_corps/github/sync/sync_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 47 additions & 1 deletion test/lib/code_corps/github/webhook/handler_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"},
Expand Down
2 changes: 2 additions & 0 deletions test/lib/code_corps_web/views/github_event_view_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions test/support/test_environment_helper.ex
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7b77518

Please sign in to comment.