i18n-tasks icon indicating copy to clipboard operation
i18n-tasks copied to clipboard

error handling

Open estani opened this issue 4 years ago • 2 comments

It would be nice, to know where problems arise.

We have a lot of i18n files and there was a problem returning just:

bundler: failed to load command: i18n-tasks (/Users/estani/.rbenv/versions/2.5.7/bin/i18n-tasks)
ArgumentError: usages must be a Data::Tree::Instance
  /Users/adam/.rbenv/versions/2.5.7/lib/ruby/gems/2.5.0/gems/i18n-tasks-0.9.31/lib/i18n/tasks/references.rb:11:in `process_references'

I saw where the problem in i18n-tasks was thrown and realized a simple fail was being thrown. It would be great to encapsulate those errors and add information regarding which file failed.

This is how I did it for this case, just in case it helps to depict what2 I mean, but it certainly should be done differently.

    def fail_w_info(error, message, data_refs)
      warn "Error in #{data_refs.map { |e| e.data[:path] }.uniq}"
      fail error, message
    end

estani avatar Apr 06 '20 10:04 estani

This an internal error, i.e. a bug in i18n-tasks. i18n-tasks should handle invalid data gracefully.

Do you have an example of a file that results in this error being thrown?

glebm avatar Apr 06 '20 20:04 glebm

puh took me a while to find the (possible) bug... the problem is that values returned from i18n.t are not only strings, they can be hashes too: https://guides.rubyonrails.org/i18n.html#bulk-and-namespace-lookup and symbols...

if you mix those two up, then the problem arise, e.g.:

en:
  this:
   will_not: :work 

And used as:

def get_bulk
   some_hash = i18n.t('this')
   #...
end

The code breaks, here my byebug session showing it:

[15, 24] in /Users/estani/.rbenv/versions/2.5.7/lib/ruby/gems/2.5.0/gems/i18n-tasks-0.9.31/lib/i18n/tasks/references.rb
   15:       refs = empty_forest
   16:       data_refs.key_to_node.each do |ref_key_part, ref_node|
   17:         usages.each do |usage_node|
   18:           next unless usage_node.key == ref_key_part
   19:           require 'byebug'; byebug
=> 20:           if ref_node.leaf?
   21:             process_leaf!(ref_node, usage_node, raw_refs, resolved_refs, refs)
   22:           else
   23:             process_non_leaf!(ref_node, usage_node, raw_refs, resolved_refs, refs)
   24:           end
(byebug) ref_node
this
  is: ⮕ broken {:path=>"config/locales/en.yml", :locale=>"en"}
(byebug) ref_node.leaf?
false
(byebug) usage_node
this:  {:occurrences=>[Occurrence(app/views/layouts/application.html.erb:14:9:332:this:)]}
(byebug) usage_node.children
nil

Next step, it tries to handle the non existing children, because ref_node says is not a leaf and it breaks.

What I couldn't find is why this works, if :broken is changed to a string. There is somewhere in code a branch handling it differently, that I couldn't find.

I've added a basic rails example with the problem here: https://github.com/estani/i18n-tasks-bug-example

The key question, that I couldn't find an answer to is: should be symbols allowed as yaml values of i18n files?

My colleague uses i18n files to store default i18n data to initialize objects, I'm not entirely sure is a good Idea, but does the trick. I couldn't find any info regarding this usage, or its discourage.

estani avatar Apr 07 '20 08:04 estani