NoMethodError when using valid? or fake? method with FB useragent about legitbot HOT 11 CLOSED

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.

from legitbot.

Hey @alaz, why not use dig here?

from legitbot.

@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.

from legitbot.

@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.

from legitbot.

@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?

from legitbot.

@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:

from legitbot.

@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

from legitbot.

@alaz Was a PR created for this?

from legitbot.

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

from legitbot.

@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.

from legitbot.

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: #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.

from legitbot.