laravel-geoip icon indicating copy to clipboard operation
laravel-geoip copied to clipboard

Possible bug

Open zgelici opened this issue 5 years ago • 12 comments

Hi,

I get an issue with a part of code. i did some checks.

vendor\torann\geoip\src\Services\IPApi.php

Line 74 gives notice:

Notice: Trying to get property 'status' of non-object

current code:

if ($json->status !== 'success') {
            throw new Exception('Request failed (' . $json->message . ')');
        }

i think this will solve it:

if ($json && $json->status !== 'success') {
            throw new Exception('Request failed (' . $json->message . ')');
        }

I get the issue when i call this:

geoip()->getLocation($ip)['country']

i dont get the notice everytime.

zgelici avatar Jul 07 '19 19:07 zgelici

I'm also getting that sometimes

jadjoubran avatar Jul 19 '19 20:07 jadjoubran

We experience this error also frequently. @zgelici can you make a pull request containing your fix?

lamalamaMark avatar Aug 04 '19 20:08 lamalamaMark

We experience this error also frequently. @zgelici can you make a pull request containing your fix?

I did it

zgelici avatar Aug 04 '19 21:08 zgelici

I think the solution needs to be worked out further because the code after this condition also relies on the content of the $json variable. Did you look at that part?

lamalamaMark avatar Aug 05 '19 07:08 lamalamaMark

@lamalamaMark

You are right, the other part need to be also inside $json check, but than its possible that the api or something gives empty results. What effect it will have.

zgelici avatar Aug 05 '19 07:08 zgelici

@lamalamaMark can you check now the new pull request

its possible that requests or something fails, so i did multi tries. if not it give empty. i didnt test it and its late here. tomorrow evening i will check my code again. but i already did push.

zgelici avatar Aug 05 '19 21:08 zgelici

I have same problem, sentry always alarm it.

paulocastellano avatar Aug 12 '19 13:08 paulocastellano

Same situation with sentry, but this is happening after update from Laravel 5.8 to 6.0

JexPY avatar Oct 21 '19 19:10 JexPY

@lamalamaMark the fix of @zgelici its very useful why dont accept that PR?

huesoamz avatar Oct 30 '19 03:10 huesoamz

Hi guys!

I have te same problem.

Did you find the solution?

gutitrombotto avatar Apr 01 '20 14:04 gutitrombotto

I just had the same problem today. And I think it is related to the rate limiters defined here.

Usage limits This endpoint is limited to 45 requests per minute from an IP address.

If you go over the limit your requests will be throttled (HTTP 429) until your rate limit window is reset. If you constantly go over the limit your IP address will be banned for 1 hour.

The returned HTTP header X-Rl contains the number of requests remaining in the current rate limit window. X-Ttl contains the seconds until the limit is reset. Your implementation should always check the value of the X-Rl header, and if its is 0 you must not send any more requests for the duration of X-Ttl in seconds.

We do not allow commercial use of this endpoint. Please see our pro service for SSL access, unlimited queries and commercial support.

While the PR proposed solves the problem with the non-object error it does not solve the problem of going above the limit on a non-paid environment.

wlepinski avatar May 23 '20 16:05 wlepinski

same error

hosamalzagh avatar Feb 28 '23 11:02 hosamalzagh