From 7f648f8d4d91c10e57786002a10e2eae38fa419b Mon Sep 17 00:00:00 2001 From: Patrick Schrall Date: Tue, 23 Jun 2026 08:32:27 +0200 Subject: [PATCH] fix(bluesky): harden video upload per review + retry transient transcode failures Address review feedback and a transient failure mode seen in production: - Guard tempnam()/fopen() returning false (bail cleanly instead of a TypeError later) and guard the finally-block unlink() with is_string(). - pollVideoJob() now bails on a non-2xx getJobStatus (e.g. expired token) instead of sleeping to the timeout and masking the real error. - resolvePdsEndpoint() maps did:web colon segments to host + path per the did:web spec (did:web:example.com:user:alice -> .../user/alice/did.json) instead of treating the whole remainder as a host. - The video service occasionally fails a job transiently (JOB_STATE_FAILED "Failed to process video") for valid input, so the upload+poll is retried up to VIDEO_UPLOAD_ATTEMPTS times (split into a new attemptVideoUpload() helper; the upload service-auth token is minted once and reused across attempts). Adds a test covering retry-after-transient-failure. Full suite (23) green. Co-Authored-By: Claude Opus 4.8 --- app/Services/Social/BlueskyPublisher.php | 151 +++++++++++++----- .../Services/Social/BlueskyPublisherTest.php | 65 +++++++- 2 files changed, 172 insertions(+), 44 deletions(-) diff --git a/app/Services/Social/BlueskyPublisher.php b/app/Services/Social/BlueskyPublisher.php index fc6664fe..71e08e6f 100644 --- a/app/Services/Social/BlueskyPublisher.php +++ b/app/Services/Social/BlueskyPublisher.php @@ -20,6 +20,9 @@ class BlueskyPublisher { use HasSocialHttpClient; + /** Times to retry a transiently-failing video transcode before giving up. */ + private const VIDEO_UPLOAD_ATTEMPTS = 3; + public function publish(PostPlatform $postPlatform): array { $this->validateContentLength($postPlatform); @@ -206,6 +209,12 @@ private function uploadVideo(SocialAccount $account, string $service, string $ur $tempFile = tempnam(sys_get_temp_dir(), 'bsky_video_'); + if ($tempFile === false) { + Log::error('Bluesky could not create temp file for video upload', ['url' => $url]); + + return null; + } + try { $downloadResponse = Http::withOptions(['sink' => $tempFile])->timeout(600)->get($url); @@ -240,55 +249,32 @@ private function uploadVideo(SocialAccount $account, string $service, string $ur // The upload token is consumed by the video service to write the // blob back to the user's PDS, so its audience is the PDS itself. + // Minted once (valid 30 min) and reused across retries below. $uploadToken = $this->getServiceAuth($account, $pds, "did:web:{$pdsHost}", BlueskyLexicon::UPLOAD_BLOB); if ($uploadToken === null) { return null; } - $name = bin2hex(random_bytes(8)).'.mp4'; - $uploadUrl = "{$videoService}/xrpc/".BlueskyLexicon::VIDEO_UPLOAD - .'?did='.rawurlencode($did).'&name='.rawurlencode($name); + // The transcoder occasionally fails a job transiently + // (JOB_STATE_FAILED "Failed to process video") even for valid input, + // so retry the upload+poll a couple of times before giving up. + for ($attempt = 1; $attempt <= self::VIDEO_UPLOAD_ATTEMPTS; $attempt++) { + $blob = $this->attemptVideoUpload($account, $pds, $videoService, $did, $uploadToken, $tempFile); - $stream = fopen($tempFile, 'r'); + if ($blob !== null) { + return $blob; + } - $response = $this->socialHttp()->withToken($uploadToken) - ->withHeaders(['Content-Type' => 'video/mp4']) - ->withBody($stream, 'video/mp4') - ->post($uploadUrl); - - if (is_resource($stream)) { - fclose($stream); + if ($attempt < self::VIDEO_UPLOAD_ATTEMPTS) { + Log::warning('Bluesky video upload attempt failed, retrying', [ + 'attempt' => $attempt, + 'url' => $url, + ]); + } } - $jobStatus = data_get($response->json(), 'jobStatus'); - - // A re-upload of identical bytes returns 409 with the already - // finished job, whose blob we can embed directly. - if ($response->failed() && $response->status() !== 409) { - Log::error('Bluesky video upload failed', [ - 'status' => $response->status(), - 'body' => $this->redactResponseBody($response->body()), - ]); - - return null; - } - - if (data_get($jobStatus, 'blob')) { - return data_get($jobStatus, 'blob'); - } - - $jobId = data_get($jobStatus, 'jobId'); - - if (! is_string($jobId) || $jobId === '') { - Log::error('Bluesky video upload returned no jobId', [ - 'body' => $this->redactResponseBody($response->body()), - ]); - - return null; - } - - return $this->pollVideoJob($account, $pds, $videoService, $jobId); + return null; } catch (Exception $e) { Log::error('Bluesky video upload exception', [ 'error' => $e->getMessage(), @@ -297,10 +283,72 @@ private function uploadVideo(SocialAccount $account, string $service, string $ur return null; } finally { - @unlink($tempFile); + if (is_string($tempFile)) { + @unlink($tempFile); + } } } + /** + * A single upload-and-poll attempt against the video service. Returns the + * processed blob, or null if this attempt failed (so the caller can retry). + */ + private function attemptVideoUpload(SocialAccount $account, string $pds, string $videoService, string $did, string $uploadToken, string $tempFile): ?array + { + $stream = fopen($tempFile, 'r'); + + if ($stream === false) { + Log::error('Bluesky could not open video file for upload', ['file' => $tempFile]); + + return null; + } + + $name = bin2hex(random_bytes(8)).'.mp4'; + $uploadUrl = "{$videoService}/xrpc/".BlueskyLexicon::VIDEO_UPLOAD + .'?did='.rawurlencode($did).'&name='.rawurlencode($name); + + $response = $this->socialHttp()->withToken($uploadToken) + ->withHeaders(['Content-Type' => 'video/mp4']) + ->withBody($stream, 'video/mp4') + ->post($uploadUrl); + + if (is_resource($stream)) { + fclose($stream); + } + + // uploadVideo returns the jobStatus object directly (top-level), + // while getJobStatus wraps it under a `jobStatus` key — accept both. + $body = $response->json(); + $jobStatus = data_get($body, 'jobStatus') ?: $body; + + // A re-upload of identical bytes returns 409 with the already + // finished job, whose blob we can embed directly. + if ($response->failed() && $response->status() !== 409) { + Log::error('Bluesky video upload failed', [ + 'status' => $response->status(), + 'body' => $this->redactResponseBody($response->body()), + ]); + + return null; + } + + if (data_get($jobStatus, 'blob')) { + return data_get($jobStatus, 'blob'); + } + + $jobId = data_get($jobStatus, 'jobId'); + + if (! is_string($jobId) || $jobId === '') { + Log::error('Bluesky video upload returned no jobId', [ + 'body' => $this->redactResponseBody($response->body()), + ]); + + return null; + } + + return $this->pollVideoJob($account, $pds, $videoService, $jobId); + } + /** * Poll the video service until the transcode job finishes, then return its * blob. Returns null if the job fails or never completes in time. @@ -322,7 +370,20 @@ private function pollVideoJob(SocialAccount $account, string $pds, string $video $response = $this->socialHttp()->withToken($jobToken) ->get($statusUrl, ['jobId' => $jobId]); - $jobStatus = data_get($response->json(), 'jobStatus'); + // Bail on a hard error (e.g. expired/invalid token) instead of + // sleeping to the timeout and masking the real failure. + if ($response->failed()) { + Log::error('Bluesky getJobStatus failed', [ + 'jobId' => $jobId, + 'status' => $response->status(), + 'body' => $this->redactResponseBody($response->body()), + ]); + + return null; + } + + $body = $response->json(); + $jobStatus = data_get($body, 'jobStatus') ?: $body; $state = data_get($jobStatus, 'state'); if ($state === 'JOB_STATE_COMPLETED' && data_get($jobStatus, 'blob')) { @@ -396,8 +457,12 @@ private function resolvePdsEndpoint(SocialAccount $account, string $service): st $directory = (string) config('trypost.platforms.bluesky.plc_directory'); $docUrl = "{$directory}/".rawurlencode($did); } elseif (str_starts_with($did, 'did:web:')) { - $host = substr($did, strlen('did:web:')); - $docUrl = "https://{$host}/.well-known/did.json"; + // Per the did:web spec, colon-separated segments map to a host + // plus optional path; a bare host uses /.well-known/did.json. + $segments = array_map('rawurldecode', explode(':', substr($did, strlen('did:web:')))); + $host = array_shift($segments); + $path = $segments === [] ? '/.well-known/did.json' : '/'.implode('/', $segments).'/did.json'; + $docUrl = "https://{$host}{$path}"; } if ($docUrl !== null) { diff --git a/tests/Feature/Services/Social/BlueskyPublisherTest.php b/tests/Feature/Services/Social/BlueskyPublisherTest.php index 1db84237..53c31945 100644 --- a/tests/Feature/Services/Social/BlueskyPublisherTest.php +++ b/tests/Feature/Services/Social/BlueskyPublisherTest.php @@ -619,7 +619,8 @@ function fakeBlueskyVideoPipeline(string $jobState = 'JOB_STATE_COMPLETED', bool } if (str_contains($url, 'app.bsky.video.uploadVideo')) { - return Http::response(['jobStatus' => ['jobId' => 'job-123', 'state' => 'JOB_STATE_CREATED']], 200); + // uploadVideo returns the jobStatus object directly (not wrapped). + return Http::response(['jobId' => 'job-123', 'did' => 'did:plc:testuser123', 'state' => 'JOB_STATE_CREATED'], 200); } if (str_contains($url, 'app.bsky.video.getJobStatus')) { @@ -721,3 +722,65 @@ function fakeBlueskyVideoPipeline(string $jobState = 'JOB_STATE_COMPLETED', bool && ! isset($request['record']['embed']); }); }); + +test('bluesky publisher retries a transient video transcode failure', function () { + $this->post->update([ + 'media' => [[ + 'id' => 'test-video-id', + 'path' => 'media/2026-01/test-video.mp4', + 'url' => 'https://example.com/media/2026-01/test-video.mp4', + 'mime_type' => 'video/mp4', + 'original_filename' => 'test.mp4', + ]], + ]); + + $jobCalls = 0; + Http::fake(function ($request) use (&$jobCalls) { + $url = $request->url(); + + if (str_contains($url, 'plc.directory')) { + return Http::response(['service' => [[ + 'id' => '#atproto_pds', + 'type' => 'AtprotoPersonalDataServer', + 'serviceEndpoint' => 'https://pds.example.host', + ]]], 200); + } + if (str_contains($url, 'getServiceAuth')) { + return Http::response(['token' => 'service-auth-token'], 200); + } + if (str_contains($url, 'app.bsky.video.uploadVideo')) { + return Http::response(['jobId' => 'job-123', 'state' => 'JOB_STATE_CREATED'], 200); + } + if (str_contains($url, 'app.bsky.video.getJobStatus')) { + $jobCalls++; + // First attempt's job fails transiently; the retry succeeds. + if ($jobCalls === 1) { + return Http::response(['jobStatus' => ['jobId' => 'job-123', 'state' => 'JOB_STATE_FAILED', 'message' => 'transient']], 200); + } + + return Http::response(['jobStatus' => [ + 'jobId' => 'job-123', 'state' => 'JOB_STATE_COMPLETED', + 'blob' => ['$type' => 'blob', 'ref' => ['$link' => 'bafretry456'], 'mimeType' => 'video/mp4', 'size' => 2048], + ]], 200); + } + if (str_contains($url, 'createRecord')) { + return Http::response(['uri' => 'at://did:plc:testuser123/app.bsky.feed.post/3retry', 'cid' => 'bafretry'], 200); + } + + return Http::response(str_repeat('v', 2048), 200); + }); + + $this->publisher->publish($this->postPlatform); + + expect($jobCalls)->toBeGreaterThanOrEqual(2); + Http::assertSent(function ($request) { + if (! str_contains($request->url(), 'createRecord')) { + return false; + } + $embed = $request['record']['embed'] ?? null; + + return $embed + && $embed['$type'] === 'app.bsky.embed.video' + && data_get($embed, 'video.ref.$link') === 'bafretry456'; + }); +});