Honeybadger.notify/2 grabs the stacktrace from inside the task process

[silasjmatson]

The Honeybadger.notify/2 macro calls macro_notify/3, which expects the 3rd argument to be a list containing the stacktrace.

However, the notify/2 macro sends an empty List as the 3rd argument, which means that this Honeybadger.do_notify/3 function, which pattern matches on the empty list, is the one that is called.

The do_notify/3 function contains a call to get the current stacktrace via Process.info(self(), :current_stacktrace), which always returns the same stacktrace because it's running within a Task.start/1 process. The stacktrace in the application in which I discovered this issue is always the following whenever Honeybadger.notify/2 is called:

lib/process.ex:547:in `info`
lib/honeybadger.ex:175:in `do_notify`
lib/task/supervised.ex:85:in `do_apply`
proc_lib.erl:247:in `init_p_do_apply`

A possible solution could be to grab the stacktrace before the task is spawned in macro_notify/3 (I'm sure my quick example here probably won't work, as it doesn't take into account the macro context)

defp macro_notify(exception, metadata, []) do
  {:current_stacktrace, stacktrace} = Process.info(self(), :current_stacktrace)
  macro_notify(exception, metadata, stacktrace)
end

defp macro_notify(exception, metadata, stacktrace) do
  quote do
    Task.start fn ->
      Honeybadger.do_notify(unquote(exception), unquote(metadata), unquote(stacktrace))
    end
  end
end

[silasjmatson]

After a bit more reading of the code, I think Honeybadger.notify/1 may also be affected by this issue, which means that the only way to get useful stacktraces in manually reported errors is to use Honeybadger.notify/3 and pass in the stacktrace manually.

[joshuap]

@silasjmatson I'm sorry for dropping the ball on this; I'm now working on building a better maintenance plan for our Elixir client (and there's new stuff I'd like to add, too!).

Anyway, I'm looking into the potential fix in #93 (and getting those PRs reviewed/merged) later this week, and then I'll revisit this issue and see if there's anything left to address.

[joshuap]

Quick update, since this is taking a bit longer than I hoped. I think we're getting close on #93. We're in the process of fixing a few issues I discovered. Once those are resolved I'll probably merge the PR and either let people point their Mixfile at master to test it or release a beta. I just want to make sure this doesn't introduce any new bugs/regressions since it's a big refactor. Excited to ship this!

[joshuap]

@silasjmatson 0.7.0-beta has been released. Can you test it out locally to see if this issue still exists there? To make it report in dev mode you can override exclude_envs in your config: exclude_envs: [].

[silasjmatson]

@joshuap Awesome! I'll give it a whirl later today and report back.

[silasjmatson]

@joshuap Testing with the beta, everything is golden so far! 👍

[silasjmatson]

@joshuap Closing as this is resolved on master and in the 0.7.0-beta.

[joshuap]

@silasjmatson great, thanks!