Fallback to ancestor's context if available?

[mtwilliams]

The :proc_lib module (and by extension Task module) record ancestors by shoving them into the process dictionary under $ancestors. Although a nasty hack, looking up context in from an ancestor would drastically help when trying to reproduce a bug.

Thoughts?

[mitchellhenke]

Thanks for the report. Is this for Sentry.Logger? I don't think this is something I'd feel comfortable adding without at least understanding the use case a little bit more.

My initial thought would be to have the children send a report on terminate? Or have the supervisor handle the child crashes and report from there?

[mtwilliams]

Specifically Sentry.Context.get_context/0.

Something like,

defp get_context, do: get_context(self())
defp get_context(process) do
  [dictionary: dict] = :erlang.process_info(process, [:dictionary])
  ancestor = Map.get(dict, :"$ancestors", []) |> Enum.first
  get_ancestor_context = fn -> if ancestor, do: get_context(ancestor)
  Map.get_lazy(dict, @process_dictionary_key, get_ancestor_context)
end

[mtwilliams]

This means code that makes use of Tasks, like below, will show what request caused the crash in App.Analysis.Connections.between/2.

get "/people/:id" do
  person = Task.async(fn -> App.Repository.get!(Person, :id) end)
  connections = Task.async(fn -> App.Analysis.Connections.between(id, viewer.id))
  contextualized = Map.merge(Task.await(person), %{connections: Task.await(connections)})
  PersonSerializer.json(contextualized, pretty: true)
end

[mitchellhenke]

Does that route use Sentry.Plug?

[mtwilliams]

It's in the chain, yes.

[mitchellhenke]

I see what you mean. Something along the lines of:

  def index(conn, _params) do
    Sentry.Context.set_user_context(%{id: 1})
    Task.async(fn -> 1 / 0 end)
    |> Task.await()
    render conn, "index.html"
  end

will crash but not include the user context that was set.

Adding the above code does not seem to work well within the current tests we have. I am interested in finding a solution, but there doesn't seem to be a standard way to go up the tree either.

Absent a solution along those lines in the near term, I would probably avoid linking async tasks, and use Task Supervisors and async_nolink to avoid having the task bring down the request. Then the task can exit on its own, and using yield or yield_many, the request process will maintain its context, and would crash on a bad match from the result, or continue as normal.

[mtwilliams]

Adding the above code does not seem to work well within the current tests we have.

Please elaborate.

I am interested in finding a solution, but there doesn't seem to be a standard way to go up the tree either.

Undefined, but unchanging too.

[mitchellhenke]

Apologies for being unclear. The code above causes tests to fail, and I'm not sure this feature is something we want to add if it depends on behavior that is not currently well-defined.

I would gladly review a PR for this as I think it would better illustrate how the feature could fit into the library.

[mtwilliams]

Why do the test fail?

The code I gave was illustrative – wrote it off the top of my head.

[mitchellhenke]

I don't recall, as I only tried briefly.

I am going to close this issue, as without more specifics around the implementation, it is difficult to discuss. If you would like to submit a PR for the feature with tests, I would be happy to review and provide feedback. 🙂

[axelson]

Note: the elixir 1.8.0 release adds automatic caller tracking which would be very useful in this scenario.

https://elixir-lang.org/blog/2019/01/14/elixir-v1-8-0-released/

[mitchellhenke]

@axelson this is something definitely I plan to look into 🙂

[tfwright]

Hey! We are currently running into this exact issue using v8. Any updates on recommended ways to bring request context into errors reported inside async code blocks?