From cb24caf5c0ca0636628aca3891d61e6dc9db3863 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Sun, 10 Dec 2023 16:00:07 +0100 Subject: [PATCH] Move envelope decoding to test helpers (#672) * Move from_binary/1 to test helpers, all tests pass * Finish up * Dialyzer --- lib/sentry/envelope.ex | 83 ---------------------- test/envelope_test.exs | 129 +---------------------------------- test/plug_capture_test.exs | 39 ++++++----- test/sentry/client_test.exs | 12 ++-- test/support/test_helpers.ex | 29 ++++++-- 5 files changed, 48 insertions(+), 244 deletions(-) diff --git a/lib/sentry/envelope.ex b/lib/sentry/envelope.ex index 0bf8b05c..bdc0feda 100644 --- a/lib/sentry/envelope.ex +++ b/lib/sentry/envelope.ex @@ -56,87 +56,4 @@ defmodule Sentry.Envelope do error end end - - @doc """ - Decodes the envelope from its binary representation. - """ - @spec from_binary(String.t()) :: {:ok, t()} | {:error, :invalid_envelope} - def from_binary(binary) when is_binary(binary) do - json_library = Config.json_library() - - [raw_headers | raw_items] = String.split(binary, "\n") - - with {:ok, headers} <- json_library.decode(raw_headers), - {:ok, items} <- decode_items(raw_items, json_library) do - {:ok, - %__MODULE__{ - event_id: headers["event_id"] || nil, - items: items - }} - else - {:error, _json_error} -> {:error, :invalid_envelope} - end - end - - # - # Decoding - # - - # Steps over the item pairs in the envelope body. The item header is decoded - # first so it can be used to decode the item following it. - defp decode_items(raw_items, json_library) do - items = - raw_items - |> Enum.chunk_every(2, 2, :discard) - |> Enum.map(fn [k, v] -> - with {:ok, item_header} <- json_library.decode(k), - {:ok, item} <- decode_item(item_header, v, json_library) do - item - else - {:error, _reason} = error -> throw(error) - end - end) - - {:ok, items} - catch - {:error, reason} -> {:error, reason} - end - - defp decode_item(%{"type" => "event"}, data, json_library) do - result = json_library.decode(data) - - case result do - {:ok, fields} -> - {:ok, - %Event{ - breadcrumbs: fields["breadcrumbs"], - culprit: fields["culprit"], - environment: fields["environment"], - event_id: fields["event_id"], - source: fields["event_source"], - exception: List.wrap(fields["exception"]), - extra: fields["extra"], - fingerprint: fields["fingerprint"], - level: fields["level"], - message: fields["message"], - modules: fields["modules"], - original_exception: fields["original_exception"], - platform: fields["platform"], - release: fields["release"], - request: fields["request"], - server_name: fields["server_name"], - tags: fields["tags"], - timestamp: fields["timestamp"], - user: fields["user"] - }} - - {:error, e} -> - {:error, "Failed to decode event item: #{e}"} - end - end - - defp decode_item(%{"type" => type}, _data, _json_library), - do: {:error, "unexpected item type '#{type}'"} - - defp decode_item(_, _data, _json_library), do: {:error, "Missing item type header"} end diff --git a/test/envelope_test.exs b/test/envelope_test.exs index 984af0e5..24a283b7 100644 --- a/test/envelope_test.exs +++ b/test/envelope_test.exs @@ -3,134 +3,7 @@ defmodule Sentry.EnvelopeTest do import Sentry.TestHelpers - alias Sentry.{Envelope, Event, Interfaces} - - describe "from_binary/1" do - test "parses envelope with empty headers" do - raw = "{}\n" - - {:ok, envelope} = Envelope.from_binary(raw) - - assert envelope.event_id == nil - assert envelope.items == [] - end - - test "parses envelope with only headers" do - raw = "{\"event_id\":\"12c2d058d58442709aa2eca08bf20986\"}\n" - - {:ok, envelope} = Envelope.from_binary(raw) - - assert envelope.event_id == "12c2d058d58442709aa2eca08bf20986" - assert envelope.items == [] - end - - test "parses envelope containing an event" do - event = %Event{ - breadcrumbs: [], - environment: "test", - event_id: "1d208b37d9904203918a9c2125ea91fa", - source: nil, - exception: [], - extra: %{}, - fingerprint: ["{{ default }}"], - level: "error", - message: "hello", - modules: %{ - bypass: "2.1.0", - certifi: "2.6.1", - cowboy: "2.8.0", - cowboy_telemetry: "0.3.1", - cowlib: "2.9.1", - dialyxir: "1.1.0", - erlex: "0.2.6", - hackney: "1.17.4", - idna: "6.1.1", - jason: "1.2.2", - metrics: "1.0.1", - mime: "1.5.0", - mimerl: "1.2.0", - parse_trans: "3.3.1", - phoenix: "1.5.8", - phoenix_html: "2.14.3", - phoenix_pubsub: "2.0.0", - plug: "1.11.1", - plug_cowboy: "2.4.1", - plug_crypto: "1.2.2", - ranch: "1.7.1", - ssl_verify_fun: "1.1.6", - telemetry: "0.4.2", - unicode_util_compat: "0.7.0" - }, - original_exception: nil, - platform: :elixir, - release: nil, - request: %Interfaces.Request{}, - server_name: "john-linux", - tags: %{}, - timestamp: "2021-10-09T03:53:22", - user: %{} - } - - {:ok, raw_envelope} = - [event] - |> Envelope.new() - |> Envelope.to_binary() - - {:ok, envelope} = Envelope.from_binary(raw_envelope) - - assert envelope.event_id == event.event_id - - assert envelope.items == [ - %Event{ - breadcrumbs: [], - environment: "test", - event_id: "1d208b37d9904203918a9c2125ea91fa", - exception: [], - extra: %{}, - fingerprint: ["{{ default }}"], - level: "error", - message: "hello", - modules: %{ - "bypass" => "2.1.0", - "certifi" => "2.6.1", - "cowboy" => "2.8.0", - "cowboy_telemetry" => "0.3.1", - "cowlib" => "2.9.1", - "dialyxir" => "1.1.0", - "erlex" => "0.2.6", - "hackney" => "1.17.4", - "idna" => "6.1.1", - "jason" => "1.2.2", - "metrics" => "1.0.1", - "mime" => "1.5.0", - "mimerl" => "1.2.0", - "parse_trans" => "3.3.1", - "phoenix" => "1.5.8", - "phoenix_html" => "2.14.3", - "phoenix_pubsub" => "2.0.0", - "plug" => "1.11.1", - "plug_cowboy" => "2.4.1", - "plug_crypto" => "1.2.2", - "ranch" => "1.7.1", - "ssl_verify_fun" => "1.1.6", - "telemetry" => "0.4.2", - "unicode_util_compat" => "0.7.0" - }, - platform: "elixir", - release: nil, - request: %{}, - server_name: "john-linux", - tags: %{}, - timestamp: "2021-10-09T03:53:22", - user: %{} - } - ] - end - - test "returns an error if headers are missing" do - assert {:error, :invalid_envelope} = Envelope.from_binary("no newlines here") - end - end + alias Sentry.{Envelope, Event} describe "to_binary/1" do test "encodes an envelope" do diff --git a/test/plug_capture_test.exs b/test/plug_capture_test.exs index f11f5cc3..7099e7a8 100644 --- a/test/plug_capture_test.exs +++ b/test/plug_capture_test.exs @@ -47,10 +47,10 @@ defmodule Sentry.PlugCaptureTest do event = decode_event_from_envelope!(body) - assert event.request["url"] == "http://www.example.com/error_route" - assert event.request["method"] == "GET" - assert event.request["query_string"] == "" - assert event.request["data"] == %{} + assert event["request"]["url"] == "http://www.example.com/error_route" + assert event["request"]["method"] == "GET" + assert event["request"]["query_string"] == "" + assert event["request"]["data"] == %{} Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -63,9 +63,7 @@ defmodule Sentry.PlugCaptureTest do test "sends throws to Sentry", %{bypass: bypass} do Bypass.expect(bypass, fn conn -> {:ok, body, conn} = Plug.Conn.read_body(conn) - _event = decode_event_from_envelope!(body) - Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -75,9 +73,7 @@ defmodule Sentry.PlugCaptureTest do test "sends exits to Sentry", %{bypass: bypass} do Bypass.expect(bypass, fn conn -> {:ok, body, conn} = Plug.Conn.read_body(conn) - _event = decode_event_from_envelope!(body) - Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -94,9 +90,7 @@ defmodule Sentry.PlugCaptureTest do test "can render feedback form", %{bypass: bypass} do Bypass.expect(bypass, fn conn -> {:ok, body, conn} = Plug.Conn.read_body(conn) - _event = decode_event_from_envelope!(body) - Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -135,10 +129,10 @@ defmodule Sentry.PlugCaptureTest do event = decode_event_from_envelope!(body) - assert event.culprit == "Sentry.PlugCaptureTest.PhoenixController.error/2" + assert event["culprit"] == "Sentry.PlugCaptureTest.PhoenixController.error/2" - assert List.first(event.exception)["type"] == "RuntimeError" - assert List.first(event.exception)["value"] == "PhoenixError" + assert List.first(event["exception"])["type"] == "RuntimeError" + assert List.first(event["exception"])["value"] == "PhoenixError" Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -155,8 +149,8 @@ defmodule Sentry.PlugCaptureTest do event = decode_event_from_envelope!(body) - assert event.culprit == "Sentry.PlugCaptureTest.PhoenixController.exit/2" - assert event.message == "Uncaught exit - :test" + assert event["culprit"] == "Sentry.PlugCaptureTest.PhoenixController.exit/2" + assert event["message"] == "Uncaught exit - :test" Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -169,8 +163,8 @@ defmodule Sentry.PlugCaptureTest do event = decode_event_from_envelope!(body) - assert event.culprit == "Sentry.PlugCaptureTest.PhoenixController.throw/2" - assert event.message == "Uncaught throw - :test" + assert event["culprit"] == "Sentry.PlugCaptureTest.PhoenixController.throw/2" + assert event["message"] == "Uncaught throw - :test" Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -203,8 +197,8 @@ defmodule Sentry.PlugCaptureTest do assert_receive {^ref, sentry_body} event = decode_event_from_envelope!(sentry_body) - assert event.culprit == "Sentry.PlugCaptureTest.PhoenixController.action_clause_error/2" - assert [exception] = event.exception + assert event["culprit"] == "Sentry.PlugCaptureTest.PhoenixController.action_clause_error/2" + assert [exception] = event["exception"] assert exception["type"] == "Phoenix.ActionClauseError" assert exception["value"] =~ ~s(params: %{"password" => "*********"}) end @@ -235,7 +229,7 @@ defmodule Sentry.PlugCaptureTest do Bypass.expect(bypass, fn conn -> {:ok, body, conn} = Plug.Conn.read_body(conn) event = decode_event_from_envelope!(body) - assert event.culprit == "Sentry.PlugCaptureTest.PhoenixController.assigns/2" + assert event["culprit"] == "Sentry.PlugCaptureTest.PhoenixController.assigns/2" Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -250,4 +244,9 @@ defmodule Sentry.PlugCaptureTest do defp call_plug_app(conn), do: Plug.run(conn, [{Sentry.ExamplePlugApplication, []}]) defp call_phoenix_endpoint(conn), do: Plug.run(conn, [{PhoenixEndpoint, []}]) + + defp decode_event_from_envelope!(envelope) do + assert [{%{"type" => "event"}, event}] = decode_envelope!(envelope) + event + end end diff --git a/test/sentry/client_test.exs b/test/sentry/client_test.exs index 5cb2c9f2..0ea45bd6 100644 --- a/test/sentry/client_test.exs +++ b/test/sentry/client_test.exs @@ -85,10 +85,10 @@ defmodule Sentry.ClientTest do Bypass.expect(bypass, fn conn -> assert {:ok, body, conn} = Plug.Conn.read_body(conn) - event = decode_event_from_envelope!(body) + assert [{%{"type" => "event"}, event}] = decode_envelope!(body) - assert event.extra == %{"key" => "value"} - assert event.user["id"] == 1 + assert event["extra"] == %{"key" => "value"} + assert event["user"]["id"] == 1 Plug.Conn.resp(conn, 200, ~s<{"id": "340"}>) end) @@ -224,15 +224,15 @@ defmodule Sentry.ClientTest do event = fn -> assert_receive {^ref, body}, 1000 - decode_event_from_envelope!(body) + assert [{%{"type" => "event"}, event}] = decode_envelope!(body) + event end |> Stream.repeatedly() |> Stream.reject(&is_nil/1) |> Stream.take(10) |> Enum.at(0) - assert %Event{} = event - assert event.message == "Something went wrong" + assert event["message"] == "Something went wrong" end test "dedupes events", %{bypass: bypass} do diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 88c58f88..eca6a617 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -1,13 +1,7 @@ defmodule Sentry.TestHelpers do import ExUnit.Assertions - alias Sentry.Envelope - - @spec decode_event_from_envelope!(binary()) :: Sentry.Event.t() - def decode_event_from_envelope!(body) when is_binary(body) do - {:ok, %Envelope{items: items}} = Envelope.from_binary(body) - Enum.find(items, &is_struct(&1, Sentry.Event)) - end + alias Sentry.Config @spec put_test_config(keyword()) :: :ok def put_test_config(config) when is_list(config) do @@ -48,4 +42,25 @@ defmodule Sentry.TestHelpers do def all_config do Enum.sort(for {{:sentry_config, key}, value} <- :persistent_term.get(), do: {key, value}) end + + @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) + decode_envelope_items(rest) + end + + defp decode_envelope_items(items) do + items + |> 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, item}] + + [""] -> + [] + end) + end end