Don't report events if DSN is not configured (#655)

Author: whatyouhide

lib/mix/tasks/sentry.send_test_event.ex

@@ -47,7 +47,7 @@ defmodule Mix.Tasks.Sentry.SendTestEvent do
     Mix.shell().info("Client configuration:")
 
     if Config.dsn() do
-      {endpoint, public_key, secret_key} = Sentry.Transport.get_dsn()
+      {endpoint, public_key, secret_key} = Config.dsn()
       Mix.shell().info("server: #{endpoint}")
       Mix.shell().info("public_key: #{public_key}")
       Mix.shell().info("secret_key: #{secret_key}")

lib/sentry.ex

@@ -318,22 +318,23 @@ defmodule Sentry do
   > use cases, use `capture_exception/2` or `capture_message/2`.
   """
   @spec send_event(Event.t(), keyword()) :: send_result
-  def send_event(event, opts \\ [])
+  def send_event(event, opts \\ []) do
+    # TODO: remove on v11.0.0, :included_environments was deprecated in 10.0.0.
+    included_envs = Config.included_environments()
 
-  def send_event(%Event{message: nil, exception: []}, _opts) do
-    Logger.log(Config.log_level(), "Sentry: unable to parse exception")
+    cond do
+      is_nil(event.message) and event.exception == [] ->
+        Logger.log(Config.log_level(), "Sentry: unable to parse exception")
+        :ignored
 
-    :ignored
-  end
+      !Config.dsn() ->
+        :ignored
 
-  def send_event(%Event{} = event, opts) do
-    included_environments = Config.included_environments()
-    environment_name = to_string(Config.environment_name())
+      included_envs == :all or to_string(Config.environment_name()) in included_envs ->
+        Sentry.Client.send_event(event, opts)
 
-    if included_environments == :all or environment_name in included_environments do
-      Sentry.Client.send_event(event, opts)
-    else
-      :ignored
+      true ->
+        :ignored
     end
   end
 

lib/sentry/client.ex

@@ -224,9 +224,6 @@ defmodule Sentry.Client do
   defp maybe_log_send_result(send_result, %Event{}) do
     message =
       case send_result do
-        {:error, :invalid_dsn} ->
-          "Cannot send Sentry event because of invalid DSN"
-
         {:error, {:invalid_json, error}} ->
           "Unable to encode JSON Sentry error - #{inspect(error)}"
 

lib/sentry/config.ex

@@ -3,7 +3,7 @@ defmodule Sentry.Config do
 
   basic_opts_schema = [
     dsn: [
-      type: {:or, [:string, nil]},
+      type: {:or, [nil, {:custom, __MODULE__, :__validate_string_dsn__, []}]},
       default: nil,
       type_doc: "`t:String.t/0` or `nil`",
       doc: """
@@ -316,7 +316,6 @@ defmodule Sentry.Config do
         opts
         |> normalize_included_environments()
         |> normalize_environment()
-        |> assert_dsn_has_no_query_params!()
         |> handle_deprecated_before_send()
 
       {:error, error} ->
@@ -365,7 +364,7 @@ defmodule Sentry.Config do
     """
   end
 
-  @spec dsn() :: String.t() | nil
+  @spec dsn() :: nil | {String.t(), String.t(), String.t()}
   def dsn, do: get(:dsn)
 
   # TODO: remove me on v11.0.0, :included_environments has been deprecated
@@ -500,35 +499,6 @@ defmodule Sentry.Config do
     Keyword.update!(config, :environment_name, &to_string/1)
   end
 
-  defp assert_dsn_has_no_query_params!(config) do
-    if sentry_dsn = Keyword.get(config, :dsn) do
-      uri_dsn = URI.parse(sentry_dsn)
-
-      if uri_dsn.query do
-        raise ArgumentError, """
-        using a Sentry DSN with query parameters is not supported since v9.0.0 of this library.
-        The configured DSN was:
-
-            #{inspect(sentry_dsn)}
-
-        The query string in that DSN is:
-
-            #{inspect(uri_dsn.query)}
-
-        Please remove the query parameters from your DSN and pass them in as regular
-        configuration. Check out the guide to upgrade to 9.0.0 at:
-
-          https://hexdocs.pm/sentry/upgrade-9.x.html
-
-        See the documentation for the Sentry module for more information on configuration
-        in general.
-        """
-      end
-    end
-
-    config
-  end
-
   @compile {:inline, fetch!: 1}
   defp fetch!(key) do
     :persistent_term.get({:sentry_config, key})
@@ -583,4 +553,69 @@ defmodule Sentry.Config do
       {:error, "expected #{inspect(key)} to be a #{inspect(mod)} struct, got: #{inspect(term)}"}
     end
   end
+
+  def __validate_string_dsn__(dsn) when is_binary(dsn) do
+    uri = URI.parse(dsn)
+
+    if uri.query do
+      raise ArgumentError, """
+      using a Sentry DSN with query parameters is not supported since v9.0.0 of this library.
+      The configured DSN was:
+
+          #{inspect(dsn)}
+
+      The query string in that DSN is:
+
+          #{inspect(uri.query)}
+
+      Please remove the query parameters from your DSN and pass them in as regular
+      configuration. Check out the guide to upgrade to 9.0.0 at:
+
+        https://hexdocs.pm/sentry/upgrade-9.x.html
+
+      See the documentation for the Sentry module for more information on configuration
+      in general.
+      """
+    end
+
+    unless is_binary(uri.path) do
+      throw("missing project ID at the end of the DSN URI: #{inspect(dsn)}")
+    end
+
+    unless is_binary(uri.userinfo) do
+      throw("missing user info in the DSN URI: #{inspect(dsn)}")
+    end
+
+    {public_key, secret_key} =
+      case String.split(uri.userinfo, ":", parts: 2) do
+        [public, secret] -> {public, secret}
+        [public] -> {public, nil}
+      end
+
+    with {:ok, {base_path, project_id}} <- pop_project_id(uri.path) do
+      new_path = Enum.join([base_path, "api", project_id, "envelope"], "/") <> "/"
+      endpoint_uri = URI.merge(%URI{uri | userinfo: nil}, new_path)
+
+      {:ok, {URI.to_string(endpoint_uri), public_key, secret_key}}
+    end
+  catch
+    message -> {:error, message}
+  end
+
+  def __validate_string_dsn__(other) do
+    {:error, "expected :dsn to be a string or nil, got: #{inspect(other)}"}
+  end
+
+  defp pop_project_id(uri_path) do
+    path = String.split(uri_path, "/")
+    {project_id, path} = List.pop_at(path, -1)
+
+    case Integer.parse(project_id) do
+      {_project_id, ""} ->
+        {:ok, {Enum.join(path, "/"), project_id}}
+
+      _other ->
+        {:error, "expected the DSN path to end with an integer project ID, got: #{inspect(path)}"}
+    end
+  end
 end

lib/sentry/transport.ex

@@ -20,12 +20,13 @@ defmodule Sentry.Transport do
           {:ok, envelope_id :: String.t()} | {:error, term()}
   def post_envelope(%Envelope{} = envelope, client, retries \\ @default_retries)
       when is_atom(client) and is_list(retries) do
-    with {:json, {:ok, body}} <- {:json, Envelope.to_binary(envelope)},
-         {:ok, endpoint, headers} <- get_endpoint_and_headers() do
-      post_envelope_with_retries(client, endpoint, headers, body, retries)
-    else
-      {:json, {:error, reason}} -> {:error, {:invalid_json, reason}}
-      {:error, _reason} = error -> error
+    case Envelope.to_binary(envelope) do
+      {:ok, body} ->
+        {endpoint, headers} = get_endpoint_and_headers()
+        post_envelope_with_retries(client, endpoint, headers, body, retries)
+
+      {:error, reason} ->
+        {:error, {:invalid_json, reason}}
     end
   end
 
@@ -64,43 +65,8 @@ defmodule Sentry.Transport do
   end
 
   defp get_endpoint_and_headers do
-    case get_dsn() do
-      {endpoint, public_key, secret_key} ->
-        {:ok, endpoint, authorization_headers(public_key, secret_key)}
-
-      {:error, :invalid_dsn} ->
-        {:error, :invalid_dsn}
-    end
-  end
-
-  # Made public for testing.
-  @spec get_dsn() :: {String.t(), String.t(), String.t()} | {:error, :invalid_dsn}
-  def get_dsn do
-    with dsn when is_binary(dsn) <- Config.dsn(),
-         %URI{userinfo: userinfo, host: host, port: port, path: path, scheme: protocol}
-         when is_binary(path) and is_binary(userinfo) <- URI.parse(dsn),
-         [public_key, secret_key] <- keys_from_userinfo(userinfo),
-         uri_path <- String.split(path, "/"),
-         {binary_project_id, uri_path} <- List.pop_at(uri_path, -1),
-         base_path <- Enum.join(uri_path, "/"),
-         {project_id, ""} <- Integer.parse(binary_project_id),
-         endpoint <- "#{protocol}://#{host}:#{port}#{base_path}/api/#{project_id}/envelope/" do
-      {endpoint, public_key, secret_key}
-    else
-      _ ->
-        {:error, :invalid_dsn}
-    end
-  end
+    {endpoint, public_key, secret_key} = Config.dsn()
 
-  defp keys_from_userinfo(userinfo) do
-    case String.split(userinfo, ":", parts: 2) do
-      [public, secret] -> [public, secret]
-      [public] -> [public, nil]
-      _ -> :error
-    end
-  end
-
-  defp authorization_headers(public_key, secret_key) do
     auth_query =
       [
         sentry_version: @sentry_version,
@@ -112,9 +78,11 @@ defmodule Sentry.Transport do
       |> Enum.reject(fn {_, value} -> is_nil(value) end)
       |> Enum.map_join(", ", fn {name, value} -> "#{name}=#{value}" end)
 
-    [
+    auth_headers = [
       {"User-Agent", @sentry_client},
       {"X-Sentry-Auth", "Sentry " <> auth_query}
     ]
+
+    {endpoint, auth_headers}
   end
 end

lib/sentry/transport/sender.ex

@@ -54,9 +54,6 @@ defmodule Sentry.Transport.Sender do
     else
       message =
         case send_result do
-          {:error, :invalid_dsn} ->
-            "Cannot send Sentry event because of invalid DSN"
-
           {:error, {:invalid_json, error}} ->
             "Unable to encode JSON Sentry error - #{inspect(error)}"
 

test/logger_backend_test.exs

@@ -343,7 +343,8 @@ defmodule Sentry.LoggerBackendTest do
       before_send: fn event ->
         send(pid, {ref, event})
         false
-      end
+      end,
+      dsn: "http://public:secret@localhost:9392/1"
     )
 
     ref

test/sentry/config_test.exs

@@ -5,15 +5,16 @@ defmodule Sentry.ConfigTest do
 
   describe "validate!/0" do
     test ":dsn from option" do
-      dsn = "https://public:[email protected]/1"
-      assert Config.validate!(dsn: dsn)[:dsn] == dsn
+      assert Config.validate!(dsn: "https://public:[email protected]/1")[:dsn] ==
+               {"https://app.getsentry.com/api/1/envelope/", "public", "secret"}
+
+      assert Config.validate!(dsn: nil)[:dsn] == nil
     end
 
     test ":dsn from system environment" do
-      dsn = "https://public:[email protected]/1"
-
-      with_system_env("SENTRY_DSN", dsn, fn ->
-        assert Config.validate!([])[:dsn] == dsn
+      with_system_env("SENTRY_DSN", "https://public:[email protected]/1", fn ->
+        assert Config.validate!([])[:dsn] ==
+                 {"https://app.getsentry.com/api/1/envelope/", "public", "secret"}
       end)
     end
 
@@ -26,9 +27,27 @@ defmodule Sentry.ConfigTest do
     end
 
     test "invalid :dsn" do
+      # Not a string.
       assert_raise ArgumentError, ~r/invalid value for :dsn option/, fn ->
         Config.validate!(dsn: :not_a_string)
       end
+
+      # Project ID is missing.
+      assert_raise ArgumentError, ~r/missing project ID at the end of the DSN URI/, fn ->
+        Config.validate!(dsn: "https://public:[email protected]")
+      end
+
+      # Project ID is not an integer.
+      assert_raise ArgumentError, ~r/DSN path to end with an integer project ID/, fn ->
+        Config.validate!(dsn: "https://public:[email protected]/not-an-int")
+      end
+
+      # Userinfo is missing.
+      for dsn <- ["https://app.getsentry.com/1", "https://@app.getsentry.com/1"] do
+        assert_raise ArgumentError, ~r/missing user info in the DSN URI/, fn ->
+          Config.validate!(dsn: dsn)
+        end
+      end
     end
 
     test ":release from option" do
@@ -186,7 +205,8 @@ defmodule Sentry.ConfigTest do
       new_dsn = "https://public:[email protected]/2"
       assert :ok = Config.put_config(:dsn, new_dsn)
 
-      assert Config.dsn() == new_dsn
+      assert Config.dsn() ==
+               {"https://app.getsentry.com/api/2/envelope/", "public", "secret"}
     end
 
     test "validates the given key" do

test/sentry/logger_handler_test.exs

@@ -332,7 +332,8 @@ defmodule Sentry.LoggerHandlerTest do
       before_send: fn event ->
         send(pid, {ref, event})
         false
-      end
+      end,
+      dsn: "http://public:secret@localhost:9392/1"
     )
 
     %{sender_ref: ref}

test/sentry/transport_test.exs

@@ -192,26 +192,4 @@ defmodule Sentry.TransportTest do
       assert_received {:request, ^ref}
     end
   end
-
-  describe "get_dsn/0" do
-    test "parses correct DSNs" do
-      put_test_config(dsn: "http://public:secret@localhost:3000/1")
-      assert {"http://localhost:3000/api/1/envelope/", "public", "secret"} = Transport.get_dsn()
-    end
-
-    test "errors on bad public keys" do
-      put_test_config(dsn: "https://app.getsentry.com/1")
-      assert {:error, :invalid_dsn} = Transport.get_dsn()
-    end
-
-    test "errors on non-integer project_id" do
-      put_test_config(dsn: "https://public:[email protected]/Mitchell")
-      assert {:error, :invalid_dsn} = Transport.get_dsn()
-    end
-
-    test "errors on no project_id" do
-      put_test_config(dsn: "https://public:[email protected]")
-      assert {:error, :invalid_dsn} = Transport.get_dsn()
-    end
-  end
 end