Do not use Access protocol on values that may be structs when normalizing erlang errors

[lukaszsamson]

I noticed this this error in ElixirLS. I don't know what code triggered it but the ErlangError.normalize implementation doesn't seem correct in assuming that Access protocol can be used on unknown value coming from stack trace.

** (UndefinedFunctionError) function Foo.fetch_2 is undefined (Foo does not implement the Access behaviour

You can use the "struct.field" syntax to access struct fields. You can also use Access.key/1 to access struct fields dynamically inside <REDACTED: user-file-path>)
    (foo_tools 0.1.0) Foo.fetch(%Foo{direct: MapSet.new([]), transient: %{}, all: MapSet.new([])}, :transient)
    (elixir 1.17.3) <REDACTED: user-file-path>:322: Access.get/3
    (elixir 1.17.3) <REDACTED: user-file-path>:2288: ErlangError.normalize/2
    (elixir 1.17.3) <REDACTED: user-file-path>:192: Exception.blame/3

While working on that change I started wondering what is the contract on Exception.blame. Exception.message callback does use try rescue pattern so the call should be safe. blame invokes client module code with no guarantees so a faulty implementation might throw/raise. https://github.com/elixir-lang/elixir/blob/main/lib/elixir/lib/exception.ex#L214
Shouldn't blame catch the error and fall back to the provided argument values if the callback crashes? Without that guarantees blame is at least cumbersome to use as the correct way would be

try do
  some_code()
catch kind, payload ->
  {payload, stacktrace} = try do
    Exception.blame(kind, payload, __STACKTRACE__)
  catch
     _, _ ->
       {payload, __STACKTRACE__}
  end

  Exception.format(kind, payload, stacktrace)
end

[josevalim]

💚 💙 💜 💛 ❤️

[lukaszsamson]

Shouldn't blame catch the error and fall back to the provided argument values if the callback crashes?

@josevalim Any comment on that?

[josevalim]

The issue is that then bugs on blame will remain uncaught. You shouldn’t need to rescue it in the long term.

[lukaszsamson]

The issue is that then bugs on blame will remain uncaught.

True @josevalim. On the other hand if the error is not handled then a faulty Exception.blame callback implementation in client code/third party lib will bring my error handler code down and shadow the original error and stacktrace. I'm generally more interested in the original error and this behaviour makes debugging and fixing the original issue harder. If we keep the current behaviour I think it's worth at least documenting that Exception.blame may raise on error in callback implementation while Exception.message will recover from errors in the callback implementation. Especially since the two differ