Improve error on module case mismatch

[voltone]

On a case-insensitive filesystem, a module case mismatch typo results in error messages that can be confusing:

iex(1)> Genserver.start_link(MyApp.Worker, [], name: MyApp.Worker)

09:57:03.286 [error] Loading of /lib/elixir/ebin/Elixir.Genserver.beam failed: :badfile

09:57:03.287 [error] beam/beam_load.c(1376): Error loading module 'Elixir.Genserver':
  module name in object code is Elixir.GenServer

** (UndefinedFunctionError) function Genserver.start_link/3 is undefined (module Genserver is not available)
    Genserver.start_link(MyApp.Worker, [], [name: MyApp.Worker])

The :code_server log messages do show the root cause, but it's easy to miss, and in a way they obscure the problem by suggesting the problem is in the .beam file.

This PR provides a more helpful message (the :code_server log messages still appear):

iex(1)> Genserver.start_link(MyApp.Worker, [], name: MyApp.Worker)
** (UndefinedFunctionError) function Genserver.start_link/3 is undefined (module Genserver is not available). Did you mean:

      * GenServer.start_link/3

    Genserver.start_link(MyApp.Worker, [], [name: MyApp.Worker])

Question: would it be possible/desirable to suppress the :code_server log messages?

The xref Mix task has been updated to provide the same message as well.

/cc @benwilson512

[voltone]

Hmm, tests succeed on my Mac. I'm guessing Travis runs on a case-sensitive filesystem, so this is treated as a simple missing module error. The problem with a more generic solution is that it would require traversing the code path to find potential alternatives...

[josevalim]

@fishcakez do you think :code.get_object_code is a safe operation to perform in such cases?

[josevalim]

This will probably be cleaner if you use the with construct. Also keep in mind we can pass the second argument from :code.get_object_code to :beam_lib. Here is one suggestion:

suffix =
  with {_, binary, _} <- :code.get_object_code(module),
       {:ok, {alt_module, _}} <- :beam_lib.chunks(binary, []) do
    exports = exports_for(alt_module)
    if ... do ... else ... end
  else
    _ -> ""
  end

[fishcakez]

I am keen on this change because :code.get_object_code will search the code path every time. This takes away from embedded mode with releases which will not search the code path for a module when it does not exist. This is useful to keep applications responsive if something has gone wrong. Note that the code file system might not be local.

[voltone]

Yeah, I wanted to avoid traversing the code path to look for the module that :code_server attempted to load, but of course :code.get_object_code does that as well. Bummer. So with this change the code path is traversed twice, once when the exception is first thrown, and again when the message is built.

Would there be any way to determine the name of the BEAM file :code_server just tried to load without traversing? The information is right there, in the log messages, I just want to highlight the case mismatch and avoid the suggestion that something is wrong with the BEAM file...

[fishcakez]

I am not sure what we can do :(. If interactive mode, e.g. mix, then the module we want may not actually be loaded because it has not been called correctly yet. You could investigate if we can translate the log messages in logger.

[voltone]

OK, so I'll look at a possible log translator.

If the concern is the runtime impact, perhaps I can move the code out of the exception and into the xref mix task only. In that case I could just scan all module names in the load path and sort by jaro_distance, the way we do for function mismatches. Then it will also catch other typos, and case mismatches on a case-sensitive FS.

[fishcakez]

Being able to detect module typos with xref would be good. Is there a reason it wasn't implemented?

[voltone]

Regarding log translation, I can translate this:

10:48:31.286 [error] Loading of /usr/local/Cellar/elixir/1.4.0/bin/../lib/elixir/ebin/Elixir.Genserver.beam failed: :badfile

...into this:

10:48:31.286 [error] Module name case mismatch: called as 'Genserver', module defined as 'GenServer'

It requires a call to :beam_lib.chunks, but with the full path to the beam file, so it does not involve traversing the load path. The exception will be the standard 'module not available' error, but hopefully the translated log message will help, instead of sending the user down a rabbit hole.

[josevalim]

@voltone I like that! I would just say:

10:48:31.286 [error] Module name case mismatch: called as 'Genserver', module defined as 'GenServer' in /usr/local/Cellar/elixir/1.4.0/bin/../lib/elixir/ebin/Elixir.Genserver.beam

@fishcakez, what do you think?

[voltone]

@josevalim The reason I dropped the path is because it includes the 'mutilated' filename, and besides, it's noise that detracts from the root cause the message tries to highlight. No?

[josevalim]

@voltone it is mostly noise but it may be useful in some situations. For example, imagine someone accidentally have a .beam file in the root of the current project named Foobar while in the project they use FooBar. Then Foobar would be picked up, they would see the warning, but never realized it is because of a beam file at root.

If you want to put less attention to it, you can put the "in path/to/beam" in parentheses.

[fishcakez]

It should be loaded not called because loading is causing the error and it does not require a call - the load is a side effect of the call to unloaded module though.