diff --git a/src/Migration/Target.php b/src/Migration/Target.php index f31976f2..a2ede6dc 100644 --- a/src/Migration/Target.php +++ b/src/Migration/Target.php @@ -16,8 +16,14 @@ abstract class Target public const int CONNECT_TIMEOUT = 10; /** - * Seconds a request may take in total. Requests are single API pages and - * file chunks, so a remote that has stopped answering fails within this. + * Seconds a request may go without receiving a byte before it is abandoned. + * Transfers of any length stay alive as long as data keeps arriving. + */ + public const int STALL_TIMEOUT = 30; + + /** + * Seconds an Appwrite SDK request may take in total. The SDK transport only + * takes a total timeout; its requests are single API pages and file chunks. */ public const int REQUEST_TIMEOUT = 30; @@ -156,10 +162,13 @@ protected function call( */ protected function client(): Client { - return $this->client ??= (new Client(new CurlAdapter())) + return $this->client ??= (new Client(new CurlAdapter(options: [ + \CURLOPT_TIMEOUT_MS => 0, + \CURLOPT_LOW_SPEED_LIMIT => 1, + \CURLOPT_LOW_SPEED_TIME => static::STALL_TIMEOUT, + ]))) ->withFollowRedirects() - ->withConnectTimeout(static::CONNECT_TIMEOUT) - ->withTimeout(static::REQUEST_TIMEOUT); + ->withConnectTimeout(static::CONNECT_TIMEOUT); } /** diff --git a/tests/Migration/Unit/General/ShortTimeoutTarget.php b/tests/Migration/Unit/General/ShortTimeoutTarget.php new file mode 100644 index 00000000..e2fc6584 --- /dev/null +++ b/tests/Migration/Unit/General/ShortTimeoutTarget.php @@ -0,0 +1,48 @@ +|string + * + * @throws \Exception + */ + public function fetch(string $url): array|string + { + return $this->call('GET', $url); + } +} diff --git a/tests/Migration/Unit/General/TargetTimeoutServerBehaviour.php b/tests/Migration/Unit/General/TargetTimeoutServerBehaviour.php new file mode 100644 index 00000000..84d21cbb --- /dev/null +++ b/tests/Migration/Unit/General/TargetTimeoutServerBehaviour.php @@ -0,0 +1,14 @@ +server)) { + \proc_terminate($this->server, 9); + \proc_close($this->server); + } + + $this->server = null; + } + + public function testTransferOutlastingEveryTimeoutSucceedsWhileDataKeepsArriving(): void + { + $url = $this->serve(Behaviour::Streaming, self::HTTP); + + $started = \microtime(true); + $body = (new ShortTimeoutTarget())->fetch($url); + $elapsed = \microtime(true) - $started; + + $this->assertSame(\str_repeat('x', Behaviour::STREAMED_BYTES), $body); + $this->assertGreaterThan( + \max(ShortTimeoutTarget::REQUEST_TIMEOUT, ShortTimeoutTarget::STALL_TIMEOUT), + $elapsed, + 'The transfer must outlast the request and stall timeouts to show neither caps a progressing transfer.', + ); + } + + /** + * @return array + */ + public static function silentRemotes(): array + { + return [ + 'accepts and never answers' => [Behaviour::Silent, self::HTTP, ShortTimeoutTarget::STALL_TIMEOUT, self::GIVE_UP_WITHIN_SECONDS], + 'stops sending mid-body' => [Behaviour::Stalled, self::HTTP, ShortTimeoutTarget::STALL_TIMEOUT, self::GIVE_UP_WITHIN_SECONDS], + 'never completes the connection' => [Behaviour::Silent, self::HTTPS, ShortTimeoutTarget::CONNECT_TIMEOUT, ShortTimeoutTarget::STALL_TIMEOUT], + ]; + } + + #[DataProvider('silentRemotes')] + public function testSilentRemoteFailsOnItsTimeout(Behaviour $behaviour, string $scheme, int $timeout, float $before): void + { + $url = $this->serve($behaviour, $scheme); + + $started = \microtime(true); + $error = null; + + try { + (new ShortTimeoutTarget())->fetch($url); + } catch (\Exception $caught) { + $error = $caught; + } + + $elapsed = \microtime(true) - $started; + + $this->assertInstanceOf(\Exception::class, $error, 'A remote that sends nothing must fail the request.'); + $this->assertSame(Exception::CODE_INTERNAL, $error->getCode(), $error->getMessage()); + $this->assertGreaterThanOrEqual( + $timeout - self::TIMER_TOLERANCE_SECONDS, + $elapsed, + 'The request must wait out its timeout rather than fail for another reason: '.$error->getMessage(), + ); + $this->assertLessThan($before, $elapsed, 'The request must fail on its own timeout: '.$error->getMessage()); + } + + private function serve(Behaviour $behaviour, string $scheme): string + { + $script = \dirname(__DIR__, 2).'/resources/timeout/server.php'; + $server = \proc_open([PHP_BINARY, $script, $behaviour->value], [1 => ['pipe', 'w']], $pipes); + $this->assertIsResource($server); + $this->server = $server; + + $port = \trim((string) \fgets($pipes[1])); + $this->assertMatchesRegularExpression('/^\d+$/', $port, 'The test server did not report its port.'); + + return "{$scheme}://127.0.0.1:{$port}/"; + } +} diff --git a/tests/Migration/resources/timeout/server.php b/tests/Migration/resources/timeout/server.php new file mode 100644 index 00000000..df143132 --- /dev/null +++ b/tests/Migration/resources/timeout/server.php @@ -0,0 +1,50 @@ +