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

When using tower_error_tracker, ErrorTracker.set_context doesn't work #19

Closed
brainlid opened this issue Nov 21, 2024 · 7 comments
Closed

Comments

@brainlid
Copy link

Elixir: 1.17.2
OTP: 27

In my mix.exs file, when I have:

{:tower_email, "~> 0.5.2"},
{:tower_error_tracker, "~> 0.3.3"},

and all the config was followed, error are correctly reported through email and ErrorTracker. However, when I use ErrorTracker.set_context(%{user_id: 123}), there is zero error context coming through in ErrorTracker. It shows as {} in the ErrorTracker interface. This is when raised in a LiveView process.

When I change the config to:

{:tower_email, "~> 0.5.2"},
{:error_tracker, "~> 0.4.0"},

And I remove the TowerErrorTracker reporter config :tower, :reporters, [TowerEmail], then I get a full set of error context along with custom set_context values.

@grzuy
Copy link
Collaborator

grzuy commented Nov 21, 2024

Hey @brainlid,

Thanks for the report!

Indeed: #20 .

It seems this is caused mainly because of the combined effects of ErrorTracker storing the context in the process dictionary, and in tower we're spawning a new process before calling the reporter.

I'll try to figure out a way it can be reported anyway...

@grzuy
Copy link
Collaborator

grzuy commented Nov 21, 2024

Having tower support something like Tower.set_context so that it can be sent later to all reporters is part of the plan/roadmap FWIW.

@brainlid
Copy link
Author

Thanks @grzuy! I suspected it might be a cross-process issue. I don't know if this helps, but I've previously copied Logger's internal metadata state (stored in the process) when crossing process boundaries so things like the a Plug assigned request_id could flow deeper into spawned workers. Perhaps something like that could help here?

@grzuy
Copy link
Collaborator

grzuy commented Nov 21, 2024

Yes, we would need some copying of process dictionary values most probably here.

The same will be needed, as you say, for when reporting any Logger.metadata, which we'll probably will in the future.

@grzuy grzuy changed the title When using tower_error_tracker, ErrorTracker.set_context doesn't work When using tower_error_tracker, ErrorTracker.set_context doesn't work Nov 25, 2024
@grzuy
Copy link
Collaborator

grzuy commented Nov 25, 2024

Hi @brainlid,

Just released tower 0.7.3 with a workaround (mimiquate/tower#93) that I think will fix this if you're using bandit.

To fix it for web apps using plug_cowboy will probably need a different workaround/approach.

@grzuy
Copy link
Collaborator

grzuy commented Nov 25, 2024

Having tower support something like Tower.set_context so that it can be sent later to all reporters is part of the plan/roadmap FWIW.

For the record, opening issue in tower regarding the above: mimiquate/tower#94.

@brainlid
Copy link
Author

That fixed it for me! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants