Better error messages in Standard Library functions

[Landon Schropp]

Hello!

I'm new to Elixir development, so I'm approaching the language with fresh eyes. One of the issues I've run into is difficulty understand the error messages from the standard library. For example, today I ran this code:

string
|> String.replace(string, ~r/[^\[\](){}]/, "")
|> IO.inspect(label: "\n")
|> String.codepoints
|> check_brackets([])

When I run this code through one of my test cases, I get the following error:

  1) test math expression (BracketPushTest)
     test/bracket_push_test.exs:52
     ** (FunctionClauseError) no function clause matching in Keyword.get/3
     The following arguments were given to Keyword.get/3:


         # 1                                                                                                                                                           ""


         # 2
         :insert_replaced


         # 3
         nil
                                                                                                                                                                   Attempted function clauses (showing 1 out of 1):


         def get(keywords, key, default) when is_list(keywords) and is_atom(key)


     code: assert BracketPush.check_brackets("(((185 + 223.85) * 15) - 543)/2") == true
     stacktrace:
       (elixir) lib/keyword.ex:195: Keyword.get/3
       (elixir) lib/string.ex:1372: String.replace/4
       (bracket_push) lib/bracket_push.ex:15: BracketPush.check_brackets/1
       test/bracket_push_test.exs:53: (test)

     test/bracket_push_test.exs:52
     ** (FunctionClauseError) no function clause matching in Keyword.get/3
     The following arguments were given to Keyword.get/3:


         # 1                                                                                                                                                           ""


         # 2
         :insert_replaced


         # 3
         nil
                                                                                                                                                                   Attempted function clauses (showing 1 out of 1):


         def get(keywords, key, default) when is_list(keywords) and is_atom(key)


     code: assert BracketPush.check_brackets("(((185 + 223.85) * 15) - 543)/2") == true
     stacktrace:
       (elixir) lib/keyword.ex:195: Keyword.get/3
       (elixir) lib/string.ex:1372: String.replace/4
       (bracket_push) lib/bracket_push.ex:15: BracketPush.check_brackets/1

As a new language user, it's really hard to understand what I did wrong when the error is thrown from inside the inner implementation of String.replace/4. In order to debug this, I'd either have to look at the inner implementation of String.replace/4 or attempt to fiddle with the arguments. My preference would be for standard library functions to guard their own interfaces, and throw specific and actionable messages back to the developer.

Thanks for taking the time to read!

[Landon Schropp]

The problem in this case was I didn't remove the first argument when I created the pipe, and so the arguments were the wrong type. But I'd still like to see a better error message for this.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/ec0892c5-04b8-4fe7-88b5-e12312483fa6%40googlegroups.com.

[José Valim]

A better error message would be attained by simply adding guards to the replace function. A PR is definitely welcome.

--

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

[Dmitry Belyaev]

The error message is clear:
BracketPush.check_brackets makes a call to https://hexdocs.pm/elixir/String.html#replace/4 and passes empty string "" as the last argument instead of a list.

In dynamically typed languages we expect the caller to pass the correct data for the function to behave as documented. Imagine every function typechecked every argument to the deepest primitive types - the code would become an incomprehensible mess and performance would be terrible wth every function type checking the data in runtime which was already typechecked one level higher.

--
Kind regards,
Dmitry Belyaev

[Landon Schropp]

@Jose Thanks for the quick reply! I agree that this would provide a better experience for Elixir. I'd be happy to contribute, but I'm very new to the language. I may be able to do it with some hand-holding if you're up for it.

@Dmitry I disagree—I don't think this error message is clear. String.replace/4 is calling Keyword.get/3 somewhere under the hood, and that's where the error message is being thrown. As someone who's new to the language, it's hard for me to tell what I did wrong when calling the method.

For context with another language, if I call "1234".gsub(:nope, "banana") in Ruby, I get this error:

Traceback (most recent call last):
        1: from test.rb:1:in `<main>'
test.rb:1:in `gsub': wrong argument type Symbol (expected Regexp) (TypeError)

In Python, "1234".replace(1234, "banana", 1) produces this:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: replace() argument 1 must be str, not int

Traceback (most recent call last):

  File "<stdin>", line 1, in <module>

TypeError: replace() argument 1 must be str, not int

[José Valim]

> @Jose Thanks for the quick reply! I agree that this would provide a better experience for Elixir. I'd be happy to contribute, but I'm very new to the language. I may be able to do it with some hand-holding if you're up for it.

Sure, the first step is to get Elixir test suite passing locally. You can follow the steps here: https://github.com/elixir-lang/elixir/#compiling-from-source

Next, you should find the source for String.replace/3 and change things like this:

def replace(subject, pattern, replacement, options)

To this:

def replace(subject, pattern, replacement, options) when is_list(options)

Or similar. This is not a step by step guide, so feel free to ask more questions.

> For context with another language, if I call "1234".gsub(:nope, "banana") in Ruby, I get this error: [...] In Python, "1234".replace(1234, "banana", 1) produces this

To be fair, those are not good examples because those methods are implemented in C in those languages, and languages typically do type checking when crossing the C boundary (otherwise you can end-up with segmentation faults or similar).  String.replace/3 is implemented in pure Elixir which is why the error can happen deep inside the stacktrace. A similar example in Ruby would likely raise a NoMethodError on one of the objects given as parameters or lead to something like "TypeError: no implicit conversion of Symbol into Integer" for something like "string"[:global].

I am not saying this to excuse the behaviour but rather to say that a similar behaviour to our current one will also be frequently found in other dynamic languages whenever you go past the built-in functions.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

--

You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/52657dee-f2e3-44de-a430-0b8d72eeccad%40googlegroups.com.

[Landon Schropp]

Thanks for the reply, Jose! I'd be happy to dive in, but it's probably going to be Sunday before I can take a look at it.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4LVm9tq4%2BQ1G3Gk0zTaaNiCySBy3XYYmhJHkj28DpDxXQ%40mail.gmail.com.

[Landon Schropp]

Hi Jose,

Sorry for the late reply on this! I finally cracked open the Elixir source code today and took a look at the String.replace/3 implementation. It looks like this may have already been taken care of.

@spec replace(t, pattern | Regex.t(), t | (t -> t | iodata), keyword) :: t

def replace(subject, pattern, replacement, options \\ [])

    when is_binary(subject) and

           (is_binary(replacement) or is_function(replacement, 1)) and

           is_list(options) do

  replace_guarded(subject, pattern, replacement, options)

end

If so, thank you very much! If not, I'm happy to tackle it. Am I missing something here?

Thanks again!

Landon