-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Introduce __STACKTRACE__ #7097
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
Comments
__STACKTRACE__
__STACKTRACE__
So I looked a bit into actually implementing this, and one problem with __STACKTRACE__ # 0
try do
__STACKTRACE__ # 1
throw :foo
catch
:foo ->
try do
__STACKTRACE__ # 2
throw :bar
catch
:bar ->
__STACKTRACE__ # 3
end
__STACKTRACE__ # 4
end It's clear that While it's true that nested |
@michalmuskala we could make #2 fail, forcing the user to explicitly set a variable before. |
Yeah, this also comes down to approach to compiling the thing. I initially tried to make it work similar to |
@michalmuskala we can generate fresh variables every time. :) |
@michallepicki we could even expand our catch/rescue to have three elements, the same as Erlang, but that would be completely internal. This way |
You pinged the wrong Michał. 😆 But that would mean we'd need to expand |
that's fine, I got used to it already, happens every few months ;) |
Oops, sorry!
I don't think we should do that because that is a behaviour that is fairly specific to Erlang. For example, ElixirScript probably wants to keep the high level semantics so Maybe rescue should have two arguments too... |
I am reopening because we need to do more work to prepare for Erlang 21. |
How should we handle situations where For example, we have some code where we handle error tuples by collecting information, which includes the stacktrace, and send the data to a monitoring service. def set_error(transaction, name, message, reason) do
backtrace = ([reason] ++ System.stacktrace()) |> Enum.map(&inspect/1)
Appsignal.Transaction.set_error(transaction, name, message, backtrace)
end |
@dideler You should not rely on the ability to get the stacktrace outside of a |
A workaround is just to throw an exception and then immediately catch it of course, which can be wrapped up in a function. |
Moving to https://elixirforum.com/t/using-system-stacktrace-within-exq-middleware-behaviour-implementation/15813, sorry for the noise. |
@dideler no problem at all. Just one last note: |
What's the correct way of printing the def unreliable_method(item) do
# Do something with `item`
:ok
rescue
_err ->
Logger.error("Failed for item: #{inspect(item)}")
Logger.error(inspect(__STACKTRACE__))
{:error, :processing_failed}
end |
The canonical way would be to use |
@michalmuskala Thank you. I looked at other methods of the Logger.error(Exception.format(:error, err, __STACKTRACE__)) |
Using `System.get_stacktrace()` may cause the emulator to hold on to the stack trace for a very long time More details in: elixir-lang/elixir#7097
Using `System.get_stacktrace()` may cause the emulator to hold on to the stack trace for a very long time More details in: elixir-lang/elixir#7097
In elixir 1.7 (elixir-lang/elixir#7097), `System.stacktrace` is deprecated and suggests replacing with `__STACKTRACE__` inside a rescue/catch. EN-5855
In elixir 1.7 (elixir-lang/elixir#7097), `System.stacktrace` is deprecated and suggests replacing with `__STACKTRACE__` inside a rescue/catch. EN-5855
TL;DR Replace the use of
System.get_stacktrace()
with a pseudo-variable__STACKTRACE__
similar to__MODULE__
or__CALLER__
available only insidecatch
andrescue
clauses of thetry
expression.Reasons
Erlang recently set out to overhaul how stack traces are handled. The
:erlang.get_stacktrace/0
call (and thus it's Elixir's equivalentSystem.get_stacktrace/0
) are problematic - they may cause the emulator to hold on to the stack trace for a very long time. This can be harmful, since the stack trace can be very big - in case ofFunctionClauseError
it includes all the arguments. The emulator has to hold on the stack trace for the last exception in general until the next exception is raised - which can be a long time.To rectify this potential performance issue, OTP 20 started warning on the use of
:erlang.get_stacktrace()
outside of thecatch
clause of atry
expression (in Elixir this would be outside ofcatch
andrescue
). The plan was to make the call return an empty list in some future Erlang version when called outside ofcatch
.OTP 21 will introduce an even more restrictive mechanism, to understand it we need to understand the syntax for
catch
in Erlang:Where
Kind
can be one of:exit
,:error
and:throw
. The proposed (and merged) enhancement replaces this with:Where the
Stack
variable would contain the stack-trace for the just-rescued exception. It is forbidden to pattern match on this variable or use an already-bound variable (which would be equivalent to pinning it in Elixir). It's now planned that some future Erlang release will always return empty lists from:erlang.get_stacktrace()
and the only way to acquire the stacktrace will be with this syntax. This means we need a compatible Elixir solution.Proposal
To solve this, we propose introduction of a new pseudo-variable
__STACKTRACE__
that would be available insidecatch
andrescue
and would return the stack trace of the exception being currently handled.This means that this code:
Would become:
The
__STACKTRACE__
pseudo-variable would be only available insidecatch
andresuce
parts oftry
- it should fail to compile if present in any other place, similar to how__CALLER__
is not available outside of macros.Having this as this kind of pseudo-variable is also compatible with how this works in Erlang - since neither pattern matching nor pinning of the stack variable is allowed, we don't loose any features.
Implementation
Since OTP 21 is not stable yet, and we also want to support multiple Erlang versions, a conditional compilation of this feature is required.
On VMs without new syntax
When compiling on systems without support for the new syntax, the
__STACKTRACE__
pseudo-variable should compile into a call to:erlang.get_stacktrace/0
.Additionally, the call to
:erlang.get_stacktrace/0
should happen as the very first thing in the block, assigning the result to some internal variable, which could be later substituted for the__STACKTRACE__
expression. This provides a very important advantage compared to the currentSystem.get_stacktrace/0
call by protecting against the "wrong stack trace" bug, which is very easy to introduce accidentally:If the
this_may_throw()
operation does indeed throw, theSystem.get_stacktrace/0
call would return the value for that throw, instead of the exception we've just caught, since it always returns the stack trace of the last exception. With the use of the__STACKTRACE__
pseudo-variable, this would compile into something conceptually like:Which does not suffer from the issue. This means not only is the proposed feature giving us compatibility with Erlang, it is also a significant improvement in terms of preventing strange and hard to find bugs.
On VMs with new syntax
When compiling on a VM that supports the new syntax, the
__STACKTRACE__
call should just use it, the previous example would compile into the following equivalent Erlang code:Alternatives
An alternative would be to also support something like:
There are couple of problems with that approach:
rescue
orcatch
with just one pattern (which catches only throws).Compatibility
Because of the conditional-compilation, the feature becomes compatible with all the Erlang releases. There is the constraint that code compiled on VM that supports the feature wouldn't work on older VMs, but this is also true today - code compiled on OTP 20 wouldn't work on OTP 18, even though Elixir in itself supports running on both VMs.
Roadmap
The
System.get_stacktrace()
call should be soft-deprecated in the next release that includes the__STACKTRACE__
feature. Additionally a loud deprecation should happen after 2 releases or as soon as Elixir declares compatibility for an Erlang VM that would always return an empty list for:erlang.get_stacktrace()
. This might break the general deprecation policy approach of minimum 2 releases, but this is somewhat out of our hands.The text was updated successfully, but these errors were encountered: