3

Why this code:

geoip_country_code_by_name('unknown'); 

generate ErrorException, when must return false ?

Wrikken
  • 69,272
  • 8
  • 97
  • 136
maxim.u
  • 331
  • 2
  • 9
  • 1
    Do you have GeoIP *installed*? http://www.php.net/manual/en/geoip.setup.php ErrorExceptions come with error messages indicating what the error was, by the way. – ceejayoz Sep 27 '13 at 15:57
  • yep, geoip is installed. Error message: "geoip_country_code_by_name(): Host unknown not found", but in documentation (http://php.net/manual/en/function.geoip-country-code-by-name.php) this function should return false for this situation. – maxim.u Sep 27 '13 at 16:05
  • Have you tried a real hostname? Maybe it knows that `unknown` isn't a legit one. – ceejayoz Sep 27 '13 at 16:16
  • Why is this question tagged with laravel? It seems unrelated, at least with the information provided in the question. – Matteus Hemström Sep 27 '13 at 16:22
  • Yes, if host or ip not in geoIP database, the same. – maxim.u Sep 27 '13 at 16:23
  • @Fnatte - because this problem i have only in laravel. – maxim.u Sep 27 '13 at 16:30
  • do you execute the code within a framework? It can be that the framework code turns warnings to exceptions – hek2mgl Sep 27 '13 at 16:39
  • 1
    @Fnatte: Laraval _is to the point_. it defines the error handler which throws the `ErrorException`. – Wrikken Sep 27 '13 at 16:44
  • 1
    PHP generates a notice (no ip found for hostname), Laravel makes an `ErrorException` of it. – Wrikken Sep 27 '13 at 16:45
  • @hek2mgl yes, without framework all good. – maxim.u Sep 27 '13 at 18:33

2 Answers2

4

This is a bug in GeoIP package and is not fixed in any release (<= 1.0.8). It's fixed in the trunk however (see this revision). You can solve this by compiling the source from the latest trunk.

Edited: thanks to Wrikken for pointing out how Laravel handles errors.

With GeoIP <= 1.0.8 geoip_country_code_by_name will trigger an error (E_NOTICE) whenever the name cannot be found. Laravel will always set error_reporting to -1 and handle all errors (even notices) and translate them into ErrorExceptions. Normally one can catch ErrorExceptions using a try-catch block, but in this case it is not possible because Laravel never throws the exception, it just translate it for displaying and logging purposes.

It is possible to ignore the error with the @-operator. It's a bit bad to do so since it will ignore all errors that the function might throw. In this case however, the only other error geoip_country_code_by_name can trigger is warning when the database can't be reached. Therefore you can safely ignore the error if you make sure the database is available: (Code not tested)

if (geoip_db_avail(GEOIP_COUNTRY_EDITION))
{
    @geoip_country_code_by_name('unknown');
}
else
{
    // Throw exception or handle the error
    throw new Exception(
       "Required database not available at " . 
       geoip_db_filename(GEOIP_COUNTRY_EDITION) 
    );

}

Edit: Laravel now throws the ErrorException so that one can catch it using a try-catch block. At the time of writing, this change is not yet in any released tag. But a catching errors will probably work with Laravel/Framework >= 4.0.8.

Community
  • 1
  • 1
Matteus Hemström
  • 3,796
  • 2
  • 26
  • 34
  • 1
    Sure about what? That the trunk has it fixed? I didn't try but [this](http://svn.php.net/viewvc/pecl/geoip/trunk/geoip.c?r1=318845&r2=318844&pathrev=318845) makes it pretty clear. – Matteus Hemström Sep 27 '13 at 17:01
  • If you edit in the part about Laravel creating Exceptions from Notices, it's complete, and I'll delete my answer :) (as soon as I return, as I'm about to go out...) – Wrikken Sep 27 '13 at 17:14
  • @Wrikken Added the Laravel part. Anything missing?:) – Matteus Hemström Sep 27 '13 at 19:22
  • @Fnatte:, well, `try... catch..`-ing _does_ work, otherwise, perfect :) – Wrikken Sep 28 '13 at 05:49
  • Erm, (1) Laravel _does_ throw the `ErrorException` (2) One _can_ catch it. Only _if nothing catches it_, the default exception handler is called which is just a generic 'something is wrong' message is `debug'` is set to `false in `app/config/app.php`, or a complete exception + trace is it is set to `true`. So what works for this problem are several options: (1) suppress the error with `@` (2) Use `error_reporting()` to (temporarily?) ignore `E_NOTICE` errors. (3) Recomple `mod_geoip` so it does not triggger notices. (4) Use a `try{ ...} catch {...}` block around the code. – Wrikken Sep 28 '13 at 09:24
  • Wow. Laravel 4 has been throwing ErrorExceptions since... yesterday [(this commit)](https://github.com/laravel/framework/commit/694d624d89ec3a60bc27a5e37fbc9d24b5464502). I didn't have that commit in my test environment. I will edit the answer. – Matteus Hemström Sep 28 '13 at 11:21
  • @Wrikken i tried to use `try{...} catch {...}`, but its not working. Laravel still handled error. So (4) not suitable – maxim.u Sep 28 '13 at 16:24
  • @maxim.u: _my_ Laravel does. What exact version are you running? – Wrikken Sep 28 '13 at 16:54
  • @Wrikken laravel-4.0.7, php-5.5.4. I mean `try{ geoip_country_code_by_name('unknown')} catch(\ErrorException $e){}catch(\Exception $e){}` does not catch the error. Default laravel handler does. – maxim.u Sep 28 '13 at 17:20
  • Hm, you're right, it does something different in HTTP context then in CLI context (which is evil), it does not _throw_ the exception (which is even more evil), but directly calls the defined catch function directly. The only way for it to work: alter `$this->handleException($e);` in `vendor/laravel/framework/src/Illuminate/Exception/Handler.php`, line 133, to `throw $e;` (what they should have done in the first place. – Wrikken Sep 28 '13 at 18:36
  • (FWIW: this is already altered in the repo's, but apparently not in a tag yet, so an update with composer doesn't work until they do) – Wrikken Sep 28 '13 at 18:53
  • Correction: an update with composer set to `4.0.*` does not work, an update to `dev-master` _does_. – Wrikken Sep 28 '13 at 18:59
1

In 1.1.0 module version bug fixed https://pecl.php.net/package-changelog.php?package=geoip&release=1.1.0

FallDi
  • 660
  • 7
  • 21