diff --git a/config/location.php b/config/location.php index 50b0a01..b26bcd1 100644 --- a/config/location.php +++ b/config/location.php @@ -68,6 +68,19 @@ 'connect_timeout' => 3, ], + /* + |-------------------------------------------------------------------------- + | Total Timeout + |-------------------------------------------------------------------------- + | + | The seconds a lookup may take across the driver above and all of its + | fallbacks. Set to null to instead give each driver its own timeout, + | which makes a failing lookup take them all added up. + | + */ + + 'total_timeout' => null, + /* |-------------------------------------------------------------------------- | Localhost Testing diff --git a/readme.md b/readme.md index 8b597aa..1ef036c 100644 --- a/readme.md +++ b/readme.md @@ -186,6 +186,33 @@ information from the visitor. If an exception occurs trying to grab a driver (such as a 400/500 error if the providers API changes), it will automatically use the next driver in line. +### Timeouts + +The `http` config option times out a single driver's request. Each driver gets +its own, so with the default configuration an unreachable provider costs you +that timeout five times over before the lookup gives up. + +Set `total_timeout` to give the whole lookup one shared budget instead: + +```php +'total_timeout' => 5, +``` + +### Observing failures + +Drivers fail quietly so the next fallback can take over. To see why one failed, +listen for the `LookupFailed` event: + +```php +use Stevebauman\Location\Events\LookupFailed; + +Event::listen(function (LookupFailed $event) { + Log::warning("Location lookup failed via [$event->driver].", [ + 'ip' => $event->ip, 'exception' => $event->exception, + ]); +}); +``` + ### Creating your own drivers To create your own driver, simply create a class in your application, and extend the abstract Driver: diff --git a/src/Deadline.php b/src/Deadline.php new file mode 100644 index 0000000..a171886 --- /dev/null +++ b/src/Deadline.php @@ -0,0 +1,39 @@ +http()->acceptJson()->get( $this->url($request->getIp()) ); - throw_if($response->failed()); + return new Fluent($response->throw()->json()); + } catch (Throwable $e) { + // Failures are expected, they're what calls the next fallback + // driver. We won't report them to keep the log quiet, but + // we will announce them so they can still be observed. + event(new LookupFailed(static::class, $request->getIp(), $e)); - return new Fluent($response->json()); - }, false, false); + return false; + } } /** @@ -51,11 +63,30 @@ protected function http(): PendingRequest { $callback = static::$httpResolver ?: fn ($http) => $http; - return value($callback, Http::withOptions( - config('location.http', [ - 'timeout' => 3, - 'connect_timeout' => 3, - ]) - )); + return value($callback, Http::withOptions($this->options())); + } + + /** + * Get the options to use for the HTTP request. + */ + protected function options(): array + { + $options = config('location.http', [ + 'timeout' => 3, + 'connect_timeout' => 3, + ]); + + if (is_null($remaining = Deadline::remaining())) { + return $options; + } + + // No request may outlive what's left of the lookup's budget. A + // missing or zero timeout is unlimited as far as Guzzle is + // concerned, so those get the whole remaining budget. + foreach (['timeout', 'connect_timeout'] as $option) { + $options[$option] = min(($options[$option] ?? 0) ?: INF, $remaining); + } + + return $options; } } diff --git a/src/Events/LookupFailed.php b/src/Events/LookupFailed.php new file mode 100644 index 0000000..d3ddeb2 --- /dev/null +++ b/src/Events/LookupFailed.php @@ -0,0 +1,14 @@ +driver->get($this->request()->setIp($ip)); + return Deadline::for( + config('location.total_timeout'), + fn () => $this->driver->get($this->request()->setIp($ip)) + ); } /** diff --git a/tests/Drivers/HttpDriverTest.php b/tests/Drivers/HttpDriverTest.php new file mode 100644 index 0000000..fdc24bd --- /dev/null +++ b/tests/Drivers/HttpDriverTest.php @@ -0,0 +1,47 @@ + IpApi::class, 'location.fallbacks' => []]); +}); + +it('announces a failure to connect', function () { + Http::fake(fn () => throw new ConnectionException('cURL error 28: Operation timed out')); + + expect(Location::get('8.8.8.8'))->toBeFalse(); + + Event::assertDispatched(LookupFailed::class, fn (LookupFailed $e) => $e->driver === IpApi::class + && $e->ip === '8.8.8.8' + && $e->exception instanceof ConnectionException + ); +}); + +it('announces an unsuccessful response', function () { + Http::fake(fn () => Http::response('Service Unavailable', 503)); + + expect(Location::get('8.8.8.8'))->toBeFalse(); + + Event::assertDispatched(LookupFailed::class, fn (LookupFailed $e) => $e->exception instanceof RequestException + && $e->exception->response->status() === 503 + ); +}); + +it('announces nothing on success', function () { + Http::fake(fn () => Http::response(['countryCode' => 'US'])); + + expect(Location::get('8.8.8.8'))->toBeInstanceOf(Position::class); + + Event::assertNotDispatched(LookupFailed::class); +}); diff --git a/tests/TotalTimeoutTest.php b/tests/TotalTimeoutTest.php new file mode 100644 index 0000000..3c2b583 --- /dev/null +++ b/tests/TotalTimeoutTest.php @@ -0,0 +1,81 @@ +exchangeArray([]); + + HttpDriver::resolveHttpBy(function ($http) use ($captured) { + $captured[] = $http->getOptions(); + + return $http; + }); + + config([ + 'location.driver' => IpApi::class, + 'location.fallbacks' => [IpInfo::class, GeoPlugin::class], + 'location.http' => ['timeout' => 3, 'connect_timeout' => 3], + 'location.total_timeout' => null, + ]); + + Http::fake(function () { + usleep(200_000); + + return Http::response('', 500); + }); +}); + +afterEach(fn () => HttpDriver::resolveHttpBy(fn ($http) => $http)); + +it('leaves the configured timeouts alone without a total timeout', function () use ($captured) { + expect(Location::get('8.8.8.8'))->toBeFalse(); + + foreach ($captured as $options) { + expect($options['timeout'])->toBe(3)->and($options['connect_timeout'])->toBe(3); + } + + expect($captured)->toHaveCount(3); +}); + +it('caps each driver to the time left in the total budget', function () use ($captured) { + config(['location.total_timeout' => 1]); + + expect(Location::get('8.8.8.8'))->toBeFalse(); + expect($captured)->toHaveCount(3); + + // Each driver burns roughly 200ms, so the budget keeps shrinking. + expect($captured[0]['timeout'])->toBeLessThanOrEqual(1.0) + ->and($captured[1]['timeout'])->toBeLessThan($captured[0]['timeout']) + ->and($captured[2]['timeout'])->toBeLessThan($captured[1]['timeout']); +}); + +it('caps timeouts that are unset or disabled', function () use ($captured) { + config(['location.http' => ['timeout' => 0], 'location.total_timeout' => 1]); + + expect(Location::get('8.8.8.8'))->toBeFalse(); + + expect($captured[0]['timeout'])->toBeGreaterThan(0)->toBeLessThanOrEqual(1.0) + ->and($captured[0]['connect_timeout'])->toBeGreaterThan(0)->toBeLessThanOrEqual(1.0); +}); + +it('stops calling drivers once the total budget is spent', function () use ($captured) { + config(['location.total_timeout' => 0.3]); + + expect(Location::get('8.8.8.8'))->toBeFalse(); + expect($captured)->toHaveCount(2); + + // The budget belongs to the lookup, not to the process. + Location::get('8.8.4.4'); + + expect($captured)->toHaveCount(4); +});