From 2ec7457783d918364c9c661ff5f85f259110bfa7 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sat, 26 Jun 2021 17:22:34 +0200 Subject: [PATCH 1/5] Handle actor being something else than Group or Person when deleting it Signed-off-by: Thomas Citharel --- lib/mobilizon/actors/actors.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index 2d5d9c409..a86e25b58 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/lib/mobilizon/actors/actors.ex @@ -360,6 +360,9 @@ defmodule Mobilizon.Actors do Multi.run(multi, :reset_default_actor_id, fn _, _ -> reset_default_actor_id(actor) end) + + _ -> + multi end multi = From b1eeebe05ad3fb5e1e93bd31152d36c61e757bc8 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sat, 26 Jun 2021 17:52:07 +0200 Subject: [PATCH 2/5] Default to UTC when no timezone in user settings in recaps Signed-off-by: Thomas Citharel --- lib/service/workers/send_activity_recap_worker.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/service/workers/send_activity_recap_worker.ex b/lib/service/workers/send_activity_recap_worker.ex index 5145ad778..c1a52a262 100644 --- a/lib/service/workers/send_activity_recap_worker.ex +++ b/lib/service/workers/send_activity_recap_worker.ex @@ -96,7 +96,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do timezone: timezone } }) do - is_between_hours(timezone: timezone) + is_between_hours(timezone: timezone || "Etc/UTC") end # If we're on the first day of the week between notification hours @@ -107,6 +107,6 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do timezone: timezone } }) do - is_between_hours_on_first_day(timezone: timezone, locale: locale) + is_between_hours_on_first_day(timezone: timezone || "Etc/UTC", locale: locale) end end From 93297931bbd71f2d593432f2da836ec97154581c Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sat, 26 Jun 2021 17:56:42 +0200 Subject: [PATCH 3/5] Disable Cldr warning that it didn't match any language Signed-off-by: Thomas Citharel --- .sobelow-skips | 5 ++++- lib/web/router.ex | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.sobelow-skips b/.sobelow-skips index 0d0acdd80..dbe9da3e0 100644 --- a/.sobelow-skips +++ b/.sobelow-skips @@ -3,4 +3,7 @@ 752C0E897CA81ACD81F4BB215FA5F8E4 23412CF16549E4E88366DC9DECF39071 -81C1F600C5809C7029EE32DE4818CD7D \ No newline at end of file +81C1F600C5809C7029EE32DE4818CD7D +155A1FB53DE39EC8EFCFD7FB94EA823D +73B351E4CB3AF715AD450A085F5E6304 +BBACD7F0BACD4A6D3010C26604671692 \ No newline at end of file diff --git a/lib/web/router.ex b/lib/web/router.ex index 4b54a1350..3f5d7b7a7 100644 --- a/lib/web/router.ex +++ b/lib/web/router.ex @@ -37,7 +37,8 @@ defmodule Mobilizon.Web.Router do plug(:put_secure_browser_headers) plug(Cldr.Plug.AcceptLanguage, - cldr_backend: Mobilizon.Cldr + cldr_backend: Mobilizon.Cldr, + no_match_log_level: :debug ) end @@ -50,7 +51,8 @@ defmodule Mobilizon.Web.Router do plug(Plug.Static, at: "/", from: "priv/static") plug(Cldr.Plug.AcceptLanguage, - cldr_backend: Mobilizon.Cldr + cldr_backend: Mobilizon.Cldr, + no_match_log_level: :debug ) plug(:accepts, ["html"]) From 3ed25bab818e459bc7623fe50ace56964b0f545d Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sun, 27 Jun 2021 11:21:34 +0200 Subject: [PATCH 4/5] Send notifications to event organizer when new comment is posted Signed-off-by: Thomas Citharel --- lib/mobilizon/events/events.ex | 11 +++++ lib/service/activity/comment.ex | 19 ++++++-- lib/service/activity/renderer/comment.ex | 41 +++++++++++++++++ lib/service/notifier/email.ex | 37 +++++++-------- lib/service/notifier/filter.ex | 10 +++- .../workers/legacy_notifier_builder.ex | 14 ++++++ .../activity/_comment_activity_item.html.eex | 33 +++++++++++++ .../activity/_comment_activity_item.text.eex | 46 ++++++------------- test/service/activity/comment_test.exs | 14 ++++-- 9 files changed, 161 insertions(+), 64 deletions(-) diff --git a/lib/mobilizon/events/events.ex b/lib/mobilizon/events/events.ex index 997c41e37..f1a99c06a 100644 --- a/lib/mobilizon/events/events.ex +++ b/lib/mobilizon/events/events.ex @@ -214,6 +214,17 @@ defmodule Mobilizon.Events do |> Repo.exists?() end + @doc """ + Gets an event by its UUID, with all associations loaded. + """ + @spec get_event_by_uuid_with_preload(String.t()) :: Event.t() | nil + def get_event_by_uuid_with_preload(uuid) do + uuid + |> event_by_uuid_query() + |> preload_for_event() + |> Repo.one() + end + @doc """ Gets an event by its UUID, with all associations loaded. """ diff --git a/lib/service/activity/comment.ex b/lib/service/activity/comment.ex index 6d3993483..b2d8b4067 100644 --- a/lib/service/activity/comment.ex +++ b/lib/service/activity/comment.ex @@ -159,10 +159,11 @@ defmodule Mobilizon.Service.Activity.Comment do %Comment{ actor_id: actor_id, in_reply_to_comment_id: in_reply_to_comment_id, - id: comment_id - }, + id: comment_id, + uuid: comment_uuid + } = comment, %Event{ - uuid: uuid, + uuid: event_uuid, title: title, attributed_to: nil, organizer_actor_id: organizer_actor_id @@ -174,8 +175,10 @@ defmodule Mobilizon.Service.Activity.Comment do "subject" => :event_new_comment, "subject_params" => %{ event_title: title, - event_uuid: uuid, - comment_reply_to: !is_nil(in_reply_to_comment_id) + event_uuid: event_uuid, + comment_reply_to: !is_nil(in_reply_to_comment_id), + comment_uuid: comment_uuid, + comment_reply_to_uuid: reply_to_comment_uuid(comment) }, "author_id" => actor_id, "object_id" => to_string(comment_id) @@ -185,4 +188,10 @@ defmodule Mobilizon.Service.Activity.Comment do end defp notify(_, _, _, _), do: {:ok, :skipped} + + @spec reply_to_comment_uuid(Comment.t()) :: String.t() | nil + defp reply_to_comment_uuid(%Comment{in_reply_to_comment: %Comment{uuid: comment_reply_to_uuid}}), + do: comment_reply_to_uuid + + defp reply_to_comment_uuid(%Comment{in_reply_to_comment: _}), do: nil end diff --git a/lib/service/activity/renderer/comment.ex b/lib/service/activity/renderer/comment.ex index 6689a7d8d..87f62796c 100644 --- a/lib/service/activity/renderer/comment.ex +++ b/lib/service/activity/renderer/comment.ex @@ -45,6 +45,35 @@ defmodule Mobilizon.Service.Activity.Renderer.Comment do ), url: event_url(activity) } + + :event_new_comment -> + if activity.subject_params["comment_reply_to"] do + %{ + body: + dgettext( + "activity", + "%{profile} has posted a new reply under your event %{event}.", + %{ + profile: profile, + event: event_title(activity) + } + ), + url: event_comment_url(activity) + } + else + %{ + body: + dgettext( + "activity", + "%{profile} has posted a new comment under your event %{event}.", + %{ + profile: profile, + event: event_title(activity) + } + ), + url: event_comment_url(activity) + } + end end end @@ -56,6 +85,18 @@ defmodule Mobilizon.Service.Activity.Renderer.Comment do ) end + defp event_comment_url(activity) do + "#{event_url(activity)}#comment-#{comment_uuid(activity)}" + end + + defp comment_uuid(activity) do + if activity.subject_params["comment_reply_to"] do + "#{activity.subject_params["reply_to_comment_uuid"]}-#{activity.subject_params["comment_uuid"]}" + else + activity.subject_params["comment_uuid"] + end + end + defp profile(activity), do: Actor.display_name_and_username(activity.author) defp event_title(activity), do: activity.subject_params["event_title"] end diff --git a/lib/service/notifier/email.ex b/lib/service/notifier/email.ex index 5a8b3c7f2..304074696 100644 --- a/lib/service/notifier/email.ex +++ b/lib/service/notifier/email.ex @@ -56,7 +56,8 @@ defmodule Mobilizon.Service.Notifier.Email do @always_direct_subjects [ :participation_event_comment, :event_comment_mention, - :discussion_mention + :discussion_mention, + :event_new_comment ] @spec can_send_activity?(Activity.t(), User.t(), Keyword.t()) :: boolean() @@ -96,30 +97,24 @@ defmodule Mobilizon.Service.Notifier.Email do when subject in @always_direct_subjects, do: true + # First notification EVER! + defp match_group_notifications_setting(:one_hour, _, last_notification_sent, _) + when is_nil(last_notification_sent), + do: true + + # Delay ok since last notification + defp match_group_notifications_setting(:one_hour, _, %DateTime{} = last_notification_sent, _) do + is_delay_ok_since_last_notification_sent(last_notification_sent) + end + + # This is a recap defp match_group_notifications_setting( - group_notifications, + _group_notifications, _subject, - last_notification_sent, + _last_notification_sent, options ) do - cond do - # This is a recap - Keyword.get(options, :recap, false) != false -> - true - - # First notification EVER! - group_notifications == :one_hour && is_nil(last_notification_sent) -> - true - - # Delay ok since last notification - group_notifications == :one_hour && - is_delay_ok_since_last_notification_sent(last_notification_sent) -> - true - - # Otherwise, no thanks - true -> - false - end + Keyword.get(options, :recap, false) != false end @default_behavior %{ diff --git a/lib/service/notifier/filter.ex b/lib/service/notifier/filter.ex index 8370247dd..c6af546c2 100644 --- a/lib/service/notifier/filter.ex +++ b/lib/service/notifier/filter.ex @@ -23,10 +23,13 @@ defmodule Mobilizon.Service.Notifier.Filter do defp enabled?(nil, activity_setting, get_default), do: get_default.(activity_setting) defp enabled?(%ActivitySetting{enabled: enabled}, _activity_setting, _get_default), do: enabled - # Comment mention + # Mention defp map_activity_to_activity_setting(%Activity{subject: :event_comment_mention}), do: "event_comment_mention" + defp map_activity_to_activity_setting(%Activity{subject: :discussion_mention}), + do: "discussion_mention" + # Participation @spec map_activity_to_activity_setting(Activity.t()) :: String.t() | false defp map_activity_to_activity_setting(%Activity{subject: :participation_event_updated}), @@ -42,6 +45,9 @@ defmodule Mobilizon.Service.Notifier.Filter do defp map_activity_to_activity_setting(%Activity{subject: :event_new_participation}), do: "event_new_participation" + defp map_activity_to_activity_setting(%Activity{subject: :event_new_comment}), + do: "event_new_comment" + # Event defp map_activity_to_activity_setting(%Activity{subject: :event_created}), do: "event_created" defp map_activity_to_activity_setting(%Activity{type: :event}), do: "event_updated" @@ -60,7 +66,7 @@ defmodule Mobilizon.Service.Notifier.Filter do defp map_activity_to_activity_setting(%Activity{subject: :member_request}), do: "member_request" - defp map_activity_to_activity_setting(%Activity{type: :member}), do: "member" + defp map_activity_to_activity_setting(%Activity{type: :member}), do: "member_updated" defp map_activity_to_activity_setting(_), do: false end diff --git a/lib/service/workers/legacy_notifier_builder.ex b/lib/service/workers/legacy_notifier_builder.ex index 25018b92f..83ebe0399 100644 --- a/lib/service/workers/legacy_notifier_builder.ex +++ b/lib/service/workers/legacy_notifier_builder.ex @@ -5,6 +5,8 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do alias Mobilizon.Activities.Activity alias Mobilizon.{Actors, Events, Users} + alias Mobilizon.Actors.Actor + alias Mobilizon.Events.Event alias Mobilizon.Service.Notifier use Mobilizon.Service.Workers.Helper, queue: "activity" @@ -66,6 +68,18 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do |> users_from_actor_ids(Keyword.fetch!(options, :author_id)) end + defp users_to_notify( + %{"subject" => "event_new_comment", "subject_params" => %{"event_uuid" => event_uuid}}, + options + ) do + event_uuid + |> Events.get_event_by_uuid_with_preload() + |> (fn %Event{organizer_actor: %Actor{id: actor_id}} -> [actor_id] end).() + |> users_from_actor_ids(Keyword.fetch!(options, :author_id)) + end + + defp users_to_notify(_, _), do: [] + @spec users_from_actor_ids(list(), integer() | String.t()) :: list(Users.t()) defp users_from_actor_ids(actor_ids, author_id) do actor_ids diff --git a/lib/web/templates/email/activity/_comment_activity_item.html.eex b/lib/web/templates/email/activity/_comment_activity_item.html.eex index df908b9c9..4a9abd54b 100644 --- a/lib/web/templates/email/activity/_comment_activity_item.html.eex +++ b/lib/web/templates/email/activity/_comment_activity_item.html.eex @@ -29,4 +29,37 @@ " } ) |> raw %> + + <% :event_new_comment -> %> + <%= if @activity.subject_params["comment_reply_to"] do %> + <%= + dgettext("activity", "%{profile} has posted a new reply under your event %{event}.", + %{ + profile: "#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}", + event: " URI.decode()}#comment-#{@activity.subject_params["comment_reply_to_uuid"]}-#{@activity.subject_params["comment_uuid"]}\"> + #{@activity.subject_params["event_title"]} + " + } + ) |> raw %> + <% else %> + <%= + dgettext("activity", "%{profile} has posted a new comment under your event %{event}.", + %{ + profile: "#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}", + event: " URI.decode()}#comment-#{@activity.subject_params["comment_uuid"]}\"> + #{@activity.subject_params["event_title"]} + " + } + ) |> raw %> + <% end %> <% end %> \ No newline at end of file diff --git a/lib/web/templates/email/activity/_comment_activity_item.text.eex b/lib/web/templates/email/activity/_comment_activity_item.text.eex index 7742f369a..145a3d7c1 100644 --- a/lib/web/templates/email/activity/_comment_activity_item.text.eex +++ b/lib/web/templates/email/activity/_comment_activity_item.text.eex @@ -1,34 +1,4 @@ -<%= case @activity.subject do %><% :discussion_created -> %><%= dgettext("activity", "%{profile} created the discussion %{discussion}.", - %{ - profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), - discussion: @activity.subject_params["discussion_title"] - } -) %> -<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_replied -> %><%= dgettext("activity", "%{profile} replied to the discussion %{discussion}.", - %{ - profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), - discussion: @activity.subject_params["discussion_title"] - } -) %> -<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_renamed -> %><%= dgettext("activity", "%{profile} renamed the discussion %{discussion}.", - %{ - profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), - discussion: @activity.subject_params["discussion_title"] - } -) %> -<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_archived -> %><%= dgettext("activity", "%{profile} archived the discussion %{discussion}.", - %{ - profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), - discussion: @activity.subject_params["discussion_title"] - } -) %> -<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_deleted -> %><%= dgettext("activity", "%{profile} deleted the discussion %{discussion}.", - %{ - profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), - discussion: @activity.subject_params["discussion_title"] - } -) %> -<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :event_comment_mention -> %><%= dgettext("activity", "%{profile} mentionned you in a comment under event %{event}.", +<%= case @activity.subject do %><% :event_comment_mention -> %><%= dgettext("activity", "%{profile} mentionned you in a comment under event %{event}.", %{ profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), event: @activity.subject_params["event_title"] @@ -40,4 +10,16 @@ event: @activity.subject_params["event_title"] } ) %> -<%= page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode() %><% end %> \ No newline at end of file +<%= page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode() %><% :event_new_comment -> %><%= if @activity.subject_params["comment_reply_to"] do %><%=dgettext("activity", "%{profile} has posted a new reply under your event %{event}.", + %{ + profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), + event: @activity.subject_params["event_title"] + } +) %> +<%= "#{page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode()}#comment-#{@activity.subject_params["comment_reply_to_uuid"]}-#{@activity.subject_params["comment_uuid"]}" %><% else %><%= dgettext("activity", "%{profile} has posted a new comment under your event %{event}.", +%{ + profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author), + event: @activity.subject_params["event_title"] +} +) %> +<%= "#{page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode()}#comment-#{@activity.subject_params["comment_uuid"]}"%><% end %><% end %> \ No newline at end of file diff --git a/test/service/activity/comment_test.exs b/test/service/activity/comment_test.exs index 95118a469..5e4ae6233 100644 --- a/test/service/activity/comment_test.exs +++ b/test/service/activity/comment_test.exs @@ -18,7 +18,9 @@ defmodule Mobilizon.Service.Activity.CommentTest do describe "handle comment with mentions" do test "with no mentions" do %Event{title: event_title, uuid: event_uuid} = event = insert(:event) - %Comment{id: comment_id, actor_id: author_id} = comment = insert(:comment, event: event) + + %Comment{id: comment_id, actor_id: author_id, uuid: comment_uuid} = + comment = insert(:comment, event: event) assert {:ok, [organizer: :enqueued, announcement: :skipped, mentionned: :skipped]} == CommentActivity.insert_activity(comment) @@ -37,9 +39,11 @@ defmodule Mobilizon.Service.Activity.CommentTest do "op" => "legacy_notify", "subject" => "event_new_comment", "subject_params" => %{ + "comment_reply_to_uuid" => nil, "event_title" => event_title, "event_uuid" => event_uuid, - "comment_reply_to" => false + "comment_reply_to" => false, + "comment_uuid" => comment_uuid }, "type" => "comment" } @@ -51,7 +55,7 @@ defmodule Mobilizon.Service.Activity.CommentTest do %Actor{id: actor_id} = actor = insert(:actor, user: user) %Event{uuid: event_uuid, title: event_title} = event = insert(:event) - %Comment{id: comment_id, actor_id: author_id} = + %Comment{id: comment_id, actor_id: author_id, uuid: comment_uuid} = comment = insert(:comment, text: "Hey @you", event: event) comment = %Comment{ @@ -90,9 +94,11 @@ defmodule Mobilizon.Service.Activity.CommentTest do "op" => "legacy_notify", "subject" => "event_new_comment", "subject_params" => %{ + "comment_reply_to_uuid" => nil, "event_title" => event_title, "event_uuid" => event_uuid, - "comment_reply_to" => false + "comment_reply_to" => false, + "comment_uuid" => comment_uuid }, "type" => "comment" } From 7ec6f158ec409165e2c66b0ef46cf214d674fa87 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sun, 27 Jun 2021 13:15:24 +0200 Subject: [PATCH 5/5] Add wrapper to Sentry to not load it when not configured Signed-off-by: Thomas Citharel --- config/config.exs | 2 +- lib/federation/activity_pub/fetcher.ex | 1 + lib/federation/activity_pub/refresher.ex | 1 + lib/federation/activity_pub/transmogrifier.ex | 1 + lib/federation/http_signatures/signature.ex | 1 + lib/mobilizon.ex | 7 +-- lib/mobilizon/actors/actors.ex | 1 + lib/service/error_reporter.ex | 17 ------- .../error_reporting/error_reporting.ex | 35 +++++++++++++++ lib/service/error_reporting/sentry.ex | 45 +++++++++++++++++++ lib/web/auth/context.ex | 6 ++- lib/web/email/mailer.ex | 1 + lib/web/endpoint.ex | 8 +++- 13 files changed, 102 insertions(+), 24 deletions(-) delete mode 100644 lib/service/error_reporter.ex create mode 100644 lib/service/error_reporting/error_reporting.ex create mode 100644 lib/service/error_reporting/sentry.ex diff --git a/config/config.exs b/config/config.exs index 15f93c26e..177bcc9bf 100644 --- a/config/config.exs +++ b/config/config.exs @@ -113,7 +113,7 @@ config :mobilizon, Mobilizon.Web.Email.Mailer, # Configures Elixir's Logger config :logger, :console, - backends: [:console, Sentry.LoggerBackend], + backends: [:console], format: "$time $metadata[$level] $message\n", metadata: [:request_id] diff --git a/lib/federation/activity_pub/fetcher.ex b/lib/federation/activity_pub/fetcher.ex index 019e0237e..7bac0fee3 100644 --- a/lib/federation/activity_pub/fetcher.ex +++ b/lib/federation/activity_pub/fetcher.ex @@ -9,6 +9,7 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do alias Mobilizon.Federation.HTTPSignatures.Signature alias Mobilizon.Federation.ActivityPub.{Relay, Transmogrifier} alias Mobilizon.Federation.ActivityStream.Converter.Actor, as: ActorConverter + alias Mobilizon.Service.ErrorReporting.Sentry alias Mobilizon.Service.HTTP.ActivityPub, as: ActivityPubClient import Mobilizon.Federation.ActivityPub.Utils, diff --git a/lib/federation/activity_pub/refresher.ex b/lib/federation/activity_pub/refresher.ex index 57399a69a..7385da96e 100644 --- a/lib/federation/activity_pub/refresher.ex +++ b/lib/federation/activity_pub/refresher.ex @@ -8,6 +8,7 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do alias Mobilizon.Federation.ActivityPub alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor alias Mobilizon.Federation.ActivityPub.{Fetcher, Relay, Transmogrifier, Utils} + alias Mobilizon.Service.ErrorReporting.Sentry require Logger @doc """ diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index b2430899f..172673e0c 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -21,6 +21,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor alias Mobilizon.Federation.ActivityPub.Types.Ownable alias Mobilizon.Federation.ActivityStream.{Converter, Convertible} + alias Mobilizon.Service.ErrorReporting.Sentry alias Mobilizon.Service.Workers.Background alias Mobilizon.Tombstone alias Mobilizon.Web.Email.Participation diff --git a/lib/federation/http_signatures/signature.ex b/lib/federation/http_signatures/signature.ex index ff703a3ed..e68251d3b 100644 --- a/lib/federation/http_signatures/signature.ex +++ b/lib/federation/http_signatures/signature.ex @@ -13,6 +13,7 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do alias Mobilizon.Actors.Actor alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor + alias Mobilizon.Service.ErrorReporting.Sentry require Logger diff --git a/lib/mobilizon.ex b/lib/mobilizon.ex index c754964ac..9734d802b 100644 --- a/lib/mobilizon.ex +++ b/lib/mobilizon.ex @@ -16,7 +16,7 @@ defmodule Mobilizon do alias Mobilizon.{Config, Storage, Web} alias Mobilizon.Federation.ActivityPub - alias Mobilizon.Service.{ErrorPage, ErrorReporter} + alias Mobilizon.Service.{ErrorPage, ErrorReporting} alias Mobilizon.Service.Export.{Feed, ICalendar} @name Mix.Project.config()[:name] @@ -66,13 +66,14 @@ defmodule Mobilizon do ] ++ task_children(@env) - Logger.add_backend(Sentry.LoggerBackend) + ErrorReporting.configure() + :ok = Oban.Telemetry.attach_default_logger() :telemetry.attach_many( "oban-errors", [[:oban, :job, :exception], [:oban, :circuit, :trip]], - &ErrorReporter.handle_event/4, + &ErrorReporting.handle_event/4, %{} ) diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index a86e25b58..5b1466f45 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/lib/mobilizon/actors/actors.ex @@ -16,6 +16,7 @@ defmodule Mobilizon.Actors do alias Mobilizon.Events.FeedToken alias Mobilizon.Federation.ActivityPub alias Mobilizon.Medias.File + alias Mobilizon.Service.ErrorReporting.Sentry alias Mobilizon.Service.Workers alias Mobilizon.Storage.{Page, Repo} alias Mobilizon.Users diff --git a/lib/service/error_reporter.ex b/lib/service/error_reporter.ex deleted file mode 100644 index 46900d945..000000000 --- a/lib/service/error_reporter.ex +++ /dev/null @@ -1,17 +0,0 @@ -defmodule Mobilizon.Service.ErrorReporter do - @moduledoc """ - Module to delegate all exceptions to Sentry - """ - def handle_event([:oban, :job, :exception], measure, %{job: job} = meta, _) do - extra = - job - |> Map.take([:id, :args, :meta, :queue, :worker]) - |> Map.merge(measure) - - Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: extra) - end - - def handle_event([:oban, :circuit, :trip], _measure, meta, _) do - Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: meta) - end -end diff --git a/lib/service/error_reporting/error_reporting.ex b/lib/service/error_reporting/error_reporting.ex new file mode 100644 index 000000000..6f9dd53db --- /dev/null +++ b/lib/service/error_reporting/error_reporting.ex @@ -0,0 +1,35 @@ +defmodule Mobilizon.Service.ErrorReporting do + @moduledoc """ + Mpdule to load and configure error reporting adapters + """ + + @callback enabled? :: boolean() + + @callback configure :: any() + + @callback handle_event(list(atom()), map(), map(), any()) :: any() + + @spec adapter :: module() | nil + def adapter do + adapter = Mobilizon.Config.get([__MODULE__, :adapter]) + if adapter && adapter.enabled?(), do: adapter, else: nil + end + + @spec handle_event(list(atom()), map(), map(), any()) :: any() + def handle_event(event_name, event_measurements, event_metadata, handler_config) do + adapter = adapter() + + if adapter do + adapter.handle_event(event_name, event_measurements, event_metadata, handler_config) + end + end + + @spec configure :: any() + def configure do + adapter = adapter() + + if adapter do + adapter.configure() + end + end +end diff --git a/lib/service/error_reporting/sentry.ex b/lib/service/error_reporting/sentry.ex new file mode 100644 index 000000000..d5aff6c5d --- /dev/null +++ b/lib/service/error_reporting/sentry.ex @@ -0,0 +1,45 @@ +defmodule Mobilizon.Service.ErrorReporting.Sentry do + @moduledoc """ + Sentry adapter for error reporting + """ + + alias Mobilizon.Service.ErrorReporting + @behaviour ErrorReporting + + @impl ErrorReporting + def enabled? do + !is_nil(Application.get_env(:sentry, :dsn)) + end + + @impl ErrorReporting + def configure do + Logger.add_backend(Sentry.LoggerBackend) + end + + def capture_message(message, opts \\ []) when is_binary(message) do + if enabled?() do + Sentry.capture_message(message, opts) + end + end + + def capture_exception(exception, opts \\ []) do + if enabled?() do + Sentry.capture_exception(exception, opts) + end + end + + @impl ErrorReporting + def handle_event([:oban, :job, :exception], measure, %{job: job} = meta, _) do + extra = + job + |> Map.take([:id, :args, :meta, :queue, :worker]) + |> Map.merge(measure) + + Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: extra) + end + + @impl ErrorReporting + def handle_event([:oban, :circuit, :trip], _measure, meta, _) do + Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: meta) + end +end diff --git a/lib/web/auth/context.ex b/lib/web/auth/context.ex index aaa16e25a..468a0d692 100644 --- a/lib/web/auth/context.ex +++ b/lib/web/auth/context.ex @@ -6,6 +6,7 @@ defmodule Mobilizon.Web.Auth.Context do import Plug.Conn + alias Mobilizon.Service.ErrorReporting.Sentry, as: SentryAdapter alias Mobilizon.Users.User def init(opts) do @@ -24,7 +25,10 @@ defmodule Mobilizon.Web.Auth.Context do context = case Guardian.Plug.current_resource(conn) do %User{id: user_id, email: user_email} = user -> - Sentry.Context.set_user_context(%{id: user_id, name: user_email}) + if SentryAdapter.enabled?() do + Sentry.Context.set_user_context(%{id: user_id, name: user_email}) + end + Map.put(context, :current_user, user) nil -> diff --git a/lib/web/email/mailer.ex b/lib/web/email/mailer.ex index 4996f7696..6f4693746 100644 --- a/lib/web/email/mailer.ex +++ b/lib/web/email/mailer.ex @@ -3,6 +3,7 @@ defmodule Mobilizon.Web.Email.Mailer do Mobilizon Mailer. """ use Bamboo.Mailer, otp_app: :mobilizon + alias Mobilizon.Service.ErrorReporting.Sentry def send_email_later(email) do Mobilizon.Web.Email.Mailer.deliver_later!(email) diff --git a/lib/web/endpoint.ex b/lib/web/endpoint.ex index 5d062f6a5..286ab9778 100644 --- a/lib/web/endpoint.ex +++ b/lib/web/endpoint.ex @@ -2,7 +2,10 @@ defmodule Mobilizon.Web.Endpoint do @moduledoc """ Endpoint for Mobilizon app """ - if Application.fetch_env!(:mobilizon, :env) !== :test do + alias Mobilizon.Service.ErrorReporting.Sentry, as: SentryAdapter + + if Application.fetch_env!(:mobilizon, :env) !== :test && + SentryAdapter.enabled?() do use Sentry.PlugCapture end @@ -92,7 +95,8 @@ defmodule Mobilizon.Web.Endpoint do String.replace_leading(url(), "http", "ws") end - if Application.fetch_env!(:mobilizon, :env) !== :test do + if Application.fetch_env!(:mobilizon, :env) !== :test && + SentryAdapter.enabled?() do plug(Sentry.PlugContext) end end