diff --git a/README.md b/README.md index fb56f7be..d3f1a5cc 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,7 @@ config :sentry, environment_name: Mix.env(), included_environments: [:prod], enable_source_code_context: true, - root_source_code_path: File.cwd!() + root_source_code_paths: [File.cwd!()] ``` The `environment_name` and `included_environments` work together to determine @@ -169,7 +169,7 @@ The full range of options is the following: | `in_app_module_whitelist` | False | `[]` | | | `report_deps` | False | True | Will attempt to load Mix dependencies at compile time to report alongside events | | `enable_source_code_context` | False | False | | -| `root_source_code_path` | Required if `enable_source_code_context` is enabled | | Should generally be set to `File.cwd!()`| +| `root_source_code_paths` | Required if `enable_source_code_context` is enabled | | Should usually be set to `[File.cwd!()]`. For umbrella applications you should list all your applications paths in this list (e.g. `["#{File.cwd!()}/apps/app_1", "#{File.cwd!()}/apps/app_2"]`. | | `context_lines` | False | 3 | | | `source_code_exclude_patterns` | False | `[~r"/_build/", ~r"/deps/", ~r"/priv/"]` | | | `source_code_path_pattern` | False | `"**/*.ex"` | | diff --git a/config/config.exs b/config/config.exs index 9843744c..b2ae7374 100644 --- a/config/config.exs +++ b/config/config.exs @@ -5,6 +5,6 @@ config :sentry, environment_name: :dev, tags: %{}, enable_source_code_context: true, - root_source_code_path: File.cwd!() + root_source_code_paths: [File.cwd!()] import_config "#{Mix.env()}.exs" diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index e7fdf963..1bb33386 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -77,6 +77,7 @@ defmodule Sentry.Config do get_config(:enable_source_code_context, default: false, check_dsn: false) end + @deprecated "Use root_source_code_paths/0 instead" def root_source_code_path do path = get_config(:root_source_code_path) @@ -87,6 +88,38 @@ defmodule Sentry.Config do end end + # :root_source_code_path (single path) was replaced by :root_source_code_paths (list of + # paths). + # + # In order for this to not be a breaking change we still accept the old + # :root_source_code_path as a fallback. + # + # We should deprecate this the :root_source_code_path completely in the next major + # release. + def root_source_code_paths do + paths = get_config(:root_source_code_paths) + path = get_config(:root_source_code_path) + + cond do + not is_nil(path) and not is_nil(paths) -> + raise ArgumentError, """ + :root_source_code_path and :root_source_code_paths can't be configured at the \ + same time. + + :root_source_code_path is deprecated. Set :root_source_code_paths instead. + """ + + not is_nil(paths) -> + paths + + not is_nil(path) -> + [path] + + true -> + raise ArgumentError.exception(":root_source_code_paths must be configured") + end + end + def source_code_path_pattern do get_config(:source_code_path_pattern, default: @default_path_pattern, check_dsn: false) end diff --git a/lib/sentry/sources.ex b/lib/sentry/sources.ex index ba96b34c..e3265476 100644 --- a/lib/sentry/sources.ex +++ b/lib/sentry/sources.ex @@ -8,13 +8,14 @@ defmodule Sentry.Sources do ### Configuration There is configuration required to set up this functionality. The options - include `:enable_source_code_context`, `:root_source_code_path`, `:context_lines`, + include `:enable_source_code_context`, `:root_source_code_paths`, `:context_lines`, `:source_code_exclude_patterns`, and `:source_code_path_pattern`. * `:enable_source_code_context` - when `true`, enables reporting source code alongside exceptions. - * `:root_source_code_path` - The path from which to start recursively reading files from. - Should usually be set to `File.cwd!()`. + * `:root_source_code_paths` - List of paths from which to start recursively reading files from. + Should usually be set to `[File.cwd!()]`. For umbrella applications you should list all your + applications paths in this list (e.g. `["#{File.cwd!()}/apps/app_1", "#{File.cwd!()}/apps/app_2"]`. * `:context_lines` - The number of lines of source code before and after the line that caused the exception to be included. Defaults to `3`. * `:source_code_exclude_patterns` - a list of Regex expressions used to exclude file paths that @@ -28,7 +29,7 @@ defmodule Sentry.Sources do config :sentry, dsn: "https://public:secret@app.getsentry.com/1", enable_source_code_context: true, - root_source_code_path: File.cwd!(), + root_source_code_path: [File.cwd!()], context_lines: 5 ### Source code storage @@ -64,19 +65,11 @@ defmodule Sentry.Sources do @type source_map :: %{String.t() => file_map} def load_files do - root_path = Config.root_source_code_path() - path_pattern = Config.source_code_path_pattern() - exclude_patterns = Config.source_code_exclude_patterns() - - Path.join(root_path, path_pattern) - |> Path.wildcard() - |> exclude_files(exclude_patterns) - |> Enum.reduce(%{}, fn path, acc -> - key = Path.relative_to(path, root_path) - value = source_to_lines(File.read!(path)) - - Map.put(acc, key, value) - end) + Enum.reduce( + Config.root_source_code_paths(), + %{}, + &load_files_for_root_path/2 + ) end @doc """ @@ -125,6 +118,37 @@ defmodule Sentry.Sources do end) end + defp load_files_for_root_path(root_path, files) do + root_path + |> find_files_for_root_path() + |> Enum.reduce(files, fn path, acc -> + key = Path.relative_to(path, root_path) + + if Map.has_key?(acc, key) do + raise RuntimeError, """ + Found two source files in different source root paths with the same relative \ + path: #{key} + + This means that both source files would be reported to Sentry as the same \ + file. Please rename one of them to avoid this. + """ + else + value = source_to_lines(File.read!(path)) + + Map.put(acc, key, value) + end + end) + end + + defp find_files_for_root_path(root_path) do + path_pattern = Config.source_code_path_pattern() + exclude_patterns = Config.source_code_exclude_patterns() + + Path.join(root_path, path_pattern) + |> Path.wildcard() + |> exclude_files(exclude_patterns) + end + defp exclude_files(file_names, []), do: file_names defp exclude_files(file_names, [exclude_pattern | rest]) do diff --git a/test/config_test.exs b/test/config_test.exs index 8e63ca5e..44df9ff1 100644 --- a/test/config_test.exs +++ b/test/config_test.exs @@ -72,4 +72,46 @@ defmodule Sentry.ConfigTest do assert ["test", "dev"] == Config.included_environments() end end + + describe "root_source_code_paths" do + test "raises error if :root_source_code_path and :root_source_code_paths are set" do + modify_env(:sentry, root_source_code_path: "/test") + modify_env(:sentry, root_source_code_paths: ["/test"]) + + expected_error_message = """ + :root_source_code_path and :root_source_code_paths can't be configured at the \ + same time. + + :root_source_code_path is deprecated. Set :root_source_code_paths instead. + """ + + assert_raise ArgumentError, expected_error_message, fn -> + Config.root_source_code_paths() + end + end + + test "raises error if :root_source_code_path and :root_source_code_paths are not set" do + delete_env(:sentry, [:root_source_code_path, :root_source_code_paths]) + + expected_error_message = ":root_source_code_paths must be configured" + + assert_raise ArgumentError, expected_error_message, fn -> + Config.root_source_code_paths() + end + end + + test "returns :root_source_code_path if it's set" do + modify_env(:sentry, root_source_code_path: "/test") + modify_env(:sentry, root_source_code_paths: nil) + + assert Config.root_source_code_paths() == ["/test"] + end + + test "returns :root_source_code_paths if it's set" do + modify_env(:sentry, root_source_code_path: nil) + modify_env(:sentry, root_source_code_paths: ["/test"]) + + assert Config.root_source_code_paths() == ["/test"] + end + end end diff --git a/test/sources_test.exs b/test/sources_test.exs index 0e48e736..005536cc 100644 --- a/test/sources_test.exs +++ b/test/sources_test.exs @@ -3,6 +3,51 @@ defmodule Sentry.SourcesTest do use Plug.Test import Sentry.TestEnvironmentHelper + describe "load_files/0" do + test "loads files" do + modify_env(:sentry, root_source_code_paths: [ + File.cwd!() <> "/test/support/example-umbrella-app/apps/app_a", + File.cwd!() <> "/test/support/example-umbrella-app/apps/app_b" + ]) + + assert %{ + "lib/module_a.ex" => %{ + 1 => "defmodule ModuleA do", + 2 => " def test do", + 3 => " \"test a\"", + 4 => " end", + 5 => "end" + }, + "lib/module_b.ex" => %{ + 1 => "defmodule ModuleB do", + 2 => " def test do", + 3 => " \"test b\"", + 4 => " end", + 5 => "end" + } + } = Sentry.Sources.load_files() + end + + test "raises error when two files have the same relative path" do + modify_env(:sentry, root_source_code_paths: [ + File.cwd!() <> "/test/support/example-umbrella-app-with-conflict/apps/app_a", + File.cwd!() <> "/test/support/example-umbrella-app-with-conflict/apps/app_b" + ]) + + expected_error_message = """ + Found two source files in different source root paths with the same relative \ + path: lib/module_a.ex + + This means that both source files would be reported to Sentry as the same \ + file. Please rename one of them to avoid this. + """ + + assert_raise RuntimeError, expected_error_message, fn -> + Sentry.Sources.load_files() + end + end + end + test "exception makes call to Sentry API" do correct_context = %{ "context_line" => " raise RuntimeError, \"Error\"", diff --git a/test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex b/test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex new file mode 100644 index 00000000..9c3fd6ca --- /dev/null +++ b/test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex @@ -0,0 +1,5 @@ +defmodule ModuleA do + def test do + "test a" + end +end diff --git a/test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex b/test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex new file mode 100644 index 00000000..771a6e26 --- /dev/null +++ b/test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex @@ -0,0 +1,5 @@ +defmodule ModuleB do + def test do + "test b" + end +end diff --git a/test/support/example-umbrella-app/apps/app_a/lib/module_a.ex b/test/support/example-umbrella-app/apps/app_a/lib/module_a.ex new file mode 100644 index 00000000..9c3fd6ca --- /dev/null +++ b/test/support/example-umbrella-app/apps/app_a/lib/module_a.ex @@ -0,0 +1,5 @@ +defmodule ModuleA do + def test do + "test a" + end +end diff --git a/test/support/example-umbrella-app/apps/app_b/lib/module_b.ex b/test/support/example-umbrella-app/apps/app_b/lib/module_b.ex new file mode 100644 index 00000000..771a6e26 --- /dev/null +++ b/test/support/example-umbrella-app/apps/app_b/lib/module_b.ex @@ -0,0 +1,5 @@ +defmodule ModuleB do + def test do + "test b" + end +end