Adds better Ruby error messages for TypeErrors by including the field name

[ebenoist]

Currently the TypeErrors thrown by the generated classes do not include the field name in the error description, which can make debugging these issues a matter of bisecting messages until the error is repeatable. Including the name of the field in the TypeErrors thrown by these classes helps immensely with their debug-ability.

I've added extra tests to demonstrate this new behavior, but wanted to get some feedback before tackling the field names for the RepeatedField and Map type containers, neither of which currently hold their field name. If the above approach for primitives seems sane, I'd love some guidance on how to add the field names to those type containers so they can be passed to the type checkers for richer error messages.

• TODO: Add field name for repeated fields and maps (anything using the typed field containers)
• TODO: jRuby support. I am happy to add the jRuby support here as well once we everyone is onboard with this change.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
• If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
• In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

[ebenoist]

I signed it!

[googlebot]

CLAs look good, thanks!

[ebenoist]

Debugging type errors in Ruby is very difficult because of the poor error messages here. What can be done to move this PR along?

[anandolee]

ping @TeBoring

[xfxyjwf]

ping @TeBoring

[TeBoring]

There seems to be an error for this line.

[ebenoist]

Thats correct. As I noted in the writeup, I haven't implemented reporting the field names for repeated types. I wasn't sure how to pull the field name out of the repeated container. I wanted to make sure you all were okay with this approach before trying to figure out how to grab that information.

[ebenoist]

Alternatively I can punt on repeated fields for now if this is enough of an improvement on a first pass.

[TeBoring]

Repeated field is tricky. What if this repeated field is assigned to other message and the new field is not called foo?

[ebenoist]

I couldn't agree more :). Why don't I back out the repeated field stuff for now and we can focus on just the setters for primitives?

[TeBoring]

Without this change, when there is error, does the place of calling setter show in stack trace?

[ebenoist]

@TeBoring Currently and with this change, yes, a TypeError is thrown and the stack trace should emit from that location.

[TeBoring]

FMO, the current error message + stack track is already enough for debugging.
Besides, the native_slot_set was shared for map/repeated field. If you do detailed error for primitive field but do something else for map/repeated field, the logic will become unnecessarily complicate.
Sorry that for now I don't want this change.

[ebenoist]

@TeBoring Sorry to hear that. This is particularly a problem not for individual setters, but when using the constructor interface to set a number of fields. When using the constructor you have have no idea which field doesn't currently meet the type expectations. This means literally having to bisect larger structs with many fields to debug, which of the fields is throwing an error.

I'll give an example.

my_msg = MyMsg.new({
  field_one: "2",
  field_two: 1,
  field_three: {}
})

If all of those fields expect an integer, I'll get a stack track that emits from the construction of MyMsg. In the stack trace there is no information about which field is currently emitting a TypeError.

[ebenoist]

@TeBoring Here's a great practical example of the current issue:

Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Reverb::Search::Service
  #core_completions
    performs a core completions search (FAILED - 1)

Failures:

  1) Reverb::Search::Service#core_completions performs a core completions search
     Failure/Error:
       Reverb::Search::SuggestOption.new({
         text: option.text,
         id: option.id,
         score: option.score,
         output: option.doc[:output].to_s,
         core_payload: present_core_payload(option.doc)
       })

     TypeError:
       Invalid argument for string field.
     # ./lib/service/handlers/core_completions_handler.rb:43:in `initialize'
     # ./lib/service/handlers/core_completions_handler.rb:43:in `new'
     # ./lib/service/handlers/core_completions_handler.rb:43:in `block in present_core_options'
     # ./lib/service/handlers/core_completions_handler.rb:42:in `map'
     # ./lib/service/handlers/core_completions_handler.rb:42:in `present_core_options'
     # ./lib/service/handlers/core_completions_handler.rb:36:in `block in present_core_completions'
     # ./lib/service/handlers/core_completions_handler.rb:33:in `map'
     # ./lib/service/handlers/core_completions_handler.rb:33:in `present_core_completions'
     # ./lib/service/handlers/core_completions_handler.rb:26:in `handle'
     # ./lib/service/service.rb:20:in `core_completions'
     # ./spec/service/handlers/core_completions_handler_spec.rb:60:in `block (3 levels) in <top (required)>'

With the current error I have no idea which of the fields currently has an issue, as the error raises from line 43, where the constructor is called.