From 4454612de0521408a6044ef928863ed3c074c7c0 Mon Sep 17 00:00:00 2001 From: Jonathan Moraes Date: Mon, 30 Dec 2024 16:32:52 +0100 Subject: [PATCH 1/4] Allow the usage of JSON for Elixir 1.18+ --- config/config.exs | 10 ++++- lib/sentry/client.ex | 12 ++---- lib/sentry/config.ex | 15 +++---- lib/sentry/envelope.ex | 39 ++++++------------- lib/sentry/transport.ex | 9 ++--- test/envelope_test.exs | 28 ++++++------- test/mix/sentry.package_source_code_test.exs | 4 +- test/plug_capture_test.exs | 4 +- test/sentry/client_test.exs | 14 +++---- test/sentry/config_test.exs | 2 +- test/sentry/transport_test.exs | 10 +++-- test/support/example_plug_application.ex | 4 +- test/support/test_error_view.ex | 5 ++- test/support/test_helpers.ex | 9 +++-- .../phoenix_app/test/support/test_helpers.ex | 6 +-- 15 files changed, 82 insertions(+), 89 deletions(-) diff --git a/config/config.exs b/config/config.exs index 1184ea49..579c4402 100644 --- a/config/config.exs +++ b/config/config.exs @@ -1,8 +1,16 @@ import Config +json_library = + if Version.compare(System.version(), "1.18.0") == :lt do + Jason + else + JSON + end + if config_env() == :test do config :sentry, environment_name: :test, + json_library: json_library, tags: %{}, enable_source_code_context: true, root_source_code_paths: [File.cwd!()], @@ -15,4 +23,4 @@ if config_env() == :test do config :logger, backends: [] end -config :phoenix, :json_library, Jason +config :phoenix, :json_library, json_library diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index 87c124fc..f21c3c51 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -294,14 +294,10 @@ defmodule Sentry.Client do end defp sanitize_non_jsonable_value(value, json_library) do - try do - json_library.encode(value) - catch - _type, _reason -> {:changed, inspect(value)} - else - {:ok, _encoded} -> :unchanged - {:error, _reason} -> {:changed, inspect(value)} - end + json_library.encode!(value) + :unchanged + rescue + _error -> {:changed, inspect(value)} end defp update_if_present(map, key, fun) do diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index a2407512..61f8b3b2 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -152,7 +152,7 @@ defmodule Sentry.Config do type_doc: "`t:module/0`", doc: """ A module that implements the "standard" Elixir JSON behaviour, that is, exports the - `encode/1` and `decode/1` functions. If you use the default, make sure to add + `encode!/1` and `decode!/1` functions. If you use the default, make sure to add [`:jason`](https://hex.pm/packages/jason) as a dependency of your application. """ ], @@ -695,19 +695,14 @@ defmodule Sentry.Config do def __validate_json_library__(mod) when is_atom(mod) do try do - with {:ok, %{}} <- mod.decode("{}"), - {:ok, "{}"} <- mod.encode(%{}) do - {:ok, mod} - else - _ -> - {:error, - "configured :json_library #{inspect(mod)} does not implement decode/1 and encode/1"} - end + %{} = mod.decode!("{}") + "{}" = mod.encode!(%{}) + {:ok, mod} rescue UndefinedFunctionError -> {:error, """ - configured :json_library #{inspect(mod)} is not available or does not implement decode/1 and encode/1. + configured :json_library #{inspect(mod)} is not available or does not implement decode!/1 and encode!/1. Do you need to add #{inspect(mod)} to your mix.exs? """} end diff --git a/lib/sentry/envelope.ex b/lib/sentry/envelope.ex index 649f6e85..d28c1e58 100644 --- a/lib/sentry/envelope.ex +++ b/lib/sentry/envelope.ex @@ -76,19 +76,14 @@ defmodule Sentry.Envelope do items_iodata = Enum.map(envelope.items, &item_to_binary(json_library, &1)) {:ok, IO.iodata_to_binary([headers_iodata, items_iodata])} - catch - {:error, _reason} = error -> error + rescue + error -> {:error, error} end defp item_to_binary(json_library, %Event{} = event) do - case event |> Sentry.Client.render_event() |> json_library.encode() do - {:ok, encoded_event} -> - header = ~s({"type":"event","length":#{byte_size(encoded_event)}}) - [header, ?\n, encoded_event, ?\n] - - {:error, _reason} = error -> - throw(error) - end + encoded_event = event |> Sentry.Client.render_event() |> json_library.encode!() + header = ~s({"type":"event","length":#{byte_size(encoded_event)}}) + [header, ?\n, encoded_event, ?\n] end defp item_to_binary(json_library, %Attachment{} = attachment) do @@ -100,30 +95,20 @@ defmodule Sentry.Envelope do into: header, do: {Atom.to_string(key), value} - {:ok, header_iodata} = json_library.encode(header) + header_iodata = json_library.encode!(header) [header_iodata, ?\n, attachment.data, ?\n] end defp item_to_binary(json_library, %CheckIn{} = check_in) do - case check_in |> CheckIn.to_map() |> json_library.encode() do - {:ok, encoded_check_in} -> - header = ~s({"type":"check_in","length":#{byte_size(encoded_check_in)}}) - [header, ?\n, encoded_check_in, ?\n] - - {:error, _reason} = error -> - throw(error) - end + encoded_check_in = check_in |> CheckIn.to_map() |> json_library.encode!() + header = ~s({"type":"check_in","length":#{byte_size(encoded_check_in)}}) + [header, ?\n, encoded_check_in, ?\n] end defp item_to_binary(json_library, %ClientReport{} = client_report) do - case client_report |> Map.from_struct() |> json_library.encode() do - {:ok, encoded_client_report} -> - header = ~s({"type":"client_report","length":#{byte_size(encoded_client_report)}}) - [header, ?\n, encoded_client_report, ?\n] - - {:error, _reason} = error -> - throw(error) - end + encoded_client_report = client_report |> Map.from_struct() |> json_library.encode!() + header = ~s({"type":"client_report","length":#{byte_size(encoded_client_report)}}) + [header, ?\n, encoded_client_report, ?\n] end end diff --git a/lib/sentry/transport.ex b/lib/sentry/transport.ex index a353b2b8..4c94387d 100644 --- a/lib/sentry/transport.ex +++ b/lib/sentry/transport.ex @@ -93,11 +93,10 @@ defmodule Sentry.Transport do end defp request(client, endpoint, headers, body) do - with {:ok, 200, _headers, body} <- - client_post_and_validate_return_value(client, endpoint, headers, body), - {:ok, json} <- Config.json_library().decode(body) do - {:ok, Map.get(json, "id")} - else + case client_post_and_validate_return_value(client, endpoint, headers, body) do + {:ok, 200, _headers, body} -> + {:ok, body |> Config.json_library().decode!() |> Map.get("id")} + {:ok, 429, headers, _body} -> delay_ms = with timeout when is_binary(timeout) <- diff --git a/test/envelope_test.exs b/test/envelope_test.exs index dd887d3d..702e3eee 100644 --- a/test/envelope_test.exs +++ b/test/envelope_test.exs @@ -15,10 +15,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert Jason.decode!(id_line) == %{"event_id" => event.event_id} - assert %{"type" => "event", "length" => _} = Jason.decode!(header_line) + assert json_library().decode!(id_line) == %{"event_id" => event.event_id} + assert %{"type" => "event", "length" => _} = json_library().decode!(header_line) - assert {:ok, decoded_event} = Jason.decode(event_line) + assert {:ok, decoded_event} = json_library().decode(event_line) assert decoded_event["event_id"] == event.event_id assert decoded_event["breadcrumbs"] == [] assert decoded_event["environment"] == "test" @@ -65,29 +65,29 @@ defmodule Sentry.EnvelopeTest do "..." ] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = Jason.decode!(id_line) + assert %{"event_id" => _} = json_library().decode!(id_line) - assert Jason.decode!(attachment1_header) == %{ + assert json_library().decode!(attachment1_header) == %{ "type" => "attachment", "length" => 3, "filename" => "example.dat" } - assert Jason.decode!(attachment2_header) == %{ + assert json_library().decode!(attachment2_header) == %{ "type" => "attachment", "length" => 6, "filename" => "example.txt", "content_type" => "text/plain" } - assert Jason.decode!(attachment3_header) == %{ + assert json_library().decode!(attachment3_header) == %{ "type" => "attachment", "length" => 2, "filename" => "example.json", "content_type" => "application/json" } - assert Jason.decode!(attachment4_header) == %{ + assert json_library().decode!(attachment4_header) == %{ "type" => "attachment", "length" => 3, "filename" => "dump", @@ -105,10 +105,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = Jason.decode!(id_line) - assert %{"type" => "check_in", "length" => _} = Jason.decode!(header_line) + assert %{"event_id" => _} = json_library().decode!(id_line) + assert %{"type" => "check_in", "length" => _} = json_library().decode!(header_line) - assert {:ok, decoded_check_in} = Jason.decode(event_line) + assert {:ok, decoded_check_in} = json_library().decode(event_line) assert decoded_check_in["check_in_id"] == check_in_id assert decoded_check_in["monitor_slug"] == "test" assert decoded_check_in["status"] == "ok" @@ -128,10 +128,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = Jason.decode!(id_line) - assert %{"type" => "client_report", "length" => _} = Jason.decode!(header_line) + assert %{"event_id" => _} = json_library().decode!(id_line) + assert %{"type" => "client_report", "length" => _} = json_library().decode!(header_line) - assert {:ok, decoded_client_report} = Jason.decode(event_line) + assert {:ok, decoded_client_report} = json_library().decode(event_line) assert decoded_client_report["timestamp"] == client_report.timestamp assert decoded_client_report["discarded_events"] == [ diff --git a/test/mix/sentry.package_source_code_test.exs b/test/mix/sentry.package_source_code_test.exs index f60474b9..bf7e722b 100644 --- a/test/mix/sentry.package_source_code_test.exs +++ b/test/mix/sentry.package_source_code_test.exs @@ -66,8 +66,8 @@ defmodule Mix.Tasks.Sentry.PackageSourceCodeTest do # "loadpaths" and "compile" to the dependencies of this Mix task. test "supports custom configured :json_library" do defmodule Sentry.ExampleJSON do - defdelegate encode(term), to: Jason - defdelegate decode(term), to: Jason + defdelegate encode!(term), to: json_library() + defdelegate decode!(term), to: json_library() end put_test_config(json_library: Sentry.ExampleJSON) diff --git a/test/plug_capture_test.exs b/test/plug_capture_test.exs index 226fc2cf..8480483f 100644 --- a/test/plug_capture_test.exs +++ b/test/plug_capture_test.exs @@ -29,7 +29,7 @@ defmodule Sentry.PlugCaptureTest do use Phoenix.Endpoint, otp_app: :sentry use Plug.Debugger, otp_app: :sentry - plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason + plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: json_library() plug Sentry.PlugContext plug PhoenixRouter end @@ -45,7 +45,7 @@ defmodule Sentry.PlugCaptureTest do use Phoenix.Endpoint, otp_app: :sentry use Plug.Debugger, otp_app: :sentry - plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason + plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: json_library() plug Sentry.PlugContext plug PhoenixRouter end diff --git a/test/sentry/client_test.exs b/test/sentry/client_test.exs index 8e11ab73..fa46997e 100644 --- a/test/sentry/client_test.exs +++ b/test/sentry/client_test.exs @@ -67,10 +67,10 @@ defmodule Sentry.ClientTest do test "works if the JSON library crashes" do defmodule RaisingJSONClient do - def encode(:crash), do: raise("Oops") - def encode(term), do: Jason.encode(term) + def encode!(:crash), do: raise("Oops") + def encode!(term), do: json_library().encode!(term) - def decode(term), do: Jason.decode(term) + def decode!(term), do: json_library().decode!(term) end put_test_config(json_library: RaisingJSONClient) @@ -336,10 +336,10 @@ defmodule Sentry.ClientTest do test "logs an error when unable to encode JSON" do defmodule BadJSONClient do - def encode(term) when term == %{}, do: {:ok, "{}"} - def encode(_term), do: {:error, :im_just_bad} + def encode!(term) when term == %{}, do: "{}" + def encode!(_term), do: raise("im_just_bad") - def decode(term), do: Jason.decode(term) + def decode!(term), do: json_library().decode!(term) end put_test_config(json_library: BadJSONClient) @@ -347,7 +347,7 @@ defmodule Sentry.ClientTest do assert capture_log(fn -> Client.send_event(event, result: :sync) - end) =~ "the Sentry SDK could not encode the event to JSON: :im_just_bad" + end) =~ "the Sentry SDK could not encode the event to JSON: im_just_bad" end test "uses the async sender pool when :result is :none", %{bypass: bypass} do diff --git a/test/sentry/config_test.exs b/test/sentry/config_test.exs index 3637c712..04e64f06 100644 --- a/test/sentry/config_test.exs +++ b/test/sentry/config_test.exs @@ -158,7 +158,7 @@ defmodule Sentry.ConfigTest do end test ":json_library" do - assert Config.validate!(json_library: Jason)[:json_library] == Jason + assert Config.validate!(json_library: JSON)[:json_library] == JSON # Default assert Config.validate!([])[:json_library] == Jason diff --git a/test/sentry/transport_test.exs b/test/sentry/transport_test.exs index 0b7d7e85..10a64507 100644 --- a/test/sentry/transport_test.exs +++ b/test/sentry/transport_test.exs @@ -158,10 +158,10 @@ defmodule Sentry.TransportTest do envelope = Envelope.from_event(Event.create_event(message: "Hello")) defmodule CrashingJSONLibrary do - defdelegate encode(term), to: Jason + defdelegate encode!(term), to: json_library() - def decode("{}"), do: {:ok, %{}} - def decode(_body), do: raise("I'm a really bad JSON library") + def decode!("{}"), do: %{} + def decode!(_body), do: raise("I'm a really bad JSON library") end Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> @@ -190,7 +190,9 @@ defmodule Sentry.TransportTest do Plug.Conn.resp(conn, 200, ~s) end) - assert {:request_failure, %Jason.DecodeError{}} = + exception = Module.concat(json_library(), DecodeError) + + assert {:error, %^exception{}, _stacktrace} = error(fn -> Transport.encode_and_post_envelope(envelope, HackneyClient, _retries = [0]) end) diff --git a/test/support/example_plug_application.ex b/test/support/example_plug_application.ex index ee6e88c5..082ffe1a 100644 --- a/test/support/example_plug_application.ex +++ b/test/support/example_plug_application.ex @@ -5,6 +5,8 @@ defmodule Sentry.ExamplePlugApplication do import ExUnit.Assertions + alias Sentry.Config + plug Plug.Parsers, parsers: [:multipart, :urlencoded] plug Sentry.PlugContext plug :match @@ -50,7 +52,7 @@ defmodule Sentry.ExamplePlugApplication do {event_id, :plug} -> opts = %{title: "Testing", eventId: event_id} - |> Jason.encode!() + |> Config.json_library().encode!() """ diff --git a/test/support/test_error_view.ex b/test/support/test_error_view.ex index 29dcd7e4..473c6511 100644 --- a/test/support/test_error_view.ex +++ b/test/support/test_error_view.ex @@ -1,14 +1,17 @@ defmodule Sentry.ErrorView do use Phoenix.Component + import Phoenix.HTML, only: [raw: 1] + alias Sentry.Config + def render(_, _) do case Sentry.get_last_event_id_and_source() do {event_id, :plug} -> opts = %{title: "Testing", eventId: event_id} - |> Jason.encode!() + |> Config.json_library().encode!() assigns = %{opts: opts} diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index eca6a617..3d45703a 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -3,6 +3,9 @@ defmodule Sentry.TestHelpers do alias Sentry.Config + @spec json_library :: module() + def json_library, do: Config.json_library() + @spec put_test_config(keyword()) :: :ok def put_test_config(config) when is_list(config) do all_original_config = all_config() @@ -46,7 +49,7 @@ defmodule Sentry.TestHelpers do @spec decode_envelope!(binary()) :: [{header :: map(), item :: map()}] def decode_envelope!(binary) do [id_line | rest] = String.split(binary, "\n") - {:ok, %{"event_id" => _}} = Config.json_library().decode(id_line) + %{"event_id" => _} = json_library().decode!(id_line) decode_envelope_items(rest) end @@ -55,8 +58,8 @@ defmodule Sentry.TestHelpers do |> Enum.chunk_every(2) |> Enum.flat_map(fn [header, item] -> - {:ok, header} = Config.json_library().decode(header) - {:ok, item} = Config.json_library().decode(item) + header = json_library().decode!(header) + item = json_library().decode!(item) [{header, item}] [""] -> diff --git a/test_integrations/phoenix_app/test/support/test_helpers.ex b/test_integrations/phoenix_app/test/support/test_helpers.ex index eca6a617..7cdfede7 100644 --- a/test_integrations/phoenix_app/test/support/test_helpers.ex +++ b/test_integrations/phoenix_app/test/support/test_helpers.ex @@ -46,7 +46,7 @@ defmodule Sentry.TestHelpers do @spec decode_envelope!(binary()) :: [{header :: map(), item :: map()}] def decode_envelope!(binary) do [id_line | rest] = String.split(binary, "\n") - {:ok, %{"event_id" => _}} = Config.json_library().decode(id_line) + %{"event_id" => _} = Config.json_library().decode!(id_line) decode_envelope_items(rest) end @@ -55,8 +55,8 @@ defmodule Sentry.TestHelpers do |> Enum.chunk_every(2) |> Enum.flat_map(fn [header, item] -> - {:ok, header} = Config.json_library().decode(header) - {:ok, item} = Config.json_library().decode(item) + header = Config.json_library().decode!(header) + item = Config.json_library().decode!(item) [{header, item}] [""] -> From 217cbdfd24989656de79c9ae4caa063c24c2642a Mon Sep 17 00:00:00 2001 From: Jonathan Moraes Date: Thu, 2 Jan 2025 09:11:02 +0100 Subject: [PATCH 2/4] Revert "Allow the usage of JSON for Elixir 1.18+" This reverts commit 4454612de0521408a6044ef928863ed3c074c7c0. --- config/config.exs | 10 +---- lib/sentry/client.ex | 12 ++++-- lib/sentry/config.ex | 15 ++++--- lib/sentry/envelope.ex | 39 +++++++++++++------ lib/sentry/transport.ex | 9 +++-- test/envelope_test.exs | 28 ++++++------- test/mix/sentry.package_source_code_test.exs | 4 +- test/plug_capture_test.exs | 4 +- test/sentry/client_test.exs | 14 +++---- test/sentry/config_test.exs | 2 +- test/sentry/transport_test.exs | 10 ++--- test/support/example_plug_application.ex | 4 +- test/support/test_error_view.ex | 5 +-- test/support/test_helpers.ex | 9 ++--- .../phoenix_app/test/support/test_helpers.ex | 6 +-- 15 files changed, 89 insertions(+), 82 deletions(-) diff --git a/config/config.exs b/config/config.exs index 579c4402..1184ea49 100644 --- a/config/config.exs +++ b/config/config.exs @@ -1,16 +1,8 @@ import Config -json_library = - if Version.compare(System.version(), "1.18.0") == :lt do - Jason - else - JSON - end - if config_env() == :test do config :sentry, environment_name: :test, - json_library: json_library, tags: %{}, enable_source_code_context: true, root_source_code_paths: [File.cwd!()], @@ -23,4 +15,4 @@ if config_env() == :test do config :logger, backends: [] end -config :phoenix, :json_library, json_library +config :phoenix, :json_library, Jason diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index f21c3c51..87c124fc 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -294,10 +294,14 @@ defmodule Sentry.Client do end defp sanitize_non_jsonable_value(value, json_library) do - json_library.encode!(value) - :unchanged - rescue - _error -> {:changed, inspect(value)} + try do + json_library.encode(value) + catch + _type, _reason -> {:changed, inspect(value)} + else + {:ok, _encoded} -> :unchanged + {:error, _reason} -> {:changed, inspect(value)} + end end defp update_if_present(map, key, fun) do diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index 61f8b3b2..a2407512 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -152,7 +152,7 @@ defmodule Sentry.Config do type_doc: "`t:module/0`", doc: """ A module that implements the "standard" Elixir JSON behaviour, that is, exports the - `encode!/1` and `decode!/1` functions. If you use the default, make sure to add + `encode/1` and `decode/1` functions. If you use the default, make sure to add [`:jason`](https://hex.pm/packages/jason) as a dependency of your application. """ ], @@ -695,14 +695,19 @@ defmodule Sentry.Config do def __validate_json_library__(mod) when is_atom(mod) do try do - %{} = mod.decode!("{}") - "{}" = mod.encode!(%{}) - {:ok, mod} + with {:ok, %{}} <- mod.decode("{}"), + {:ok, "{}"} <- mod.encode(%{}) do + {:ok, mod} + else + _ -> + {:error, + "configured :json_library #{inspect(mod)} does not implement decode/1 and encode/1"} + end rescue UndefinedFunctionError -> {:error, """ - configured :json_library #{inspect(mod)} is not available or does not implement decode!/1 and encode!/1. + configured :json_library #{inspect(mod)} is not available or does not implement decode/1 and encode/1. Do you need to add #{inspect(mod)} to your mix.exs? """} end diff --git a/lib/sentry/envelope.ex b/lib/sentry/envelope.ex index d28c1e58..649f6e85 100644 --- a/lib/sentry/envelope.ex +++ b/lib/sentry/envelope.ex @@ -76,14 +76,19 @@ defmodule Sentry.Envelope do items_iodata = Enum.map(envelope.items, &item_to_binary(json_library, &1)) {:ok, IO.iodata_to_binary([headers_iodata, items_iodata])} - rescue - error -> {:error, error} + catch + {:error, _reason} = error -> error end defp item_to_binary(json_library, %Event{} = event) do - encoded_event = event |> Sentry.Client.render_event() |> json_library.encode!() - header = ~s({"type":"event","length":#{byte_size(encoded_event)}}) - [header, ?\n, encoded_event, ?\n] + case event |> Sentry.Client.render_event() |> json_library.encode() do + {:ok, encoded_event} -> + header = ~s({"type":"event","length":#{byte_size(encoded_event)}}) + [header, ?\n, encoded_event, ?\n] + + {:error, _reason} = error -> + throw(error) + end end defp item_to_binary(json_library, %Attachment{} = attachment) do @@ -95,20 +100,30 @@ defmodule Sentry.Envelope do into: header, do: {Atom.to_string(key), value} - header_iodata = json_library.encode!(header) + {:ok, header_iodata} = json_library.encode(header) [header_iodata, ?\n, attachment.data, ?\n] end defp item_to_binary(json_library, %CheckIn{} = check_in) do - encoded_check_in = check_in |> CheckIn.to_map() |> json_library.encode!() - header = ~s({"type":"check_in","length":#{byte_size(encoded_check_in)}}) - [header, ?\n, encoded_check_in, ?\n] + case check_in |> CheckIn.to_map() |> json_library.encode() do + {:ok, encoded_check_in} -> + header = ~s({"type":"check_in","length":#{byte_size(encoded_check_in)}}) + [header, ?\n, encoded_check_in, ?\n] + + {:error, _reason} = error -> + throw(error) + end end defp item_to_binary(json_library, %ClientReport{} = client_report) do - encoded_client_report = client_report |> Map.from_struct() |> json_library.encode!() - header = ~s({"type":"client_report","length":#{byte_size(encoded_client_report)}}) - [header, ?\n, encoded_client_report, ?\n] + case client_report |> Map.from_struct() |> json_library.encode() do + {:ok, encoded_client_report} -> + header = ~s({"type":"client_report","length":#{byte_size(encoded_client_report)}}) + [header, ?\n, encoded_client_report, ?\n] + + {:error, _reason} = error -> + throw(error) + end end end diff --git a/lib/sentry/transport.ex b/lib/sentry/transport.ex index 4c94387d..a353b2b8 100644 --- a/lib/sentry/transport.ex +++ b/lib/sentry/transport.ex @@ -93,10 +93,11 @@ defmodule Sentry.Transport do end defp request(client, endpoint, headers, body) do - case client_post_and_validate_return_value(client, endpoint, headers, body) do - {:ok, 200, _headers, body} -> - {:ok, body |> Config.json_library().decode!() |> Map.get("id")} - + with {:ok, 200, _headers, body} <- + client_post_and_validate_return_value(client, endpoint, headers, body), + {:ok, json} <- Config.json_library().decode(body) do + {:ok, Map.get(json, "id")} + else {:ok, 429, headers, _body} -> delay_ms = with timeout when is_binary(timeout) <- diff --git a/test/envelope_test.exs b/test/envelope_test.exs index 702e3eee..dd887d3d 100644 --- a/test/envelope_test.exs +++ b/test/envelope_test.exs @@ -15,10 +15,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert json_library().decode!(id_line) == %{"event_id" => event.event_id} - assert %{"type" => "event", "length" => _} = json_library().decode!(header_line) + assert Jason.decode!(id_line) == %{"event_id" => event.event_id} + assert %{"type" => "event", "length" => _} = Jason.decode!(header_line) - assert {:ok, decoded_event} = json_library().decode(event_line) + assert {:ok, decoded_event} = Jason.decode(event_line) assert decoded_event["event_id"] == event.event_id assert decoded_event["breadcrumbs"] == [] assert decoded_event["environment"] == "test" @@ -65,29 +65,29 @@ defmodule Sentry.EnvelopeTest do "..." ] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = json_library().decode!(id_line) + assert %{"event_id" => _} = Jason.decode!(id_line) - assert json_library().decode!(attachment1_header) == %{ + assert Jason.decode!(attachment1_header) == %{ "type" => "attachment", "length" => 3, "filename" => "example.dat" } - assert json_library().decode!(attachment2_header) == %{ + assert Jason.decode!(attachment2_header) == %{ "type" => "attachment", "length" => 6, "filename" => "example.txt", "content_type" => "text/plain" } - assert json_library().decode!(attachment3_header) == %{ + assert Jason.decode!(attachment3_header) == %{ "type" => "attachment", "length" => 2, "filename" => "example.json", "content_type" => "application/json" } - assert json_library().decode!(attachment4_header) == %{ + assert Jason.decode!(attachment4_header) == %{ "type" => "attachment", "length" => 3, "filename" => "dump", @@ -105,10 +105,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = json_library().decode!(id_line) - assert %{"type" => "check_in", "length" => _} = json_library().decode!(header_line) + assert %{"event_id" => _} = Jason.decode!(id_line) + assert %{"type" => "check_in", "length" => _} = Jason.decode!(header_line) - assert {:ok, decoded_check_in} = json_library().decode(event_line) + assert {:ok, decoded_check_in} = Jason.decode(event_line) assert decoded_check_in["check_in_id"] == check_in_id assert decoded_check_in["monitor_slug"] == "test" assert decoded_check_in["status"] == "ok" @@ -128,10 +128,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = json_library().decode!(id_line) - assert %{"type" => "client_report", "length" => _} = json_library().decode!(header_line) + assert %{"event_id" => _} = Jason.decode!(id_line) + assert %{"type" => "client_report", "length" => _} = Jason.decode!(header_line) - assert {:ok, decoded_client_report} = json_library().decode(event_line) + assert {:ok, decoded_client_report} = Jason.decode(event_line) assert decoded_client_report["timestamp"] == client_report.timestamp assert decoded_client_report["discarded_events"] == [ diff --git a/test/mix/sentry.package_source_code_test.exs b/test/mix/sentry.package_source_code_test.exs index bf7e722b..f60474b9 100644 --- a/test/mix/sentry.package_source_code_test.exs +++ b/test/mix/sentry.package_source_code_test.exs @@ -66,8 +66,8 @@ defmodule Mix.Tasks.Sentry.PackageSourceCodeTest do # "loadpaths" and "compile" to the dependencies of this Mix task. test "supports custom configured :json_library" do defmodule Sentry.ExampleJSON do - defdelegate encode!(term), to: json_library() - defdelegate decode!(term), to: json_library() + defdelegate encode(term), to: Jason + defdelegate decode(term), to: Jason end put_test_config(json_library: Sentry.ExampleJSON) diff --git a/test/plug_capture_test.exs b/test/plug_capture_test.exs index 8480483f..226fc2cf 100644 --- a/test/plug_capture_test.exs +++ b/test/plug_capture_test.exs @@ -29,7 +29,7 @@ defmodule Sentry.PlugCaptureTest do use Phoenix.Endpoint, otp_app: :sentry use Plug.Debugger, otp_app: :sentry - plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: json_library() + plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason plug Sentry.PlugContext plug PhoenixRouter end @@ -45,7 +45,7 @@ defmodule Sentry.PlugCaptureTest do use Phoenix.Endpoint, otp_app: :sentry use Plug.Debugger, otp_app: :sentry - plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: json_library() + plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason plug Sentry.PlugContext plug PhoenixRouter end diff --git a/test/sentry/client_test.exs b/test/sentry/client_test.exs index fa46997e..8e11ab73 100644 --- a/test/sentry/client_test.exs +++ b/test/sentry/client_test.exs @@ -67,10 +67,10 @@ defmodule Sentry.ClientTest do test "works if the JSON library crashes" do defmodule RaisingJSONClient do - def encode!(:crash), do: raise("Oops") - def encode!(term), do: json_library().encode!(term) + def encode(:crash), do: raise("Oops") + def encode(term), do: Jason.encode(term) - def decode!(term), do: json_library().decode!(term) + def decode(term), do: Jason.decode(term) end put_test_config(json_library: RaisingJSONClient) @@ -336,10 +336,10 @@ defmodule Sentry.ClientTest do test "logs an error when unable to encode JSON" do defmodule BadJSONClient do - def encode!(term) when term == %{}, do: "{}" - def encode!(_term), do: raise("im_just_bad") + def encode(term) when term == %{}, do: {:ok, "{}"} + def encode(_term), do: {:error, :im_just_bad} - def decode!(term), do: json_library().decode!(term) + def decode(term), do: Jason.decode(term) end put_test_config(json_library: BadJSONClient) @@ -347,7 +347,7 @@ defmodule Sentry.ClientTest do assert capture_log(fn -> Client.send_event(event, result: :sync) - end) =~ "the Sentry SDK could not encode the event to JSON: im_just_bad" + end) =~ "the Sentry SDK could not encode the event to JSON: :im_just_bad" end test "uses the async sender pool when :result is :none", %{bypass: bypass} do diff --git a/test/sentry/config_test.exs b/test/sentry/config_test.exs index 04e64f06..3637c712 100644 --- a/test/sentry/config_test.exs +++ b/test/sentry/config_test.exs @@ -158,7 +158,7 @@ defmodule Sentry.ConfigTest do end test ":json_library" do - assert Config.validate!(json_library: JSON)[:json_library] == JSON + assert Config.validate!(json_library: Jason)[:json_library] == Jason # Default assert Config.validate!([])[:json_library] == Jason diff --git a/test/sentry/transport_test.exs b/test/sentry/transport_test.exs index 10a64507..0b7d7e85 100644 --- a/test/sentry/transport_test.exs +++ b/test/sentry/transport_test.exs @@ -158,10 +158,10 @@ defmodule Sentry.TransportTest do envelope = Envelope.from_event(Event.create_event(message: "Hello")) defmodule CrashingJSONLibrary do - defdelegate encode!(term), to: json_library() + defdelegate encode(term), to: Jason - def decode!("{}"), do: %{} - def decode!(_body), do: raise("I'm a really bad JSON library") + def decode("{}"), do: {:ok, %{}} + def decode(_body), do: raise("I'm a really bad JSON library") end Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> @@ -190,9 +190,7 @@ defmodule Sentry.TransportTest do Plug.Conn.resp(conn, 200, ~s) end) - exception = Module.concat(json_library(), DecodeError) - - assert {:error, %^exception{}, _stacktrace} = + assert {:request_failure, %Jason.DecodeError{}} = error(fn -> Transport.encode_and_post_envelope(envelope, HackneyClient, _retries = [0]) end) diff --git a/test/support/example_plug_application.ex b/test/support/example_plug_application.ex index 082ffe1a..ee6e88c5 100644 --- a/test/support/example_plug_application.ex +++ b/test/support/example_plug_application.ex @@ -5,8 +5,6 @@ defmodule Sentry.ExamplePlugApplication do import ExUnit.Assertions - alias Sentry.Config - plug Plug.Parsers, parsers: [:multipart, :urlencoded] plug Sentry.PlugContext plug :match @@ -52,7 +50,7 @@ defmodule Sentry.ExamplePlugApplication do {event_id, :plug} -> opts = %{title: "Testing", eventId: event_id} - |> Config.json_library().encode!() + |> Jason.encode!() """ diff --git a/test/support/test_error_view.ex b/test/support/test_error_view.ex index 473c6511..29dcd7e4 100644 --- a/test/support/test_error_view.ex +++ b/test/support/test_error_view.ex @@ -1,17 +1,14 @@ defmodule Sentry.ErrorView do use Phoenix.Component - import Phoenix.HTML, only: [raw: 1] - alias Sentry.Config - def render(_, _) do case Sentry.get_last_event_id_and_source() do {event_id, :plug} -> opts = %{title: "Testing", eventId: event_id} - |> Config.json_library().encode!() + |> Jason.encode!() assigns = %{opts: opts} diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 3d45703a..eca6a617 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -3,9 +3,6 @@ defmodule Sentry.TestHelpers do alias Sentry.Config - @spec json_library :: module() - def json_library, do: Config.json_library() - @spec put_test_config(keyword()) :: :ok def put_test_config(config) when is_list(config) do all_original_config = all_config() @@ -49,7 +46,7 @@ defmodule Sentry.TestHelpers do @spec decode_envelope!(binary()) :: [{header :: map(), item :: map()}] def decode_envelope!(binary) do [id_line | rest] = String.split(binary, "\n") - %{"event_id" => _} = json_library().decode!(id_line) + {:ok, %{"event_id" => _}} = Config.json_library().decode(id_line) decode_envelope_items(rest) end @@ -58,8 +55,8 @@ defmodule Sentry.TestHelpers do |> Enum.chunk_every(2) |> Enum.flat_map(fn [header, item] -> - header = json_library().decode!(header) - item = json_library().decode!(item) + {:ok, header} = Config.json_library().decode(header) + {:ok, item} = Config.json_library().decode(item) [{header, item}] [""] -> diff --git a/test_integrations/phoenix_app/test/support/test_helpers.ex b/test_integrations/phoenix_app/test/support/test_helpers.ex index 7cdfede7..eca6a617 100644 --- a/test_integrations/phoenix_app/test/support/test_helpers.ex +++ b/test_integrations/phoenix_app/test/support/test_helpers.ex @@ -46,7 +46,7 @@ defmodule Sentry.TestHelpers do @spec decode_envelope!(binary()) :: [{header :: map(), item :: map()}] def decode_envelope!(binary) do [id_line | rest] = String.split(binary, "\n") - %{"event_id" => _} = Config.json_library().decode!(id_line) + {:ok, %{"event_id" => _}} = Config.json_library().decode(id_line) decode_envelope_items(rest) end @@ -55,8 +55,8 @@ defmodule Sentry.TestHelpers do |> Enum.chunk_every(2) |> Enum.flat_map(fn [header, item] -> - header = Config.json_library().decode!(header) - item = Config.json_library().decode!(item) + {:ok, header} = Config.json_library().decode(header) + {:ok, item} = Config.json_library().decode(item) [{header, item}] [""] -> From 96b645c17aa3fd4c221ff60c445e0633fcec20b3 Mon Sep 17 00:00:00 2001 From: Jonathan Moraes Date: Thu, 2 Jan 2025 11:54:55 +0100 Subject: [PATCH 3/4] Use middleman for JSON serialization --- README.md | 27 ++++++++++++-- config/config.exs | 2 +- lib/sentry/client.ex | 26 +++++++------- lib/sentry/config.ex | 14 +++++--- lib/sentry/envelope.ex | 22 ++++++------ lib/sentry/json.ex | 28 +++++++++++++++ lib/sentry/transport.ex | 2 +- pages/setup-with-plug-and-phoenix.md | 10 +++--- test/envelope_test.exs | 28 +++++++-------- test/mix/sentry.package_source_code_test.exs | 14 -------- test/plug_capture_test.exs | 4 +-- test/sentry/client_test.exs | 35 ------------------- test/sentry/config_test.exs | 30 ++++++++++------ test/sentry/json_test.exs | 23 ++++++++++++ test/sentry/transport_test.exs | 28 +-------------- test/support/example_plug_application.ex | 4 ++- test/support/test_error_view.ex | 4 ++- test/support/test_helpers.ex | 21 ++++++++--- .../phoenix_app/config/config.exs | 3 +- .../phoenix_app/test/support/test_helpers.ex | 18 +++++++--- 20 files changed, 188 insertions(+), 155 deletions(-) create mode 100644 lib/sentry/json.ex create mode 100644 test/sentry/json_test.exs diff --git a/README.md b/README.md index 4dbd358f..2b2c83a2 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,9 @@ This is the official Sentry SDK for [Sentry]. ### Install -To use Sentry in your project, add it as a dependency in your `mix.exs` file. Sentry does not install a JSON library nor HTTP client by itself. Sentry will default to trying to use [Jason] for JSON serialization and [Hackney] for HTTP requests, but can be configured to use other ones. To use the default ones, do: +To use Sentry in your project, add it as a dependency in your `mix.exs` file. + +Sentry does not install a JSON library nor HTTP client by itself. Sentry will default to trying to use [Hackney] for HTTP requests, but can be configured to use other ones. To use the default ones, do: ```elixir defp deps do @@ -26,12 +28,33 @@ defp deps do # ... {:sentry, "~> 10.0"}, - {:jason, "~> 1.4"}, {:hackney, "~> 1.19"} ] end ``` +For Elixir 1.18+, `JSON` kernel module will be used by default to serialize JSON data. + +For Elixir lower than 1.18, Sentry will default to trying to use [Jason] for JSON serialization. To use it, do: + +```elixir +defp deps do + [ + # ... + + {:sentry, "~> 10.0"}, + {:jason, "~> 1.4"} + ] +end +``` + +To use `Jason` or other JSON library for Elixir 1.18+, it is required to define it as a compile-time configuration: + +```elixir +# config.exs +config :sentry, json_library: Jason +``` + ### Configuration Sentry has a range of configuration options, but most applications will have a configuration that looks like the following: diff --git a/config/config.exs b/config/config.exs index 1184ea49..d63cc9d2 100644 --- a/config/config.exs +++ b/config/config.exs @@ -15,4 +15,4 @@ if config_env() == :test do config :logger, backends: [] end -config :phoenix, :json_library, Jason +config :phoenix, :json_library, if(Code.ensure_loaded?(JSON), do: JSON, else: Jason) diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index 87c124fc..cad1d4b7 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -207,8 +207,6 @@ defmodule Sentry.Client do @spec render_event(Event.t()) :: map() def render_event(%Event{} = event) do - json_library = Config.json_library() - event |> Event.remove_non_payload_keys() |> update_if_present(:breadcrumbs, fn bcs -> Enum.map(bcs, &Map.from_struct/1) end) @@ -218,9 +216,9 @@ defmodule Sentry.Client do Map.from_struct(message) end) |> update_if_present(:request, &(&1 |> Map.from_struct() |> remove_nils())) - |> update_if_present(:extra, &sanitize_non_jsonable_values(&1, json_library)) - |> update_if_present(:user, &sanitize_non_jsonable_values(&1, json_library)) - |> update_if_present(:tags, &sanitize_non_jsonable_values(&1, json_library)) + |> update_if_present(:extra, &sanitize_non_jsonable_values/1) + |> update_if_present(:user, &sanitize_non_jsonable_values/1) + |> update_if_present(:tags, &sanitize_non_jsonable_values/1) |> update_if_present(:exception, fn list -> Enum.map(list, &render_exception/1) end) |> update_if_present(:threads, fn list -> Enum.map(list, &render_thread/1) end) end @@ -255,11 +253,11 @@ defmodule Sentry.Client do :maps.filter(fn _key, value -> not is_nil(value) end, map) end - defp sanitize_non_jsonable_values(map, json_library) do + defp sanitize_non_jsonable_values(map) do # We update the existing map instead of building a new one from scratch # due to performance reasons. See the docs for :maps.map/2. Enum.reduce(map, map, fn {key, value}, acc -> - case sanitize_non_jsonable_value(value, json_library) do + case sanitize_non_jsonable_value(value) do :unchanged -> acc {:changed, value} -> Map.put(acc, key, value) end @@ -267,15 +265,15 @@ defmodule Sentry.Client do end # For performance, skip all the keys that we know for sure are JSON encodable. - defp sanitize_non_jsonable_value(value, _json_library) + defp sanitize_non_jsonable_value(value) when is_binary(value) or is_number(value) or is_boolean(value) or is_nil(value) do :unchanged end - defp sanitize_non_jsonable_value(value, json_library) when is_list(value) do + defp sanitize_non_jsonable_value(value) when is_list(value) do {mapped, changed?} = Enum.map_reduce(value, _changed? = false, fn value, changed? -> - case sanitize_non_jsonable_value(value, json_library) do + case sanitize_non_jsonable_value(value) do :unchanged -> {value, changed?} {:changed, value} -> {value, true} end @@ -288,14 +286,14 @@ defmodule Sentry.Client do end end - defp sanitize_non_jsonable_value(value, json_library) + defp sanitize_non_jsonable_value(value) when is_map(value) and not is_struct(value) do - {:changed, sanitize_non_jsonable_values(value, json_library)} + {:changed, sanitize_non_jsonable_values(value)} end - defp sanitize_non_jsonable_value(value, json_library) do + defp sanitize_non_jsonable_value(value) do try do - json_library.encode(value) + Sentry.JSON.encode(value) catch _type, _reason -> {:changed, inspect(value)} else diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index a2407512..91d01a97 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -148,11 +148,16 @@ defmodule Sentry.Config do ], json_library: [ type: {:custom, __MODULE__, :__validate_json_library__, []}, - default: Jason, + deprecated: "JSON kernel module is available for Elixir 1.18+.", type_doc: "`t:module/0`", doc: """ A module that implements the "standard" Elixir JSON behaviour, that is, exports the - `encode/1` and `decode/1` functions. If you use the default, make sure to add + `encode/1` and `decode/1` functions. + + This configuration should be set at compile-time. + + Defaults to `Jason` if `JSON` kernel module is not available. If you use the + default configuration with Elixir version lower than 1.18, make sure to add [`:jason`](https://hex.pm/packages/jason) as a dependency of your application. """ ], @@ -569,9 +574,6 @@ defmodule Sentry.Config do @spec report_deps?() :: boolean() def report_deps?, do: fetch!(:report_deps) - @spec json_library() :: module() - def json_library, do: fetch!(:json_library) - @spec log_level() :: :debug | :info | :warning | :warn | :error def log_level, do: fetch!(:log_level) @@ -693,6 +695,8 @@ defmodule Sentry.Config do {:error, "nil is not a valid value for the :json_library option"} end + def __validate_json_library__(JSON = mod), do: {:ok, mod} + def __validate_json_library__(mod) when is_atom(mod) do try do with {:ok, %{}} <- mod.decode("{}"), diff --git a/lib/sentry/envelope.ex b/lib/sentry/envelope.ex index 649f6e85..6b9e0802 100644 --- a/lib/sentry/envelope.ex +++ b/lib/sentry/envelope.ex @@ -2,7 +2,7 @@ defmodule Sentry.Envelope do @moduledoc false # https://develop.sentry.dev/sdk/envelopes/ - alias Sentry.{Attachment, CheckIn, ClientReport, Config, Event, UUID} + alias Sentry.{Attachment, CheckIn, ClientReport, Event, UUID} @type t() :: %__MODULE__{ event_id: UUID.t(), @@ -65,23 +65,21 @@ defmodule Sentry.Envelope do """ @spec to_binary(t()) :: {:ok, binary()} | {:error, any()} def to_binary(%__MODULE__{} = envelope) do - json_library = Config.json_library() - headers_iodata = case envelope.event_id do nil -> "{{}}\n" event_id -> ~s({"event_id":"#{event_id}"}\n) end - items_iodata = Enum.map(envelope.items, &item_to_binary(json_library, &1)) + items_iodata = Enum.map(envelope.items, &item_to_binary/1) {:ok, IO.iodata_to_binary([headers_iodata, items_iodata])} catch {:error, _reason} = error -> error end - defp item_to_binary(json_library, %Event{} = event) do - case event |> Sentry.Client.render_event() |> json_library.encode() do + defp item_to_binary(%Event{} = event) do + case event |> Sentry.Client.render_event() |> Sentry.JSON.encode() do {:ok, encoded_event} -> header = ~s({"type":"event","length":#{byte_size(encoded_event)}}) [header, ?\n, encoded_event, ?\n] @@ -91,7 +89,7 @@ defmodule Sentry.Envelope do end end - defp item_to_binary(json_library, %Attachment{} = attachment) do + defp item_to_binary(%Attachment{} = attachment) do header = %{"type" => "attachment", "length" => byte_size(attachment.data)} header = @@ -100,13 +98,13 @@ defmodule Sentry.Envelope do into: header, do: {Atom.to_string(key), value} - {:ok, header_iodata} = json_library.encode(header) + {:ok, header_iodata} = Sentry.JSON.encode(header) [header_iodata, ?\n, attachment.data, ?\n] end - defp item_to_binary(json_library, %CheckIn{} = check_in) do - case check_in |> CheckIn.to_map() |> json_library.encode() do + defp item_to_binary(%CheckIn{} = check_in) do + case check_in |> CheckIn.to_map() |> Sentry.JSON.encode() do {:ok, encoded_check_in} -> header = ~s({"type":"check_in","length":#{byte_size(encoded_check_in)}}) [header, ?\n, encoded_check_in, ?\n] @@ -116,8 +114,8 @@ defmodule Sentry.Envelope do end end - defp item_to_binary(json_library, %ClientReport{} = client_report) do - case client_report |> Map.from_struct() |> json_library.encode() do + defp item_to_binary(%ClientReport{} = client_report) do + case client_report |> Map.from_struct() |> Sentry.JSON.encode() do {:ok, encoded_client_report} -> header = ~s({"type":"client_report","length":#{byte_size(encoded_client_report)}}) [header, ?\n, encoded_client_report, ?\n] diff --git a/lib/sentry/json.ex b/lib/sentry/json.ex new file mode 100644 index 00000000..0052e28b --- /dev/null +++ b/lib/sentry/json.ex @@ -0,0 +1,28 @@ +defmodule Sentry.JSON do + @moduledoc false + + @default_library if(Code.ensure_loaded?(JSON), do: JSON, else: Jason) + @library Application.compile_env(:sentry, :json_library) || @default_library + + @spec decode(String.t()) :: {:ok, term()} | {:error, term()} + if @library == JSON do + def decode(binary) do + {:ok, JSON.decode!(binary)} + rescue + error -> {:error, error} + end + else + defdelegate decode(binary), to: @library + end + + @spec encode(term()) :: {:ok, String.t()} | {:error, term()} + if @library == JSON do + def encode(data) do + {:ok, JSON.encode!(data)} + rescue + error -> {:error, error} + end + else + defdelegate encode(data), to: @library + end +end diff --git a/lib/sentry/transport.ex b/lib/sentry/transport.ex index a353b2b8..91c6a170 100644 --- a/lib/sentry/transport.ex +++ b/lib/sentry/transport.ex @@ -95,7 +95,7 @@ defmodule Sentry.Transport do defp request(client, endpoint, headers, body) do with {:ok, 200, _headers, body} <- client_post_and_validate_return_value(client, endpoint, headers, body), - {:ok, json} <- Config.json_library().decode(body) do + {:ok, json} <- Sentry.JSON.decode(body) do {:ok, Map.get(json, "id")} else {:ok, 429, headers, _body} -> diff --git a/pages/setup-with-plug-and-phoenix.md b/pages/setup-with-plug-and-phoenix.md index 00271923..64c2cd1a 100644 --- a/pages/setup-with-plug-and-phoenix.md +++ b/pages/setup-with-plug-and-phoenix.md @@ -6,8 +6,8 @@ You can capture errors in Plug (and Phoenix) applications with `Sentry.PlugConte If you are using Phoenix: - 1. Add `Sentry.PlugCapture` above the `use Phoenix.Endpoint` line in your endpoint file - 1. Add `Sentry.PlugContext` below `Plug.Parsers` +1. Add `Sentry.PlugCapture` above the `use Phoenix.Endpoint` line in your endpoint file +1. Add `Sentry.PlugContext` below `Plug.Parsers` ```diff defmodule MyAppWeb.Endpoint @@ -51,7 +51,7 @@ defmodule MyAppWeb.ErrorView do def render("500.html", _assigns) do case Sentry.get_last_event_id_and_source() do {event_id, :plug} when is_binary(event_id) -> - opts = Jason.encode!(%{eventId: event_id}) + opts = JSON.encode!(%{eventId: event_id}) ~E""" @@ -72,8 +72,8 @@ end If you are in a non-Phoenix Plug application: - 1. Add `Sentry.PlugCapture` at the top of your Plug application - 1. Add `Sentry.PlugContext` below `Plug.Parsers` (if it is in your stack) +1. Add `Sentry.PlugCapture` at the top of your Plug application +1. Add `Sentry.PlugContext` below `Plug.Parsers` (if it is in your stack) ```diff defmodule MyApp.Router do diff --git a/test/envelope_test.exs b/test/envelope_test.exs index dd887d3d..0acde830 100644 --- a/test/envelope_test.exs +++ b/test/envelope_test.exs @@ -15,10 +15,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert Jason.decode!(id_line) == %{"event_id" => event.event_id} - assert %{"type" => "event", "length" => _} = Jason.decode!(header_line) + assert decode!(id_line) == %{"event_id" => event.event_id} + assert %{"type" => "event", "length" => _} = decode!(header_line) - assert {:ok, decoded_event} = Jason.decode(event_line) + assert decoded_event = decode!(event_line) assert decoded_event["event_id"] == event.event_id assert decoded_event["breadcrumbs"] == [] assert decoded_event["environment"] == "test" @@ -65,29 +65,29 @@ defmodule Sentry.EnvelopeTest do "..." ] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = Jason.decode!(id_line) + assert %{"event_id" => _} = decode!(id_line) - assert Jason.decode!(attachment1_header) == %{ + assert decode!(attachment1_header) == %{ "type" => "attachment", "length" => 3, "filename" => "example.dat" } - assert Jason.decode!(attachment2_header) == %{ + assert decode!(attachment2_header) == %{ "type" => "attachment", "length" => 6, "filename" => "example.txt", "content_type" => "text/plain" } - assert Jason.decode!(attachment3_header) == %{ + assert decode!(attachment3_header) == %{ "type" => "attachment", "length" => 2, "filename" => "example.json", "content_type" => "application/json" } - assert Jason.decode!(attachment4_header) == %{ + assert decode!(attachment4_header) == %{ "type" => "attachment", "length" => 3, "filename" => "dump", @@ -105,10 +105,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = Jason.decode!(id_line) - assert %{"type" => "check_in", "length" => _} = Jason.decode!(header_line) + assert %{"event_id" => _} = decode!(id_line) + assert %{"type" => "check_in", "length" => _} = decode!(header_line) - assert {:ok, decoded_check_in} = Jason.decode(event_line) + assert decoded_check_in = decode!(event_line) assert decoded_check_in["check_in_id"] == check_in_id assert decoded_check_in["monitor_slug"] == "test" assert decoded_check_in["status"] == "ok" @@ -128,10 +128,10 @@ defmodule Sentry.EnvelopeTest do assert {:ok, encoded} = Envelope.to_binary(envelope) assert [id_line, header_line, event_line] = String.split(encoded, "\n", trim: true) - assert %{"event_id" => _} = Jason.decode!(id_line) - assert %{"type" => "client_report", "length" => _} = Jason.decode!(header_line) + assert %{"event_id" => _} = decode!(id_line) + assert %{"type" => "client_report", "length" => _} = decode!(header_line) - assert {:ok, decoded_client_report} = Jason.decode(event_line) + assert decoded_client_report = decode!(event_line) assert decoded_client_report["timestamp"] == client_report.timestamp assert decoded_client_report["discarded_events"] == [ diff --git a/test/mix/sentry.package_source_code_test.exs b/test/mix/sentry.package_source_code_test.exs index f60474b9..ab2db4fd 100644 --- a/test/mix/sentry.package_source_code_test.exs +++ b/test/mix/sentry.package_source_code_test.exs @@ -61,20 +61,6 @@ defmodule Mix.Tasks.Sentry.PackageSourceCodeTest do ]} = Process.info(self(), :messages) end - # This is not really a regression test, but something like was reported in - # https://github.com/getsentry/sentry-elixir/issues/760. Fixed by adding - # "loadpaths" and "compile" to the dependencies of this Mix task. - test "supports custom configured :json_library" do - defmodule Sentry.ExampleJSON do - defdelegate encode(term), to: Jason - defdelegate decode(term), to: Jason - end - - put_test_config(json_library: Sentry.ExampleJSON) - - assert :ok = Mix.Task.rerun("sentry.package_source_code") - end - test "supports --no-compile and --no-deps-check" do assert :ok = Mix.Task.rerun("sentry.package_source_code", ["--no-compile", "--no-deps-check"]) end diff --git a/test/plug_capture_test.exs b/test/plug_capture_test.exs index 226fc2cf..8480483f 100644 --- a/test/plug_capture_test.exs +++ b/test/plug_capture_test.exs @@ -29,7 +29,7 @@ defmodule Sentry.PlugCaptureTest do use Phoenix.Endpoint, otp_app: :sentry use Plug.Debugger, otp_app: :sentry - plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason + plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: json_library() plug Sentry.PlugContext plug PhoenixRouter end @@ -45,7 +45,7 @@ defmodule Sentry.PlugCaptureTest do use Phoenix.Endpoint, otp_app: :sentry use Plug.Debugger, otp_app: :sentry - plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: Jason + plug Plug.Parsers, parsers: [:json], pass: ["*/*"], json_decoder: json_library() plug Sentry.PlugContext plug PhoenixRouter end diff --git a/test/sentry/client_test.exs b/test/sentry/client_test.exs index 8e11ab73..bfa98339 100644 --- a/test/sentry/client_test.exs +++ b/test/sentry/client_test.exs @@ -65,25 +65,6 @@ defmodule Sentry.ClientTest do assert rendered.tags.tokens == inspect(MapSet.new([1])) end - test "works if the JSON library crashes" do - defmodule RaisingJSONClient do - def encode(:crash), do: raise("Oops") - def encode(term), do: Jason.encode(term) - - def decode(term), do: Jason.decode(term) - end - - put_test_config(json_library: RaisingJSONClient) - - event = Event.create_event(message: "Something went wrong", extra: %{crasher: :crash}) - - assert %{} = rendered = Client.render_event(event) - assert rendered.extra.crasher == ":crash" - after - :code.delete(RaisingJSONClient) - :code.purge(RaisingJSONClient) - end - test "renders stacktrace for threads" do event = Event.create_event( @@ -334,22 +315,6 @@ defmodule Sentry.ClientTest do assert error.reason == {:request_failure, :econnrefused} end - test "logs an error when unable to encode JSON" do - defmodule BadJSONClient do - def encode(term) when term == %{}, do: {:ok, "{}"} - def encode(_term), do: {:error, :im_just_bad} - - def decode(term), do: Jason.decode(term) - end - - put_test_config(json_library: BadJSONClient) - event = Event.create_event(message: "Something went wrong") - - assert capture_log(fn -> - Client.send_event(event, result: :sync) - end) =~ "the Sentry SDK could not encode the event to JSON: :im_just_bad" - end - test "uses the async sender pool when :result is :none", %{bypass: bypass} do test_pid = self() ref = make_ref() diff --git a/test/sentry/config_test.exs b/test/sentry/config_test.exs index 3637c712..ff3ca782 100644 --- a/test/sentry/config_test.exs +++ b/test/sentry/config_test.exs @@ -158,22 +158,30 @@ defmodule Sentry.ConfigTest do end test ":json_library" do - assert Config.validate!(json_library: Jason)[:json_library] == Jason + assert ExUnit.CaptureIO.capture_io(:stderr, fn -> + assert Config.validate!(json_library: Jason)[:json_library] == Jason + end) =~ + """ + :json_library option is deprecated. \ + JSON kernel module is available for Elixir 1.18+.\ + """ # Default - assert Config.validate!([])[:json_library] == Jason + assert Config.validate!([])[:json_library] == nil - assert_raise ArgumentError, ~r/invalid value for :json_library option/, fn -> - Config.validate!(json_library: Atom) - end + ExUnit.CaptureIO.capture_io(:stderr, fn -> + assert_raise ArgumentError, ~r/invalid value for :json_library option/, fn -> + Config.validate!(json_library: Atom) + end - assert_raise ArgumentError, ~r/invalid value for :json_library option/, fn -> - Config.validate!(json_library: nil) - end + assert_raise ArgumentError, ~r/invalid value for :json_library option/, fn -> + Config.validate!(json_library: nil) + end - assert_raise ArgumentError, ~r/invalid value for :json_library option/, fn -> - Config.validate!(json_library: "not a module") - end + assert_raise ArgumentError, ~r/invalid value for :json_library option/, fn -> + Config.validate!(json_library: "not a module") + end + end) end test ":before_send" do diff --git a/test/sentry/json_test.exs b/test/sentry/json_test.exs new file mode 100644 index 00000000..e029e915 --- /dev/null +++ b/test/sentry/json_test.exs @@ -0,0 +1,23 @@ +defmodule Sentry.JSONTest do + use ExUnit.Case, async: true + + describe "decode/1" do + test "decodes empty object to empty map" do + assert Sentry.JSON.decode("{}") == {:ok, %{}} + end + + test "returns {:error, reason} if binary is not a JSON" do + assert {:error, _reason} = Sentry.JSON.decode("not JSON") + end + end + + describe "encode/1" do + test "encodes empty map to empty object" do + assert Sentry.JSON.encode(%{}) == {:ok, "{}"} + end + + test "returns {:error, reason} if data cannot be parsed to JSON" do + assert {:error, _reason} = Sentry.JSON.encode({:ok, "will fail"}) + end + end +end diff --git a/test/sentry/transport_test.exs b/test/sentry/transport_test.exs index 0b7d7e85..b717cb48 100644 --- a/test/sentry/transport_test.exs +++ b/test/sentry/transport_test.exs @@ -153,32 +153,6 @@ defmodule Sentry.TransportTest do :code.purge(ThrowingHTTPClient) end - test "returns an error if the JSON library crashes when decoding the response", - %{bypass: bypass} do - envelope = Envelope.from_event(Event.create_event(message: "Hello")) - - defmodule CrashingJSONLibrary do - defdelegate encode(term), to: Jason - - def decode("{}"), do: {:ok, %{}} - def decode(_body), do: raise("I'm a really bad JSON library") - end - - Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> - Plug.Conn.resp(conn, 200, ~s) - end) - - put_test_config(json_library: CrashingJSONLibrary) - - assert {:error, %RuntimeError{message: "I'm a really bad JSON library"}, _stacktrace} = - error(fn -> - Transport.encode_and_post_envelope(envelope, HackneyClient, _retries = []) - end) - after - :code.delete(CrashingJSONLibrary) - :code.purge(CrashingJSONLibrary) - end - test "returns an error if the response from Sentry is not valid JSON, and retries", %{bypass: bypass} do envelope = Envelope.from_event(Event.create_event(message: "Hello")) @@ -190,7 +164,7 @@ defmodule Sentry.TransportTest do Plug.Conn.resp(conn, 200, ~s) end) - assert {:request_failure, %Jason.DecodeError{}} = + assert {:request_failure, _error} = error(fn -> Transport.encode_and_post_envelope(envelope, HackneyClient, _retries = [0]) end) diff --git a/test/support/example_plug_application.ex b/test/support/example_plug_application.ex index ee6e88c5..0fc5d637 100644 --- a/test/support/example_plug_application.ex +++ b/test/support/example_plug_application.ex @@ -5,6 +5,8 @@ defmodule Sentry.ExamplePlugApplication do import ExUnit.Assertions + alias Sentry.TestHelpers + plug Plug.Parsers, parsers: [:multipart, :urlencoded] plug Sentry.PlugContext plug :match @@ -50,7 +52,7 @@ defmodule Sentry.ExamplePlugApplication do {event_id, :plug} -> opts = %{title: "Testing", eventId: event_id} - |> Jason.encode!() + |> TestHelpers.encode!() """ diff --git a/test/support/test_error_view.ex b/test/support/test_error_view.ex index 29dcd7e4..fb8fb6e1 100644 --- a/test/support/test_error_view.ex +++ b/test/support/test_error_view.ex @@ -3,12 +3,14 @@ defmodule Sentry.ErrorView do import Phoenix.HTML, only: [raw: 1] + alias Sentry.TestHelpers + def render(_, _) do case Sentry.get_last_event_id_and_source() do {event_id, :plug} -> opts = %{title: "Testing", eventId: event_id} - |> Jason.encode!() + |> TestHelpers.encode!() assigns = %{opts: opts} diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index eca6a617..dcc31b57 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -1,7 +1,20 @@ defmodule Sentry.TestHelpers do import ExUnit.Assertions - alias Sentry.Config + @spec decode!(String.t()) :: term() + def decode!(binary) do + {:ok, data} = Sentry.JSON.decode(binary) + data + end + + @spec decode!(term()) :: String.t() + def encode!(data) do + {:ok, binary} = Sentry.JSON.encode(data) + binary + end + + @spec json_library() :: module() + def json_library(), do: if(Code.ensure_loaded?(JSON), do: JSON, else: Jason) @spec put_test_config(keyword()) :: :ok def put_test_config(config) when is_list(config) do @@ -46,7 +59,7 @@ defmodule Sentry.TestHelpers do @spec decode_envelope!(binary()) :: [{header :: map(), item :: map()}] def decode_envelope!(binary) do [id_line | rest] = String.split(binary, "\n") - {:ok, %{"event_id" => _}} = Config.json_library().decode(id_line) + %{"event_id" => _} = decode!(id_line) decode_envelope_items(rest) end @@ -55,8 +68,8 @@ defmodule Sentry.TestHelpers do |> Enum.chunk_every(2) |> Enum.flat_map(fn [header, item] -> - {:ok, header} = Config.json_library().decode(header) - {:ok, item} = Config.json_library().decode(item) + header = decode!(header) + item = decode!(item) [{header, item}] [""] -> diff --git a/test_integrations/phoenix_app/config/config.exs b/test_integrations/phoenix_app/config/config.exs index 1f7d4a0d..a0ce0afe 100644 --- a/test_integrations/phoenix_app/config/config.exs +++ b/test_integrations/phoenix_app/config/config.exs @@ -57,8 +57,7 @@ config :logger, :console, format: "$time $metadata[$level] $message\n", metadata: [:request_id] -# Use Jason for JSON parsing in Phoenix -config :phoenix, :json_library, Jason +config :phoenix, :json_library, if(Code.ensure_loaded?(JSON), do: JSON, else: Jason) # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. diff --git a/test_integrations/phoenix_app/test/support/test_helpers.ex b/test_integrations/phoenix_app/test/support/test_helpers.ex index eca6a617..0b3ab7a3 100644 --- a/test_integrations/phoenix_app/test/support/test_helpers.ex +++ b/test_integrations/phoenix_app/test/support/test_helpers.ex @@ -1,7 +1,17 @@ defmodule Sentry.TestHelpers do import ExUnit.Assertions - alias Sentry.Config + @spec decode!(String.t()) :: term() + def decode!(binary) do + {:ok, data} = Sentry.JSON.decode(binary) + data + end + + @spec decode!(term()) :: String.t() + def encode!(data) do + {:ok, binary} = Sentry.JSON.encode(data) + binary + end @spec put_test_config(keyword()) :: :ok def put_test_config(config) when is_list(config) do @@ -46,7 +56,7 @@ defmodule Sentry.TestHelpers do @spec decode_envelope!(binary()) :: [{header :: map(), item :: map()}] def decode_envelope!(binary) do [id_line | rest] = String.split(binary, "\n") - {:ok, %{"event_id" => _}} = Config.json_library().decode(id_line) + %{"event_id" => _} = decode!(id_line) decode_envelope_items(rest) end @@ -55,8 +65,8 @@ defmodule Sentry.TestHelpers do |> Enum.chunk_every(2) |> Enum.flat_map(fn [header, item] -> - {:ok, header} = Config.json_library().decode(header) - {:ok, item} = Config.json_library().decode(item) + header = decode!(header) + item = decode!(item) [{header, item}] [""] -> From f2bddfd4024c4d98f9ab0a0bdeaf3396c94ce707 Mon Sep 17 00:00:00 2001 From: Jonathan Moraes Date: Fri, 3 Jan 2025 12:16:12 +0100 Subject: [PATCH 4/4] Use default from Application.compile_env/3 --- lib/sentry/json.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sentry/json.ex b/lib/sentry/json.ex index 0052e28b..67fbeec1 100644 --- a/lib/sentry/json.ex +++ b/lib/sentry/json.ex @@ -2,7 +2,7 @@ defmodule Sentry.JSON do @moduledoc false @default_library if(Code.ensure_loaded?(JSON), do: JSON, else: Jason) - @library Application.compile_env(:sentry, :json_library) || @default_library + @library Application.compile_env(:sentry, :json_library, @default_library) @spec decode(String.t()) :: {:ok, term()} | {:error, term()} if @library == JSON do