From 9f3386c4a33769249cc53edc2212786848a5f910 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 25 Mar 2026 09:22:40 -0400 Subject: [PATCH 01/10] feat(S3): make readObject handle transient failures Since we bypass the SDK to support #20033, we need to emulate more of its behavior to keep readObject consistent with other transactions. This adds basic retry behavior for the main scenarios. Signed-off-by: Josh --- .../Files/ObjectStore/S3ObjectTrait.php | 107 ++++++++++++------ 1 file changed, 75 insertions(+), 32 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 384e9f0ff58c9..ac0d588f701d5 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -42,39 +42,82 @@ abstract protected function getSSECParameters(bool $copy = false): array; */ public function readObject($urn) { $fh = SeekableHttpStream::open(function ($range) use ($urn) { - $command = $this->getConnection()->getCommand('GetObject', [ - 'Bucket' => $this->bucket, - 'Key' => $urn, - 'Range' => 'bytes=' . $range, - ] + $this->getSSECParameters()); - $request = \Aws\serialize($command); - $headers = []; - foreach ($request->getHeaders() as $key => $values) { - foreach ($values as $value) { - $headers[] = "$key: $value"; - } - } - $opts = [ - 'http' => [ - 'protocol_version' => $request->getProtocolVersion(), - 'header' => $headers, - ] - ]; - $bundle = $this->getCertificateBundlePath(); - if ($bundle) { - $opts['ssl'] = [ - 'cafile' => $bundle - ]; - } - - if ($this->getProxy()) { - $opts['http']['proxy'] = $this->getProxy(); - $opts['http']['request_fulluri'] = true; - } - - $context = stream_context_create($opts); - return fopen($request->getUri(), 'r', false, $context); + $command = $this->getConnection()->getCommand('GetObject', [ + 'Bucket' => $this->bucket, + 'Key' => $urn, + 'Range' => 'bytes=' . $range, + ] + $this->getSSECParameters()); + $request = \Aws\serialize($command); + $headers = []; + foreach ($request->getHeaders() as $key => $values) { + foreach ($values as $value) { + $headers[] = "$key: $value"; + } + } + $opts = [ + 'http' => [ + 'protocol_version' => $request->getProtocolVersion(), + 'header' => $headers, + 'ignore_errors' => true, // prevent fopen from failing on 4xx/5xx + ] + ]; + + $bundle = $this->getCertificateBundlePath(); + if ($bundle) { + $opts['ssl'] = [ + 'cafile' => $bundle + ]; + } + + if ($this->getProxy()) { + $opts['http']['proxy'] = $this->getProxy(); + $opts['http']['request_fulluri'] = true; + } + + $context = stream_context_create($opts); + + $maxAttempts = $this->retriesMaxAttempts; + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + $result = @fopen($request->getUri(), 'r', false, $context); + + if ($result !== false) { + $meta = stream_get_meta_data($result); + $statusLine = $meta['wrapper_data'][0] ?? ''; + if (preg_match('#^HTTP/\S+\s+(\d{3})#', $statusLine, $matches)) { + $statusCode = (int)$matches[1]; + + if ($statusCode < 400) { + return $result; + } + + fclose($result); + + // Only retry on server errors or throttling + if ($statusCode === 429 || $statusCode >= 500) { + if ($attempt < $maxAttempts) { + usleep(min(1000000, 100000 * (2 ** ($attempt - 1)))); + continue; + } + } + + // 4xx (except 429) — don't retry, fail immediately + return false; + } + + // Couldn't parse status but got a stream — use it + return $result; + } + + // fopen returned false — connection-level failure (DNS, timeout, etc.) + // These are retryable + if ($attempt < $maxAttempts) { + usleep(min(1000000, 100000 * (2 ** ($attempt - 1)))); + } + } + + return false; }); + if (!$fh) { throw new \Exception("Failed to read object $urn"); } From db4288e18853524160321648a10fc78445625d41 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 25 Mar 2026 09:49:12 -0400 Subject: [PATCH 02/10] chore(S3): fix status parsing, more error context, jitter in readObject Signed-off-by: Josh --- .../Files/ObjectStore/S3ObjectTrait.php | 251 ++++++++++++------ 1 file changed, 175 insertions(+), 76 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index ac0d588f701d5..a2bfd490d7a88 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -41,89 +41,188 @@ abstract protected function getSSECParameters(bool $copy = false): array; * @since 7.0.0 */ public function readObject($urn) { - $fh = SeekableHttpStream::open(function ($range) use ($urn) { - $command = $this->getConnection()->getCommand('GetObject', [ - 'Bucket' => $this->bucket, - 'Key' => $urn, - 'Range' => 'bytes=' . $range, - ] + $this->getSSECParameters()); - $request = \Aws\serialize($command); - $headers = []; - foreach ($request->getHeaders() as $key => $values) { - foreach ($values as $value) { - $headers[] = "$key: $value"; - } - } - $opts = [ - 'http' => [ - 'protocol_version' => $request->getProtocolVersion(), - 'header' => $headers, - 'ignore_errors' => true, // prevent fopen from failing on 4xx/5xx - ] - ]; - - $bundle = $this->getCertificateBundlePath(); - if ($bundle) { - $opts['ssl'] = [ - 'cafile' => $bundle - ]; - } - - if ($this->getProxy()) { - $opts['http']['proxy'] = $this->getProxy(); - $opts['http']['request_fulluri'] = true; - } - - $context = stream_context_create($opts); - - $maxAttempts = $this->retriesMaxAttempts; - for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { - $result = @fopen($request->getUri(), 'r', false, $context); - - if ($result !== false) { - $meta = stream_get_meta_data($result); - $statusLine = $meta['wrapper_data'][0] ?? ''; - if (preg_match('#^HTTP/\S+\s+(\d{3})#', $statusLine, $matches)) { - $statusCode = (int)$matches[1]; - - if ($statusCode < 400) { - return $result; - } - - fclose($result); - - // Only retry on server errors or throttling - if ($statusCode === 429 || $statusCode >= 500) { - if ($attempt < $maxAttempts) { - usleep(min(1000000, 100000 * (2 ** ($attempt - 1)))); - continue; - } - } - - // 4xx (except 429) — don't retry, fail immediately - return false; - } - - // Couldn't parse status but got a stream — use it - return $result; - } - - // fopen returned false — connection-level failure (DNS, timeout, etc.) - // These are retryable - if ($attempt < $maxAttempts) { - usleep(min(1000000, 100000 * (2 ** ($attempt - 1)))); - } - } - - return false; + $maxAttempts = max(1, $this->retriesMaxAttempts); + $lastError = 'unknown error'; + + $fh = SeekableHttpStream::open(function ($range) use ($urn, $maxAttempts, &$lastError) { + $command = $this->getConnection()->getCommand('GetObject', [ + 'Bucket' => $this->bucket, + 'Key' => $urn, + 'Range' => 'bytes=' . $range, + ] + $this->getSSECParameters()); + + $request = \Aws\serialize($command); + $requestUri = (string)$request->getUri(); + + $headers = []; + foreach ($request->getHeaders() as $key => $values) { + foreach ($values as $value) { + $headers[] = "$key: $value"; + } + } + + $opts = [ + 'http' => [ + 'protocol_version' => $request->getProtocolVersion(), + 'header' => $headers, + 'ignore_errors' => true, + ], + ]; + + $bundle = $this->getCertificateBundlePath(); + if ($bundle) { + $opts['ssl'] = [ + 'cafile' => $bundle, + ]; + } + + if ($this->getProxy()) { + $opts['http']['proxy'] = $this->getProxy(); + $opts['http']['request_fulluri'] = true; + } + + $context = stream_context_create($opts); + + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + $result = @fopen($requestUri, 'r', false, $context); + + if ($result !== false) { + $meta = stream_get_meta_data($result); + $responseHead = is_array($meta['wrapper_data'] ?? null) ? $meta['wrapper_data'] : []; + $statusCode = $this->parseHttpStatusCode($responseHead); + + if ($statusCode !== null && $statusCode < 400) { + return $result; + } + + $errorBody = stream_get_contents($result); + fclose($result); + + $errorInfo = $this->parseS3ErrorResponse($errorBody, $responseHead); + $lastError = $this->formatS3ReadError($urn, $range, $statusCode, $errorInfo, $attempt, $maxAttempts); + + if ($this->isRetryableHttpStatus($statusCode) && $attempt < $maxAttempts) { + $this->sleepBeforeRetry($attempt); + continue; + } + + return false; + } + + $lastError = "connection failure while reading object $urn range $range on attempt $attempt/$maxAttempts"; + + if ($attempt < $maxAttempts) { + $this->sleepBeforeRetry($attempt); + } + } + + return false; }); if (!$fh) { - throw new \Exception("Failed to read object $urn"); + throw new \Exception("Failed to read object $urn after $maxAttempts attempts: $lastError"); } + return $fh; } + /** + * Parse the effective HTTP status code from stream wrapper metadata. + * + * wrapper_data can contain multiple status lines, for example due to redirects, + * proxy responses, or interim 100 responses. We want the last HTTP status line. + * + * @param array|string $responseHead The wrapper_data from stream_get_meta_data + */ + private function parseHttpStatusCode(array|string $responseHead): ?int { + $lines = is_array($responseHead) ? $responseHead : [$responseHead]; + + foreach (array_reverse($lines) as $line) { + if (is_string($line) && preg_match('#^HTTP/\S+\s+(\d{3})#', $line, $matches)) { + return (int)$matches[1]; + } + } + + return null; + } + + /** + * Parse S3 error response XML and response headers into a structured array. + * + * @param string|false $body The response body + * @param array $responseHead The wrapper_data from stream_get_meta_data + * @return array{code: string, message: string, requestId: string, extendedRequestId: string} + */ + private function parseS3ErrorResponse(string|false $body, array $responseHead): array { + $errorCode = 'Unknown'; + $errorMessage = ''; + $requestId = ''; + $extendedRequestId = ''; + + if ($body) { + $xml = @simplexml_load_string($body); + if ($xml !== false) { + $errorCode = (string)($xml->Code ?? 'Unknown'); + $errorMessage = (string)($xml->Message ?? ''); + $requestId = (string)($xml->RequestId ?? ''); + } + } + + foreach ($responseHead as $header) { + if (!is_string($header)) { + continue; + } + + if (stripos($header, 'x-amz-request-id:') === 0) { + $requestId = trim(substr($header, strlen('x-amz-request-id:'))); + } elseif (stripos($header, 'x-amz-id-2:') === 0) { + $extendedRequestId = trim(substr($header, strlen('x-amz-id-2:'))); + } + } + + return [ + 'code' => $errorCode, + 'message' => $errorMessage, + 'requestId' => $requestId, + 'extendedRequestId' => $extendedRequestId, + ]; + } + + /** + * @param array{code: string, message: string, requestId: string, extendedRequestId: string} $errorInfo + */ + private function formatS3ReadError( + string $urn, + string $range, + ?int $statusCode, + array $errorInfo, + int $attempt, + int $maxAttempts, + ): string { + return sprintf( + 'HTTP %s reading object %s range %s on attempt %d/%d: %s - %s (RequestId: %s, ExtendedRequestId: %s)', + $statusCode !== null ? (string)$statusCode : 'unknown', + $urn, + $range, + $attempt, + $maxAttempts, + $errorInfo['code'], + $errorInfo['message'], + $errorInfo['requestId'], + $errorInfo['extendedRequestId'], + ); + } + + private function isRetryableHttpStatus(?int $statusCode): bool { + return $statusCode === 429 || ($statusCode !== null && $statusCode >= 500); + } + + private function sleepBeforeRetry(int $attempt): void { + $delay = min(1000000, 100000 * (2 ** ($attempt - 1))); + $delay += random_int(0, 100000); + usleep($delay); + } + private function buildS3Metadata(array $metadata): array { $result = []; foreach ($metadata as $key => $value) { From 512f8d528c12d72566735a1d83894decc7d14b43 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 25 Mar 2026 10:13:45 -0400 Subject: [PATCH 03/10] feat(S3): add readObject retry/error handling logging Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index a2bfd490d7a88..2500631b23cc5 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -44,7 +44,11 @@ public function readObject($urn) { $maxAttempts = max(1, $this->retriesMaxAttempts); $lastError = 'unknown error'; - $fh = SeekableHttpStream::open(function ($range) use ($urn, $maxAttempts, &$lastError) { + // TODO: consider unifying logger access across S3ConnectionTrait and S3ObjectTrait + // via an abstract method (e.g. getLogger()) rather than inline container lookups + $logger = \OCP\Server::get(\Psr\Log\LoggerInterface::class); + + $fh = SeekableHttpStream::open(function ($range) use ($urn, $maxAttempts, &$lastError, $logger) { $command = $this->getConnection()->getCommand('GetObject', [ 'Bucket' => $this->bucket, 'Key' => $urn, @@ -102,14 +106,21 @@ public function readObject($urn) { $lastError = $this->formatS3ReadError($urn, $range, $statusCode, $errorInfo, $attempt, $maxAttempts); if ($this->isRetryableHttpStatus($statusCode) && $attempt < $maxAttempts) { + // gives operators visibility into transient S3 issues even when retries succeed by logging + $logger->warning($lastError, ['app' => 'objectstore']); $this->sleepBeforeRetry($attempt); continue; } + // for non-retryable HTTP errors or exhausted retries, log the final failure with full S3 error context + $logger->error($lastError, ['app' => 'objectstore']); return false; } + // fopen returned false - i.e. connection-level failure (DNS, timeout, TLS, etc.) + // log occurences for operator visibility even if retried $lastError = "connection failure while reading object $urn range $range on attempt $attempt/$maxAttempts"; + $logger->warning($lastError, ['app' => 'objectstore']); if ($attempt < $maxAttempts) { $this->sleepBeforeRetry($attempt); @@ -129,8 +140,8 @@ public function readObject($urn) { /** * Parse the effective HTTP status code from stream wrapper metadata. * - * wrapper_data can contain multiple status lines, for example due to redirects, - * proxy responses, or interim 100 responses. We want the last HTTP status line. + * wrapper_data can contain multiple status lines (e.g. 100 Continue, + * redirects, proxy responses). We want the last HTTP status line. * * @param array|string $responseHead The wrapper_data from stream_get_meta_data */ From f8d1c482ab83277c3e4b7edae995420d9fe0f9ed Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 26 Mar 2026 21:39:55 -0400 Subject: [PATCH 04/10] chore(s3): avoid silently losing status/header if non-array Robustness. Avoids silently losing the only status/header information available if we happen to get a string. Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 2500631b23cc5..85f6f52b7ca1b 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -92,7 +92,7 @@ public function readObject($urn) { if ($result !== false) { $meta = stream_get_meta_data($result); - $responseHead = is_array($meta['wrapper_data'] ?? null) ? $meta['wrapper_data'] : []; + $responseHead = $meta['wrapper_data'] ?? []; $statusCode = $this->parseHttpStatusCode($responseHead); if ($statusCode !== null && $statusCode < 400) { @@ -102,7 +102,10 @@ public function readObject($urn) { $errorBody = stream_get_contents($result); fclose($result); - $errorInfo = $this->parseS3ErrorResponse($errorBody, $responseHead); + $errorInfo = $this->parseS3ErrorResponse( + $errorBody, + is_array($responseHead) ? $responseHead : [$responseHead] + ); $lastError = $this->formatS3ReadError($urn, $range, $statusCode, $errorInfo, $attempt, $maxAttempts); if ($this->isRetryableHttpStatus($statusCode) && $attempt < $maxAttempts) { From a569c0083fddc4dc79dae85bd8af00180846f533 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 26 Mar 2026 21:48:55 -0400 Subject: [PATCH 05/10] chore(s3): make 416 failures more explicit in readObject Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 85f6f52b7ca1b..da5fca923b1ea 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -110,7 +110,7 @@ public function readObject($urn) { if ($this->isRetryableHttpStatus($statusCode) && $attempt < $maxAttempts) { // gives operators visibility into transient S3 issues even when retries succeed by logging - $logger->warning($lastError, ['app' => 'objectstore']); + $logger->info($lastError, ['app' => 'objectstore']); $this->sleepBeforeRetry($attempt); continue; } @@ -213,6 +213,19 @@ private function formatS3ReadError( int $attempt, int $maxAttempts, ): string { + if ($statusCode === 416) { + return sprintf( + 'HTTP 416 reading object %s range %s on attempt %d/%d: requested range not satisfiable', + $urn, + $range, + $attempt, + $maxAttempts, + $errorInfo['code'], + $errorInfo['message'], + $errorInfo['requestId'], + $errorInfo['extendedRequestId'], + ); + } return sprintf( 'HTTP %s reading object %s range %s on attempt %d/%d: %s - %s (RequestId: %s, ExtendedRequestId: %s)', $statusCode !== null ? (string)$statusCode : 'unknown', From 39c616cb171f13f8557cbf6828e8675aa28a6e5b Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 26 Mar 2026 22:06:13 -0400 Subject: [PATCH 06/10] chore(s3): track both first and last failure during retries in readObject Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index da5fca923b1ea..5909bab131cd9 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -43,6 +43,7 @@ abstract protected function getSSECParameters(bool $copy = false): array; public function readObject($urn) { $maxAttempts = max(1, $this->retriesMaxAttempts); $lastError = 'unknown error'; + $firstError = 'unknown error'; // TODO: consider unifying logger access across S3ConnectionTrait and S3ObjectTrait // via an abstract method (e.g. getLogger()) rather than inline container lookups @@ -106,17 +107,23 @@ public function readObject($urn) { $errorBody, is_array($responseHead) ? $responseHead : [$responseHead] ); - $lastError = $this->formatS3ReadError($urn, $range, $statusCode, $errorInfo, $attempt, $maxAttempts); + $currentError = $this->formatS3ReadError($urn, $range, $statusCode, $errorInfo, $attempt, $maxAttempts); + // on retries, the last or the first failure can be most informative, but can't know which so track both + if ($firstError === 'unknown error') { + $firstError = $currentError; + } else { + $lastError = $currentError; + } if ($this->isRetryableHttpStatus($statusCode) && $attempt < $maxAttempts) { // gives operators visibility into transient S3 issues even when retries succeed by logging - $logger->info($lastError, ['app' => 'objectstore']); + $logger->warning($currentError, ['app' => 'objectstore']); $this->sleepBeforeRetry($attempt); continue; } // for non-retryable HTTP errors or exhausted retries, log the final failure with full S3 error context - $logger->error($lastError, ['app' => 'objectstore']); + $logger->error($currentError, ['app' => 'objectstore']); return false; } @@ -134,7 +141,9 @@ public function readObject($urn) { }); if (!$fh) { - throw new \Exception("Failed to read object $urn after $maxAttempts attempts: $lastError"); + throw new \Exception( + "Failed to read object $urn after $maxAttempts attempts. First failure: $firstError. Last failure: $lastError" + ); } return $fh; From fbd43d60cc0aa77f5bd507b764c03471d2df04b8 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 26 Mar 2026 22:08:20 -0400 Subject: [PATCH 07/10] chore(s3): distinguish transport failure from HTTP failure in readObject At least from a logging perspective Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 5909bab131cd9..b841516e4ad8a 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -129,7 +129,7 @@ public function readObject($urn) { // fopen returned false - i.e. connection-level failure (DNS, timeout, TLS, etc.) // log occurences for operator visibility even if retried - $lastError = "connection failure while reading object $urn range $range on attempt $attempt/$maxAttempts"; + $lastError = "connection failure while reading object $urn range $range on attempt $attempt/$maxAttempts (no HTTP response received)"; $logger->warning($lastError, ['app' => 'objectstore']); if ($attempt < $maxAttempts) { From a7e91e23154624dc8e8fef5ce1188fd8e7f374b6 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 26 Mar 2026 22:15:12 -0400 Subject: [PATCH 08/10] chore(s3): normalize blank S3 error fields in readObject Signed-off-by: Josh --- .../Files/ObjectStore/S3ObjectTrait.php | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index b841516e4ad8a..0f3f0b5da804d 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -222,6 +222,11 @@ private function formatS3ReadError( int $attempt, int $maxAttempts, ): string { + $errorCode = $errorInfo['code'] !== '' ? $errorInfo['code'] : 'Unknown'; + $errorMessage = $errorInfo['message'] !== '' ? $errorInfo['message'] : 'No error message'; + $requestId = $errorInfo['requestId'] !== '' ? $errorInfo['requestId'] : 'n/a'; + $extendedRequestId = $errorInfo['extendedRequestId'] !== '' ? $errorInfo['extendedRequestId'] : 'n/a'; + if ($statusCode === 416) { return sprintf( 'HTTP 416 reading object %s range %s on attempt %d/%d: requested range not satisfiable', @@ -229,10 +234,10 @@ private function formatS3ReadError( $range, $attempt, $maxAttempts, - $errorInfo['code'], - $errorInfo['message'], - $errorInfo['requestId'], - $errorInfo['extendedRequestId'], + $errorCode, + $errorMessage, + $requestId, + $extendedRequestId, ); } return sprintf( @@ -242,10 +247,10 @@ private function formatS3ReadError( $range, $attempt, $maxAttempts, - $errorInfo['code'], - $errorInfo['message'], - $errorInfo['requestId'], - $errorInfo['extendedRequestId'], + $errorCode, + $errorMessage, + $requestId, + $extendedRequestId, ); } From a4c8bbafa0badae1d775edc1d7b6644c005ace9a Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 26 Mar 2026 22:31:27 -0400 Subject: [PATCH 09/10] docs(s3): document retry budget in readObject Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 0f3f0b5da804d..7a43531b64b0b 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -88,6 +88,12 @@ public function readObject($urn) { $context = stream_context_create($opts); + // Retries here are per ranged HTTP fetch, not per high-level readObject() call. + // A single read may therefore trigger multiple retry sequences across seeks/reopens. + // sleepBeforeRetry() deliberately caps backoff to limit per-range delay. + // If this ever becomes an issue we might: + // - use a smaller retry count for read ranges than for write operations, or + // - cap total sleep time more aggressively. for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { $result = @fopen($requestUri, 'r', false, $context); From 743abffdb14b6eb84aa1b98f1a7e6714261b714d Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 27 Mar 2026 07:56:53 -0400 Subject: [PATCH 10/10] chore(s3): readobject retry implementation cleanup Signed-off-by: Josh --- lib/private/Files/ObjectStore/S3ObjectTrait.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 7a43531b64b0b..959df4fea6145 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -1,7 +1,7 @@ getConnection()->getCommand('GetObject', [ 'Bucket' => $this->bucket, 'Key' => $urn, @@ -134,8 +134,14 @@ public function readObject($urn) { } // fopen returned false - i.e. connection-level failure (DNS, timeout, TLS, etc.) - // log occurences for operator visibility even if retried - $lastError = "connection failure while reading object $urn range $range on attempt $attempt/$maxAttempts (no HTTP response received)"; + // log occurrences for operator visibility even if retried + $currentError = "connection failure while reading object $urn range $range on attempt $attempt/$maxAttempts (no HTTP response received)"; + if ($firstError === 'unknown error') { + $firstError = $currentError; + } else { + $lastError = $currentError; + } + $logger->warning($lastError, ['app' => 'objectstore']); if ($attempt < $maxAttempts) { @@ -235,7 +241,7 @@ private function formatS3ReadError( if ($statusCode === 416) { return sprintf( - 'HTTP 416 reading object %s range %s on attempt %d/%d: requested range not satisfiable', + 'HTTP 416 reading object %s range %s on attempt %d/%d: requested range not satisfiable [%s - %s (RequestId: %s, ExtendedRequestId: %s)]', $urn, $range, $attempt,