Merge pull request #218 from getsentry/avoid-raising-on-bad-dsn

avoid raising on bad dsn

Author: mitchellhenke

.travis.yml

@@ -9,18 +9,18 @@ otp_release:
   - 19.3
   - 20.2
 env:
-  - MIX_FORMAT=true
-  - MIX_FORMAT=false
+  - STRICT=true
+  - STRICT=false
 matrix:
   exclude:
     - elixir: 1.6
-      env: MIX_FORMAT=false
+      env: STRICT=false
     - elixir: 1.3.4
-      env: MIX_FORMAT=true
+      env: STRICT=true
     - elixir: 1.4
-      env: MIX_FORMAT=true
+      env: STRICT=true
     - elixir: 1.5
-      env: MIX_FORMAT=true
+      env: STRICT=true
     - elixir: 1.6
       otp_release: 18.3
     - elixir: 1.3.4
@@ -29,6 +29,12 @@ notifications:
   email:
     - [email protected]
 script:
+  - if [ "$STRICT" = "true" ]; then mix compile --warnings-as-errors; fi
   - mix test
   - mix credo
-  - if [ "$MIX_FORMAT" = "true" ]; then mix format --dry-run --check-formatted; fi
+  - if [ "$STRICT" = "true" ]; then mix dialyzer; fi
+  - if [ "$STRICT" = "true" ]; then mix format --dry-run --check-formatted; fi
+cache:
+  directories:
+    - .mix
+    - _build

lib/mix/tasks/sentry.send_test_event.ex

@@ -14,7 +14,7 @@ defmodule Mix.Tasks.Sentry.SendTestEvent do
 
     Application.ensure_all_started(:sentry)
 
-    Sentry.Client.get_dsn!()
+    Sentry.Client.get_dsn()
     |> print_environment_info()
 
     maybe_send_event()

lib/sentry.ex

@@ -91,7 +91,7 @@ defmodule Sentry do
   See `Sentry.Logger`
   """
 
-  @type send_result :: Sentry.Client.send_result() | :excluded | :ignored
+  @type send_result :: Sentry.Client.send_event_result() | :excluded | :ignored
 
   def start(_type, _opts) do
     children = [

lib/sentry/client.ex

@@ -38,8 +38,8 @@ defmodule Sentry.Client do
 
   require Logger
 
-  @type dsn :: {String.t(), String.t(), String.t()}
   @type send_event_result :: {:ok, Task.t() | String.t() | pid()} | :error | :unsampled
+  @type dsn :: {String.t(), String.t(), String.t()} | :error
   @sentry_version 5
   @max_attempts 4
   @hackney_pool_name :sentry_pool
@@ -85,34 +85,43 @@ defmodule Sentry.Client do
   end
 
   defp do_send_event(event, body, :async) do
-    {endpoint, public_key, secret_key} = get_dsn!()
-    auth_headers = authorization_headers(public_key, secret_key)
-
-    {:ok,
-     Task.Supervisor.async_nolink(Sentry.TaskSupervisor, fn ->
-       try_request(:post, endpoint, auth_headers, body)
-       |> maybe_call_after_send_event(event)
-     end)}
+    case get_headers_and_endpoint() do
+      {endpoint, auth_headers} ->
+        {:ok,
+         Task.Supervisor.async_nolink(Sentry.TaskSupervisor, fn ->
+           try_request(:post, endpoint, auth_headers, body)
+           |> maybe_call_after_send_event(event)
+         end)}
+
+      _ ->
+        :error
+    end
   end
 
   defp do_send_event(event, body, :sync) do
-    {endpoint, public_key, secret_key} = get_dsn!()
-    auth_headers = authorization_headers(public_key, secret_key)
+    case get_headers_and_endpoint() do
+      {endpoint, auth_headers} ->
+        try_request(:post, endpoint, auth_headers, body)
+        |> maybe_call_after_send_event(event)
 
-    try_request(:post, endpoint, auth_headers, body)
-    |> maybe_call_after_send_event(event)
+      _ ->
+        :error
+    end
   end
 
   defp do_send_event(event, body, :none) do
-    {endpoint, public_key, secret_key} = get_dsn!()
-    auth_headers = authorization_headers(public_key, secret_key)
+    case get_headers_and_endpoint() do
+      {endpoint, auth_headers} ->
+        Task.Supervisor.start_child(Sentry.TaskSupervisor, fn ->
+          try_request(:post, endpoint, auth_headers, body)
+          |> maybe_call_after_send_event(event)
+        end)
 
-    Task.Supervisor.start_child(Sentry.TaskSupervisor, fn ->
-      try_request(:post, endpoint, auth_headers, body)
-      |> maybe_call_after_send_event(event)
-    end)
+        {:ok, ""}
 
-    {:ok, ""}
+      _ ->
+        :error
+    end
   end
 
   defp try_request(method, url, headers, body, current_attempt \\ 1)
@@ -193,19 +202,25 @@ defmodule Sentry.Client do
 
   @doc """
   Get a Sentry DSN which is simply a URI.
+
+  {PROTOCOL}://{PUBLIC_KEY}:{SECRET_KEY}@{HOST}/{PATH}{PROJECT_ID}
   """
-  @spec get_dsn! :: dsn
-  def get_dsn! do
-    # {PROTOCOL}://{PUBLIC_KEY}:{SECRET_KEY}@{HOST}/{PATH}{PROJECT_ID}
-    %URI{userinfo: userinfo, host: host, port: port, path: path, scheme: protocol} =
-      URI.parse(Config.dsn())
-
-    [public_key, secret_key] = String.split(userinfo, ":", parts: 2)
-    [_, binary_project_id] = String.split(path, "/")
-    project_id = String.to_integer(binary_project_id)
-    endpoint = "#{protocol}://#{host}:#{port}/api/#{project_id}/store/"
-
-    {endpoint, public_key, secret_key}
+  @spec get_dsn :: dsn
+  def get_dsn do
+    dsn = Config.dsn()
+
+    with %URI{userinfo: userinfo, host: host, port: port, path: path, scheme: protocol}
+         when is_binary(path) <- URI.parse(dsn),
+         [public_key, secret_key] <- String.split(userinfo, ":", parts: 2),
+         [_, binary_project_id] <- String.split(path, "/"),
+         {project_id, ""} <- Integer.parse(binary_project_id),
+         endpoint <- "#{protocol}://#{host}:#{port}/api/#{project_id}/store/" do
+      {endpoint, public_key, secret_key}
+    else
+      _ ->
+        log_api_error("Cannot send event because of invalid DSN")
+        :error
+    end
   end
 
   @spec maybe_call_after_send_event(send_event_result, Event.t()) :: Event.t()
@@ -289,6 +304,16 @@ defmodule Sentry.Client do
     end
   end
 
+  defp get_headers_and_endpoint do
+    case get_dsn() do
+      {endpoint, public_key, secret_key} ->
+        {endpoint, authorization_headers(public_key, secret_key)}
+
+      _ ->
+        :error
+    end
+  end
+
   defp log_api_error(body) do
     Logger.warn(fn ->
       ["Failed to send Sentry event.", ?\n, body]

mix.exs

@@ -10,7 +10,7 @@ defmodule Sentry.Mixfile do
       package: package(),
       deps: deps(),
       elixirc_paths: elixirc_paths(Mix.env()),
-      dialyzer: [plt_add_deps: :transitive],
+      dialyzer: [plt_add_deps: :transitive, plt_add_apps: [:mix]],
       docs: [extras: ["README.md"], main: "readme"]
     ]
   end

mix.lock

@@ -1,21 +1,23 @@
-%{"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"},
+%{
+  "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"},
   "bypass": {:hex, :bypass, "0.8.1", "16d409e05530ece4a72fabcf021a3e5c7e15dcc77f911423196a0c551f2a15ca", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, repo: "hexpm", optional: false]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"},
   "certifi": {:hex, :certifi, "1.2.1", "c3904f192bd5284e5b13f20db3ceac9626e14eeacfbb492e19583cf0e37b22be", [:rebar3], [], "hexpm"},
   "cowboy": {:hex, :cowboy, "1.1.2", "61ac29ea970389a88eca5a65601460162d370a70018afe6f949a29dca91f3bb0", [:rebar3], [{:cowlib, "~> 1.0.2", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3.2", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm"},
   "cowlib": {:hex, :cowlib, "1.0.2", "9d769a1d062c9c3ac753096f868ca121e2730b9a377de23dec0f7e08b1df84ee", [:make], [], "hexpm"},
   "credo": {:hex, :credo, "0.8.10", "261862bb7363247762e1063713bb85df2bbd84af8d8610d1272cd9c1943bba63", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}], "hexpm"},
   "dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [:mix], [], "hexpm"},
-  "earmark": {:hex, :earmark, "1.2.3", "206eb2e2ac1a794aa5256f3982de7a76bf4579ff91cb28d0e17ea2c9491e46a4", [:mix], [], "hexpm"},
-  "ex_doc": {:hex, :ex_doc, "0.18.1", "37c69d2ef62f24928c1f4fdc7c724ea04aecfdf500c4329185f8e3649c915baf", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm"},
+  "earmark": {:hex, :earmark, "1.2.4", "99b637c62a4d65a20a9fb674b8cffb8baa771c04605a80c911c4418c69b75439", [:mix], [], "hexpm"},
+  "ex_doc": {:hex, :ex_doc, "0.18.3", "f4b0e4a2ec6f333dccf761838a4b253d75e11f714b85ae271c9ae361367897b7", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm"},
   "hackney": {:hex, :hackney, "1.8.6", "21a725db3569b3fb11a6af17d5c5f654052ce9624219f1317e8639183de4a423", [:rebar3], [{:certifi, "1.2.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "5.0.2", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
   "idna": {:hex, :idna, "5.0.2", "ac203208ada855d95dc591a764b6e87259cb0e2a364218f215ad662daa8cd6b4", [:rebar3], [{:unicode_util_compat, "0.2.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"},
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"},
-  "mime": {:hex, :mime, "1.1.0", "01c1d6f4083d8aa5c7b8c246ade95139620ef8effb009edde934e0ec3b28090a", [:mix], [], "hexpm"},
+  "mime": {:hex, :mime, "1.2.0", "78adaa84832b3680de06f88f0997e3ead3b451a440d183d688085be2d709b534", [:mix], [], "hexpm"},
   "mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [:rebar3], [], "hexpm"},
   "plug": {:hex, :plug, "1.4.3", "236d77ce7bf3e3a2668dc0d32a9b6f1f9b1f05361019946aae49874904be4aed", [:mix], [{:cowboy, "~> 1.0.1 or ~> 1.1", [hex: :cowboy, repo: "hexpm", optional: true]}, {:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}], "hexpm"},
   "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
   "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], [], "hexpm"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], [], "hexpm"},
   "ssl_verify_hostname": {:hex, :ssl_verify_hostname, "1.0.5", "2e73e068cd6393526f9fa6d399353d7c9477d6886ba005f323b592d389fb47be", [:make], []},
   "unicode_util_compat": {:hex, :unicode_util_compat, "0.2.0", "dbbccf6781821b1c0701845eaf966c9b6d83d7c3bfc65ca2b78b88b8678bfa35", [:rebar3], [], "hexpm"},
-  "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm"}}
+  "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm"},
+}

test/client_test.exs

@@ -8,7 +8,7 @@ defmodule Sentry.ClientTest do
 
   test "authorization" do
     modify_env(:sentry, dsn: "https://public:[email protected]/1")
-    {_endpoint, public_key, private_key} = Client.get_dsn!()
+    {_endpoint, public_key, private_key} = Client.get_dsn()
 
     assert Client.authorization_header(public_key, private_key) =~
              ~r/^Sentry sentry_version=5, sentry_client=sentry-elixir\/#{
@@ -20,15 +20,39 @@ defmodule Sentry.ClientTest do
     modify_env(:sentry, dsn: "https://public:[email protected]/1")
 
     assert {"https://app.getsentry.com:443/api/1/store/", "public", "secret"} =
-             Sentry.Client.get_dsn!()
+             Sentry.Client.get_dsn()
   end
 
   test "get dsn with system config" do
     modify_env(:sentry, dsn: {:system, "SYSTEM_KEY"})
     modify_system_env(%{"SYSTEM_KEY" => "https://public:[email protected]/1"})
 
     assert {"https://app.getsentry.com:443/api/1/store/", "public", "secret"} =
-             Sentry.Client.get_dsn!()
+             Sentry.Client.get_dsn()
+  end
+
+  test "errors on bad public/secret keys" do
+    modify_env(:sentry, dsn: "https://[email protected]/1")
+
+    capture_log(fn ->
+      assert :error = Sentry.Client.get_dsn()
+    end)
+  end
+
+  test "errors on non-integer project_id" do
+    modify_env(:sentry, dsn: "https://public:[email protected]/Mitchell")
+
+    capture_log(fn ->
+      assert :error = Sentry.Client.get_dsn()
+    end)
+  end
+
+  test "errors on no project_id" do
+    modify_env(:sentry, dsn: "https://public:[email protected]")
+
+    capture_log(fn ->
+      assert :error = Sentry.Client.get_dsn()
+    end)
   end
 
   test "logs api errors" do

test/event_test.exs

@@ -457,7 +457,7 @@ defmodule Sentry.EventTest do
              hackney: "1.8.6",
              idna: "5.0.2",
              metrics: "1.0.1",
-             mime: "1.1.0",
+             mime: "1.2.0",
              mimerl: "1.0.2",
              plug: "1.4.3",
              poison: "3.1.0",

test/support/test_client.exs

@@ -3,7 +3,7 @@ defmodule Sentry.TestClient do
   require Logger
 
   def send_event(%Sentry.Event{} = event, _opts \\ []) do
-    {endpoint, _public_key, _secret_key} = Sentry.Client.get_dsn!()
+    {endpoint, _public_key, _secret_key} = Sentry.Client.get_dsn()
     event = Sentry.Client.maybe_call_before_send_event(event)
 
     Sentry.Client.render_event(event)