NoMethodError when using valid? or fake? method with FB useragent

[Hanaffi]

[flvrone]

Hey @alaz, why not use dig here? https://github.com/alaz/legitbot/blob/624011f4c6599c13cd0282047bbb271c63036a27/lib/legitbot/facebook.rb#L16 That NoMethodError is not the best way to raise an exception here, I think. :sweat_smile:

[alaz]

@Hanaffi , it seems to be something with your network. Try debugging with whois. You can use the command from the page on Facebook Crawler as a starting point.

[alaz]

@FunkyloverOne , do you mean dig as an executable? The first of all, it may not be present in the system. The second, it would be worse security-wise.

[flvrone]

@alaz , no, I mean a method, here: https://ruby-doc.org/3.2.0/Hash.html#method-i-dig So the code would look like this:

results.dig(AS,family, AS)

Even if there is some network issue, if some error must be raised here - it better not be NoMethodError, but rather something meaningful, WDYT?

[alaz]

@FunkyloverOne thanks for clarifying. Feel free to put up a PR. Please do remember that the minimal supported Ruby version is 2.7 at the moment:

https://github.com/alaz/legitbot/blob/624011f4c6599c13cd0282047bbb271c63036a27/.github/workflows/build.yml#L14

[flvrone]

@alaz no problem at all, here it is in 2.7.0: https://ruby-doc.org/core-2.7.0/Hash.html#method-i-dig

[joancodes]

@alaz Was a PR created for this?

[flvrone]

I was thinking about how to best approach this situation, I think that we would want to introduce a custom error, something like Legitbot::IPRangeNotFoundError, inherited from Legitbot::Error < StandardError, so that for consumers it would be possible to rescue from those errors and react to them how they see fit.

But also, when do we want to raise it? Like this?

%i[ipv4 ipv6].map do |family|
  results.dig(AS, family, AS) || raise Legitbot::IPRangeNotFoundError
end.flatten

Or like this:

ranges = %i[ipv4 ipv6].map do |family|
  results.dig(AS, family, AS) || raise Legitbot::IPRangeNotFoundError
end.flatten.compact
raise Legitbot::IPRangeNotFoundError if ranges.empty?

ranges

[alaz]

@FunkyloverOne what situation? No one has helped debug this problem further

If networks with malfunctioning whois are the issue here, I suggest thinking backwards from the client code point of view. They need to know that the initialisation phase has failed and be told why.

[flvrone]

The situation is the following - if for whatever reason (network issue or Facebook changing something on their side) the results hash does not have the required keys - this code fails with NoMethodError. What's here to debug? I propose to raise the appropriate error instead of NoMethodError, so that it can be rescued and handled accordingly.

As far as I'm aware - you can reproduce this error by simply using this gem version prior to 1.9.4. This error was kind of fixed here: https://github.com/alaz/legitbot/pull/113 But, that PR fixes configuration for Facebook, but does not address the NoMethodError which would be cumbersome to rescue if any kind of similar hiccup happens again.

[alaz]

Ah, you are saying this issue is a duplicate of #112 . Then it should be closed, because they are fixed via #113 and can not be reproduced in 1.9.4. Thank you for the information.

I am happy to discuss improvements to the code in a PR if you are willing to contribute. Thank you.