Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error grouping issue #133

Merged
merged 5 commits into from
Feb 28, 2025
Merged

Fix error grouping issue #133

merged 5 commits into from
Feb 28, 2025

Conversation

odarriba
Copy link
Contributor

The errors are being grouped in a wrong way because a type mismatch comparison was done between the application name and the stacktrace application name.

The fix is to convert the configured application name to a string and be able to compare it with the stacktrace application name.

You can test it putting this script on the root project:

Mix.install([
  {:ecto_sql, "~> 3.0"},
  {:postgrex, "~> 0.1"},
  {:error_tracker, path: "."},
  {:bandit, "~> 1.0"}
])

### ErrorLogger config

Application.put_env(:error_tracker, :repo, Repo)
Application.put_env(:error_tracker, :otp_app, :myapp)
Application.put_env(:error_tracker, :enabled, true)

defmodule AddErrorTracker do
  use Ecto.Migration

  def up, do: ErrorTracker.Migration.up(version: 4)

  def down, do: ErrorTracker.Migration.down(version: 1)
end

### Common config

Application.put_env(:myapp, Repo, database: "mix_install_examples")

defmodule Repo do
  use Ecto.Repo, adapter: Ecto.Adapters.Postgres, otp_app: :myapp
end

defmodule Migration0 do
  use Ecto.Migration

  def change do
    create table("posts") do
      add(:title, :string)
      timestamps(type: :utc_datetime_usec)
    end
  end
end

defmodule Post do
  use Ecto.Schema

  schema "posts" do
    field(:title, :string)
    timestamps(type: :utc_datetime_usec)
  end
end

defmodule Main do
  import Ecto.Query, warn: false

  def migrate do
    Repo.__adapter__().storage_down(Repo.config())
    :ok = Repo.__adapter__().storage_up(Repo.config())
    {:ok, _} = Repo.start_link([])
    Ecto.Migrator.run(Repo, [{0, Migration0}, {1, AddErrorTracker}], :up, all: true, log_migrations_sql: :info)
  end

  def register_app_and_modules do
    :application.load({:application, :myapp, [
      {:description, ~c"My sample application"},
      {:vsn, ~c"0.1.0"},
      {:modules, [Router]},
      {:registered, [Router]},
      {:applications, [:kernel, :stdlib]}
    ]})
  end

  def run_http_server do
    Supervisor.start_link([{Bandit, [plug: Router, port: 1234]}], strategy: :one_for_one)
  end

  def make_requests do
    :inets.start()
    :httpc.request("http://localhost:1234/route1")
    :httpc.request("http://localhost:1234/route2")
  end

  def check_what_was_collected do
    ErrorTracker.Error
    |> Repo.all()
    |> IO.inspect()

    ErrorTracker.Occurrence
    |> Repo.all()
    |> Enum.map(&Enum.take(&1.stacktrace.lines, 2))
    |> IO.inspect()
  end
end

defmodule Router do
  use Plug.Router
  use ErrorTracker.Integrations.Plug

  plug(:match)
  plug(:dispatch)

  get "/route1" do
    Repo.get!(Post, 1)
    send_resp(conn, 200, "Anything")
  end

  get "/route2" do
    Repo.get!(Post, 2)
    send_resp(conn, 200, "Anything")
  end
end

Main.migrate()
Main.register_app_and_modules()
Main.run_http_server()
Main.make_requests()
Main.check_what_was_collected() # <- demonstrates the issue

Process.sleep(:infinity)

This closes #132

The errors are being grouped in a wrong way because a type mismatch comparison
was done between the application name and the stacktrace application name.

The fix is to convert the configured application name to a string and be able
to compare it with the stacktrace application name.
@odarriba odarriba added the bug Something isn't working label Feb 28, 2025
@odarriba odarriba requested a review from crbelaus February 28, 2025 16:42
@odarriba odarriba self-assigned this Feb 28, 2025
@odarriba odarriba force-pushed the fix-error-grouping-issue branch from 20bb533 to ab4b29e Compare February 28, 2025 17:29
Copy link
Contributor

@crbelaus crbelaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! I don't know how we missed that one.

@odarriba odarriba merged commit b563d7a into main Feb 28, 2025
4 checks passed
@odarriba odarriba deleted the fix-error-grouping-issue branch February 28, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Similar errors from different stacktraces are grouped together
2 participants