From a6e235f6162a6704428c2c9c799290ff18c2b733 Mon Sep 17 00:00:00 2001 From: "Harun R. Rayhan" Date: Mon, 26 Oct 2026 17:19:25 +0600 Subject: [PATCH 1/2] Retry PostSyncer file uploads on 429 instead of marking the upload uncertain PostSyncer throttles media uploads and answers 429 before storing anything. Video groups upload the video, then the cover, back to back, so the cover hit 429 and the whole publish was marked uncertain (BV-110). Retry the same file after Retry-After (capped at 60 s) or a 30/60/60 s backoff; a 429 that outlasts the retries is a definitive rejection. --- app/Support/Postsyncer/PostsyncerClient.php | 45 +++++++- .../Postsyncer/PostsyncerException.php | 3 + .../Postsyncer/PostsyncerClientTest.php | 109 ++++++++++++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) diff --git a/app/Support/Postsyncer/PostsyncerClient.php b/app/Support/Postsyncer/PostsyncerClient.php index e83f2883..6e87c6a0 100644 --- a/app/Support/Postsyncer/PostsyncerClient.php +++ b/app/Support/Postsyncer/PostsyncerClient.php @@ -7,6 +7,7 @@ use GuzzleHttp\Psr7\UriResolver; use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Sleep; use Symfony\Component\Mime\MimeTypes; use Throwable; @@ -17,6 +18,10 @@ class PostsyncerClient private const MAX_DOWNLOAD_REDIRECTS = 5; + private const UPLOAD_RATE_LIMIT_BACKOFF_SECONDS = [30, 60, 60]; + + private const UPLOAD_RATE_LIMIT_MAX_WAIT_SECONDS = 60; + public function __construct( private readonly PostsyncerConfig $config, private readonly ?PublicUrlGuard $urlGuard = null, @@ -215,9 +220,7 @@ public function uploadFilesFromUrls(int|string $workspaceId, array $urls, ?strin : ['Idempotency-Key' => $idempotencyKey.':'.$index]; try { - $response = $this->request('post', '/media/upload/file', [ - 'workspace_id' => $workspaceId, - ], $headers, $path); + $response = $this->uploadFileWithRateLimitRetry($workspaceId, $headers, $path); } catch (PostsyncerException $exception) { // A definitive rejection before any file was stored left // nothing behind in PostSyncer, so a retry cannot duplicate. @@ -267,6 +270,42 @@ public function uploadFilesFromUrls(int|string $workspaceId, array $urls, ?strin } } + /** + * PostSyncer's upload throttle answers 429 before storing anything, so the + * same file is retried after Retry-After (capped) or a fixed backoff. Worst + * case adds 180 s per file, well under the 900-second publish job timeout. + * A 429 that outlasts the retries is still a definitive rejection. + * + * @param array $headers + */ + private function uploadFileWithRateLimitRetry(int|string $workspaceId, array $headers, string $path): Response + { + $backoff = self::UPLOAD_RATE_LIMIT_BACKOFF_SECONDS; + + for ($attempt = 0; ; $attempt++) { + try { + return $this->request('post', '/media/upload/file', [ + 'workspace_id' => $workspaceId, + ], $headers, $path); + } catch (PostsyncerException $exception) { + if ($exception->getCode() !== 429) { + throw $exception; + } + + if ($attempt >= count($backoff)) { + throw new PostsyncerException( + $exception->getMessage().' Gave up after '.count($backoff).' retries.', + 429, + $exception, + responseReceived: true, + ); + } + + Sleep::for(min($exception->retryAfter ?? $backoff[$attempt], self::UPLOAD_RATE_LIMIT_MAX_WAIT_SECONDS))->seconds(); + } + } + } + private function downloadMedia(string $url, string $path): void { $guard = $this->urlGuard ?? app(PublicUrlGuard::class); diff --git a/app/Support/Postsyncer/PostsyncerException.php b/app/Support/Postsyncer/PostsyncerException.php index 0569ee8c..ea02e146 100644 --- a/app/Support/Postsyncer/PostsyncerException.php +++ b/app/Support/Postsyncer/PostsyncerException.php @@ -16,6 +16,7 @@ public function __construct( public readonly bool $outcomeUnknown = false, public readonly bool $responseReceived = false, public readonly bool $safeToRetry = false, + public readonly ?int $retryAfter = null, ) { parent::__construct($message, $code, $previous); } @@ -30,6 +31,7 @@ public static function fromResponse(Response $response): self $status = $response->status(); $outcomeUnknown = in_array($status, [408, 425, 429], true) || $status >= 500; + $retryAfter = $response->header('Retry-After'); return new self( "PostSyncer API error {$status}: {$message}", @@ -38,6 +40,7 @@ public static function fromResponse(Response $response): self $outcomeUnknown, $outcomeUnknown, true, + retryAfter: ctype_digit($retryAfter) ? (int) $retryAfter : null, ); } } diff --git a/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php b/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php index 40eef43d..3b8840e1 100644 --- a/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php +++ b/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php @@ -14,6 +14,7 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Sleep; use Tests\TestCase; class PostsyncerClientTest extends TestCase @@ -194,6 +195,114 @@ public function test_upload_files_from_urls_keeps_a_rejection_after_a_stored_fil } } + public function test_upload_files_from_urls_retries_a_rate_limited_file_and_keeps_order(): void + { + Sleep::fake(); + Http::fake([ + 'drive.usercontent.google.com/*' => fn () => Http::response('bytes', 200), + 'upload.postsyncer.com/api/v1/media/upload/file' => Http::sequence() + ->push(['media' => ['id' => 915]], 201) + ->push(['message' => 'Too Many Attempts.'], 429) + ->push(['media' => ['id' => 916]], 201), + ]); + + $ids = $this->clientWithKey()->uploadFilesFromUrls(15211, [ + 'https://drive.usercontent.google.com/download?id=video', + 'https://drive.usercontent.google.com/download?id=cover', + ], 'op:0:group:media'); + + $this->assertSame([915, 916], $ids); + $uploads = Http::recorded(fn ($request): bool => str_contains($request->url(), '/media/upload/file')); + $this->assertCount(3, $uploads); + $this->assertSame(['op:0:group:media:1'], $uploads->last()[0]->header('Idempotency-Key')); + Sleep::assertSequence([Sleep::for(30)->seconds()]); + } + + public function test_upload_files_from_urls_honors_a_capped_retry_after(): void + { + Sleep::fake(); + Http::fake([ + 'drive.usercontent.google.com/*' => fn () => Http::response('bytes', 200), + 'upload.postsyncer.com/api/v1/media/upload/file' => Http::sequence() + ->push(['message' => 'Too Many Attempts.'], 429, ['Retry-After' => '7']) + ->push(['message' => 'Too Many Attempts.'], 429, ['Retry-After' => '3600']) + ->push(['media' => ['id' => 915]], 201), + ]); + + $ids = $this->clientWithKey()->uploadFilesFromUrls(15211, ['https://drive.usercontent.google.com/download?id=video']); + + $this->assertSame([915], $ids); + Sleep::assertSequence([Sleep::for(7)->seconds(), Sleep::for(60)->seconds()]); + } + + public function test_upload_files_from_urls_gives_up_on_rate_limit_and_names_stored_ids(): void + { + Sleep::fake(); + Http::fake([ + 'drive.usercontent.google.com/*' => fn () => Http::response('bytes', 200), + 'upload.postsyncer.com/api/v1/media/upload/file' => Http::sequence() + ->push(['media' => ['id' => 915]], 201) + ->whenEmpty(Http::response(['message' => 'Too Many Attempts.'], 429)), + ]); + + try { + $this->clientWithKey()->uploadFilesFromUrls(15211, [ + 'https://drive.usercontent.google.com/download?id=video', + 'https://drive.usercontent.google.com/download?id=cover', + ]); + $this->fail('Expected the rate limit to throw.'); + } catch (PostsyncerException $exception) { + $this->assertSame(429, $exception->getCode()); + $this->assertFalse($exception->outcomeUnknown); + $this->assertFalse($exception->safeToRetry); + $this->assertStringContainsString('PostSyncer API error 429: Too Many Attempts.', $exception->getMessage()); + $this->assertStringContainsString('Already stored PostSyncer media ids: 915.', $exception->getMessage()); + } + + Sleep::assertSleptTimes(3); + $this->assertCount(5, Http::recorded(fn ($request): bool => str_contains($request->url(), '/media/upload/file'))); + } + + public function test_upload_files_from_urls_gives_up_on_a_first_file_rate_limit_safely(): void + { + Sleep::fake(); + Http::fake([ + 'drive.usercontent.google.com/*' => fn () => Http::response('bytes', 200), + 'upload.postsyncer.com/api/v1/media/upload/file' => Http::response(['message' => 'Too Many Attempts.'], 429), + ]); + + try { + $this->clientWithKey()->uploadFilesFromUrls(15211, ['https://drive.usercontent.google.com/download?id=video']); + $this->fail('Expected the rate limit to throw.'); + } catch (PostsyncerException $exception) { + $this->assertSame(429, $exception->getCode()); + $this->assertTrue($exception->safeToRetry); + } + } + + public function test_upload_files_from_urls_does_not_retry_a_connection_failure(): void + { + Sleep::fake(); + $attempts = 0; + Http::fake([ + 'drive.usercontent.google.com/*' => fn () => Http::response('bytes', 200), + 'upload.postsyncer.com/api/v1/media/upload/file' => function () use (&$attempts) { + $attempts++; + throw new ConnectionException('timed out'); + }, + ]); + + try { + $this->clientWithKey()->uploadFilesFromUrls(15211, ['https://drive.usercontent.google.com/download?id=video']); + $this->fail('Expected the lost upload response to throw.'); + } catch (PostsyncerException $exception) { + $this->assertTrue($exception->outcomeUnknown); + } + + Sleep::assertNeverSlept(); + $this->assertSame(1, $attempts); + } + public function test_upload_files_from_urls_download_failure_sends_nothing_to_postsyncer(): void { $before = $this->mediaTempFiles(); From b867385a46c72f5b75cf74a18cd913473023953f Mon Sep 17 00:00:00 2001 From: "Harun R. Rayhan" Date: Mon, 26 Oct 2026 17:21:52 +0600 Subject: [PATCH 2/2] Share one 120 s rate-limit wait budget across a group's file uploads (review) --- app/Support/Postsyncer/PostsyncerClient.php | 20 ++++++++---- .../Postsyncer/PostsyncerClientTest.php | 32 +++++++++++++++++-- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/app/Support/Postsyncer/PostsyncerClient.php b/app/Support/Postsyncer/PostsyncerClient.php index 6e87c6a0..b735ed8b 100644 --- a/app/Support/Postsyncer/PostsyncerClient.php +++ b/app/Support/Postsyncer/PostsyncerClient.php @@ -22,6 +22,9 @@ class PostsyncerClient private const UPLOAD_RATE_LIMIT_MAX_WAIT_SECONDS = 60; + // Shared by every file in one publish group so retries stay well inside the job timeout. + private const UPLOAD_RATE_LIMIT_TOTAL_WAIT_SECONDS = 120; + public function __construct( private readonly PostsyncerConfig $config, private readonly ?PublicUrlGuard $urlGuard = null, @@ -201,6 +204,7 @@ public function uploadFromUrls(int|string $workspaceId, array $urls, ?string $id public function uploadFilesFromUrls(int|string $workspaceId, array $urls, ?string $idempotencyKey = null): array { $paths = []; + $waitBudget = self::UPLOAD_RATE_LIMIT_TOTAL_WAIT_SECONDS; try { foreach ($urls as $url) { @@ -220,7 +224,7 @@ public function uploadFilesFromUrls(int|string $workspaceId, array $urls, ?strin : ['Idempotency-Key' => $idempotencyKey.':'.$index]; try { - $response = $this->uploadFileWithRateLimitRetry($workspaceId, $headers, $path); + $response = $this->uploadFileWithRateLimitRetry($workspaceId, $headers, $path, $waitBudget); } catch (PostsyncerException $exception) { // A definitive rejection before any file was stored left // nothing behind in PostSyncer, so a retry cannot duplicate. @@ -272,13 +276,14 @@ public function uploadFilesFromUrls(int|string $workspaceId, array $urls, ?strin /** * PostSyncer's upload throttle answers 429 before storing anything, so the - * same file is retried after Retry-After (capped) or a fixed backoff. Worst - * case adds 180 s per file, well under the 900-second publish job timeout. + * same file is retried after Retry-After (capped) or a fixed backoff. All + * files in one call share a 120 s wait budget, so retries plus re-sent + * uploads stay inside the 900-second publish job timeout. * A 429 that outlasts the retries is still a definitive rejection. * * @param array $headers */ - private function uploadFileWithRateLimitRetry(int|string $workspaceId, array $headers, string $path): Response + private function uploadFileWithRateLimitRetry(int|string $workspaceId, array $headers, string $path, int &$waitBudget): Response { $backoff = self::UPLOAD_RATE_LIMIT_BACKOFF_SECONDS; @@ -292,7 +297,9 @@ private function uploadFileWithRateLimitRetry(int|string $workspaceId, array $he throw $exception; } - if ($attempt >= count($backoff)) { + $wait = min($exception->retryAfter ?? $backoff[$attempt] ?? 0, self::UPLOAD_RATE_LIMIT_MAX_WAIT_SECONDS); + + if ($attempt >= count($backoff) || $wait > $waitBudget) { throw new PostsyncerException( $exception->getMessage().' Gave up after '.count($backoff).' retries.', 429, @@ -301,7 +308,8 @@ private function uploadFileWithRateLimitRetry(int|string $workspaceId, array $he ); } - Sleep::for(min($exception->retryAfter ?? $backoff[$attempt], self::UPLOAD_RATE_LIMIT_MAX_WAIT_SECONDS))->seconds(); + $waitBudget -= $wait; + Sleep::for($wait)->seconds(); } } } diff --git a/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php b/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php index 3b8840e1..e6103e91 100644 --- a/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php +++ b/tests/Unit/Support/Postsyncer/PostsyncerClientTest.php @@ -259,8 +259,36 @@ public function test_upload_files_from_urls_gives_up_on_rate_limit_and_names_sto $this->assertStringContainsString('Already stored PostSyncer media ids: 915.', $exception->getMessage()); } - Sleep::assertSleptTimes(3); - $this->assertCount(5, Http::recorded(fn ($request): bool => str_contains($request->url(), '/media/upload/file'))); + // 30 + 60 s spends the shared budget; the third 60 s wait would exceed it. + Sleep::assertSequence([Sleep::for(30)->seconds(), Sleep::for(60)->seconds()]); + $this->assertCount(4, Http::recorded(fn ($request): bool => str_contains($request->url(), '/media/upload/file'))); + } + + public function test_upload_files_from_urls_shares_one_rate_limit_wait_budget_across_files(): void + { + Sleep::fake(); + Http::fake([ + 'drive.usercontent.google.com/*' => fn () => Http::response('bytes', 200), + 'upload.postsyncer.com/api/v1/media/upload/file' => Http::sequence() + ->push(['message' => 'Too Many Attempts.'], 429, ['Retry-After' => '60']) + ->push(['message' => 'Too Many Attempts.'], 429, ['Retry-After' => '60']) + ->push(['media' => ['id' => 915]], 201) + ->whenEmpty(Http::response(['message' => 'Too Many Attempts.'], 429, ['Retry-After' => '1'])), + ]); + + try { + $this->clientWithKey()->uploadFilesFromUrls(15211, [ + 'https://drive.usercontent.google.com/download?id=video', + 'https://drive.usercontent.google.com/download?id=cover', + ]); + $this->fail('Expected the exhausted budget to throw.'); + } catch (PostsyncerException $exception) { + $this->assertSame(429, $exception->getCode()); + $this->assertStringContainsString('Already stored PostSyncer media ids: 915.', $exception->getMessage()); + } + + Sleep::assertSequence([Sleep::for(60)->seconds(), Sleep::for(60)->seconds()]); + $this->assertCount(4, Http::recorded(fn ($request): bool => str_contains($request->url(), '/media/upload/file'))); } public function test_upload_files_from_urls_gives_up_on_a_first_file_rate_limit_safely(): void