From c379c4c14f6234bc38da7a58a60b093a19958001 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Tue, 12 May 2026 20:32:20 -0300 Subject: [PATCH 1/3] fix(facebook): use upload_url + file_url for reel transfer phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production was failing every Facebook reel publish with the cryptic 'Video Upload Is Missing' / error_subcode 1363130. Root cause: our 'transfer' phase was making up its own API contract that doesn't exist. The Reels publishing API (per Meta docs) is a 3-step flow: 1. start → POST /{page_id}/video_reels {upload_phase=start} returns {video_id, upload_url} 2. transfer → POST {upload_url} on rupload.facebook.com Header: Authorization: OAuth {token} Body (JSON): {"file_url": "https://..."} 3. finish → POST /{page_id}/video_reels {upload_phase=finish, ...} Our code was doing: 2 (broken) → POST /{video_id} on graph endpoint Body: {video_file_chunk: '', access_token: ...} `video_file_chunk` is not a real parameter; the graph endpoint accepted the request (returning 200) but nothing was actually uploaded, so the finish phase reported 'Video was not uploaded'. Changes: - Capture upload_url from the start response and use it for transfer. - POST to that upload_url with file_url in JSON body and 'Authorization: OAuth ...' header (the format docs.facebook.com documents for the hosted-file flow). - Bail with handleApiError if the start response is missing video_id or upload_url so we don't silently no-op like before. Tests: - Existing 'can publish reel' test updated to include upload_url in the start mock and assert the transfer call actually POSTs to rupload with file_url + OAuth header (would have caught this bug before shipping). - New 'fails reel publish when start does not return upload_url' test for the safety bail-out. - Pre-existing 'cleans up temp files after reel upload' test updated to match the new mock pattern. Full suite green: 1504 passed, 2 skipped, 0 failed. --- app/Services/Social/FacebookPublisher.php | 50 +++++++++++------ .../Services/Social/FacebookPublisherTest.php | 56 +++++++++++++++++-- 2 files changed, 84 insertions(+), 22 deletions(-) diff --git a/app/Services/Social/FacebookPublisher.php b/app/Services/Social/FacebookPublisher.php index 6a612361..64811901 100644 --- a/app/Services/Social/FacebookPublisher.php +++ b/app/Services/Social/FacebookPublisher.php @@ -212,36 +212,54 @@ private function publishVideoPost(string $pageId, string $accessToken, ?string $ private function publishReel(string $pageId, string $accessToken, ?string $content, $media): array { - // Upload video as reel - $response = $this->socialHttp()->post("{$this->baseUrl}/{$pageId}/video_reels", [ + // Phase 1 (start) — graph endpoint returns video_id + upload_url. + $startResponse = $this->socialHttp()->post("{$this->baseUrl}/{$pageId}/video_reels", [ 'upload_phase' => 'start', 'access_token' => $accessToken, ]); - if ($response->failed()) { + if ($startResponse->failed()) { Log::error('Facebook reel upload start failed', [ - 'status' => $response->status(), - 'body' => $this->redactResponseBody($response->body()), + 'status' => $startResponse->status(), + 'body' => $this->redactResponseBody($startResponse->body()), ]); - $this->handleApiError($response); + $this->handleApiError($startResponse); } - $data = $response->json(); - $videoId = data_get($data, 'video_id'); + $startData = $startResponse->json(); + $videoId = data_get($startData, 'video_id'); + $uploadUrl = data_get($startData, 'upload_url'); - // Upload the video file (Facebook accepts URL in video_file_chunk) - $uploadResponse = $this->socialHttp()->post("{$this->baseUrl}/{$videoId}", [ - 'upload_phase' => 'transfer', - 'video_file_chunk' => $media->url, - 'access_token' => $accessToken, - ]); + if (! $videoId || ! $uploadUrl) { + Log::error('Facebook reel start did not return video_id/upload_url', [ + 'body' => $this->redactResponseBody($startResponse->body()), + ]); + $this->handleApiError($startResponse); + } + + // Phase 2 (transfer, hosted-file flow) — POST to upload_url on + // rupload.facebook.com with `file_url` in the JSON body and + // `Authorization: OAuth …` header. The previous implementation + // POSTed to the graph endpoint with a made-up `video_file_chunk` + // body field; Facebook accepted the request but never fetched + // the video, so finish failed with error_subcode 1363130 + // ("Video Upload Is Missing"). + $uploadResponse = $this->socialHttp() + ->withHeaders(['Authorization' => "OAuth {$accessToken}"]) + ->asJson() + ->post($uploadUrl, [ + 'file_url' => $media->url, + ]); if ($uploadResponse->failed()) { - Log::error('Facebook reel upload transfer failed', ['body' => $this->redactResponseBody($uploadResponse->body())]); + Log::error('Facebook reel upload transfer failed', [ + 'status' => $uploadResponse->status(), + 'body' => $this->redactResponseBody($uploadResponse->body()), + ]); $this->handleApiError($uploadResponse); } - // Finish and publish the reel + // Phase 3 (finish) — publish the reel. $finishResponse = $this->socialHttp()->post("{$this->baseUrl}/{$pageId}/video_reels", [ 'upload_phase' => 'finish', 'video_id' => $videoId, diff --git a/tests/Feature/Services/Social/FacebookPublisherTest.php b/tests/Feature/Services/Social/FacebookPublisherTest.php index 3f0a050f..842a47b4 100644 --- a/tests/Feature/Services/Social/FacebookPublisherTest.php +++ b/tests/Feature/Services/Social/FacebookPublisherTest.php @@ -4,6 +4,7 @@ use App\Enums\PostPlatform\ContentType; use App\Enums\SocialAccount\Platform; +use App\Exceptions\Social\FacebookPublishException; use App\Exceptions\TokenExpiredException; use App\Models\Post; use App\Models\PostPlatform; @@ -179,10 +180,12 @@ Http::fake([ '*/page_123/video_reels' => Http::sequence() - ->push(['video_id' => 'reel_video_123'], 200) + ->push([ + 'video_id' => 'reel_video_123', + 'upload_url' => 'https://rupload.facebook.com/video-upload/v25.0/reel_video_123', + ], 200) ->push(['id' => 'reel_123', 'success' => true], 200), - '*/reel_video_123' => Http::response(['success' => true], 200), - '*' => Http::response('', 200), + '*rupload.facebook.com/*' => Http::response(['success' => true], 200), ]); $result = $this->publisher->publish($this->postPlatform); @@ -190,6 +193,45 @@ expect($result)->toHaveKey('id'); expect($result['id'])->toBe('reel_123'); expect($result['url'])->toBe('https://www.facebook.com/reel/reel_123'); + + // Assert the transfer phase: POST to upload_url (rupload host) with + // file_url JSON body and OAuth header — not to the graph endpoint + // with the (legacy/wrong) video_file_chunk body param. + Http::assertSent(function ($request) { + if (! str_contains($request->url(), 'rupload.facebook.com')) { + return false; + } + + return $request['file_url'] === 'https://example.com/media/2026-01/reel.mp4' + && str_starts_with($request->header('Authorization')[0] ?? '', 'OAuth '); + }); +}); + +test('facebook publisher fails reel publish when start does not return upload_url', function () { + $this->postPlatform->update(['content_type' => ContentType::FacebookReel]); + + $this->post->update([ + 'media' => [ + [ + 'id' => 'test-media-reel', + 'path' => 'media/2026-01/reel.mp4', + 'url' => 'https://example.com/media/2026-01/reel.mp4', + 'mime_type' => 'video/mp4', + 'original_filename' => 'reel.mp4', + ], + ], + ]); + + // Missing upload_url in the start response — should not silently + // proceed to a broken transfer (which is what the old code did). + Http::fake([ + '*/page_123/video_reels' => Http::response([ + 'video_id' => 'reel_video_123', + ], 200), + ]); + + expect(fn () => $this->publisher->publish($this->postPlatform)) + ->toThrow(FacebookPublishException::class); }); test('facebook publisher can publish image story', function () { @@ -376,10 +418,12 @@ Http::fake([ '*/page_123/video_reels' => Http::sequence() - ->push(['video_id' => 'reel_video_cleanup_123'], 200) + ->push([ + 'video_id' => 'reel_video_cleanup_123', + 'upload_url' => 'https://rupload.facebook.com/video-upload/v25.0/reel_video_cleanup_123', + ], 200) ->push(['id' => 'reel_cleanup_456', 'success' => true], 200), - '*/reel_video_cleanup_123' => Http::response(['success' => true], 200), - '*' => Http::response('', 200), + '*rupload.facebook.com/*' => Http::response(['success' => true], 200), ]); $this->publisher->publish($this->postPlatform); From 1d5d6ec063e17adf5e06b075aebfb701716e6103 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Tue, 12 May 2026 20:35:54 -0300 Subject: [PATCH 2/3] fix(facebook): switch reel transfer to binary upload with Offset/file_size headers Previous attempt at the hosted-file flow (`file_url` in JSON body) hit the rupload.facebook.com validator with HTTP 400: 'HeaderValuePredicate: Header Offset not convertable to unsigned long' Facebook's rupload endpoint requires the `Offset` and `file_size` headers regardless of whether the upload is local or hosted-file. The docs describe the hosted-file path without them, but in practice rupload rejects requests that lack them. Switching to the well-documented local-file flow: - Download the media to a temp file via Http sink (already a pattern used in XPublisher for media downloads). - POST raw bytes to upload_url with three headers: - Authorization: OAuth {token} - Offset: 0 - file_size: {actual bytes} - Use mime_type from the media item as the Content-Type. - Always cleanup the temp file in a finally block. Tests updated to fake the media-download GET (returning bytes that the publisher then re-uploads) and to assert on the Offset/file_size/ Authorization headers being present and correct. The existing "cleans up temp files after reel upload" test now actually exercises its assertion since this code path creates temp files again. Full suite: 1504 passed, 2 skipped, 0 failed. --- app/Services/Social/FacebookPublisher.php | 56 ++++++++++++------- .../Services/Social/FacebookPublisherTest.php | 11 ++-- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/app/Services/Social/FacebookPublisher.php b/app/Services/Social/FacebookPublisher.php index 64811901..14ecdcb1 100644 --- a/app/Services/Social/FacebookPublisher.php +++ b/app/Services/Social/FacebookPublisher.php @@ -9,6 +9,7 @@ use App\Models\PostPlatform; use App\Services\Social\Concerns\HasSocialHttpClient; use Illuminate\Http\Client\Response; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; class FacebookPublisher @@ -237,26 +238,43 @@ private function publishReel(string $pageId, string $accessToken, ?string $conte $this->handleApiError($startResponse); } - // Phase 2 (transfer, hosted-file flow) — POST to upload_url on - // rupload.facebook.com with `file_url` in the JSON body and - // `Authorization: OAuth …` header. The previous implementation - // POSTed to the graph endpoint with a made-up `video_file_chunk` - // body field; Facebook accepted the request but never fetched - // the video, so finish failed with error_subcode 1363130 - // ("Video Upload Is Missing"). - $uploadResponse = $this->socialHttp() - ->withHeaders(['Authorization' => "OAuth {$accessToken}"]) - ->asJson() - ->post($uploadUrl, [ - 'file_url' => $media->url, - ]); + // Phase 2 (transfer, local-file flow) — download our hosted + // media then POST raw bytes to upload_url with the Offset and + // file_size headers Facebook requires (the docs describe a + // hosted-file shortcut with `file_url` in the body, but rupload + // rejects it with "Header Offset not convertable to unsigned + // long" — the headers are required either way). + $tempFile = tempnam(sys_get_temp_dir(), 'fb_reel_'); - if ($uploadResponse->failed()) { - Log::error('Facebook reel upload transfer failed', [ - 'status' => $uploadResponse->status(), - 'body' => $this->redactResponseBody($uploadResponse->body()), - ]); - $this->handleApiError($uploadResponse); + try { + $download = Http::withOptions(['sink' => $tempFile]) + ->timeout(600) + ->get($media->url); + + if ($download->failed()) { + throw new \Exception('Failed to download reel media: HTTP '.$download->status()); + } + + $fileSize = filesize($tempFile); + + $uploadResponse = Http::withHeaders([ + 'Authorization' => "OAuth {$accessToken}", + 'Offset' => '0', + 'file_size' => (string) $fileSize, + ]) + ->timeout(600) + ->withBody(file_get_contents($tempFile), $media->mime_type ?? 'video/mp4') + ->post($uploadUrl); + + if ($uploadResponse->failed()) { + Log::error('Facebook reel upload transfer failed', [ + 'status' => $uploadResponse->status(), + 'body' => $this->redactResponseBody($uploadResponse->body()), + ]); + $this->handleApiError($uploadResponse); + } + } finally { + @unlink($tempFile); } // Phase 3 (finish) — publish the reel. diff --git a/tests/Feature/Services/Social/FacebookPublisherTest.php b/tests/Feature/Services/Social/FacebookPublisherTest.php index 842a47b4..7f817d68 100644 --- a/tests/Feature/Services/Social/FacebookPublisherTest.php +++ b/tests/Feature/Services/Social/FacebookPublisherTest.php @@ -185,6 +185,7 @@ 'upload_url' => 'https://rupload.facebook.com/video-upload/v25.0/reel_video_123', ], 200) ->push(['id' => 'reel_123', 'success' => true], 200), + '*example.com/media/*' => Http::response('fake-video-binary-content', 200), '*rupload.facebook.com/*' => Http::response(['success' => true], 200), ]); @@ -194,15 +195,16 @@ expect($result['id'])->toBe('reel_123'); expect($result['url'])->toBe('https://www.facebook.com/reel/reel_123'); - // Assert the transfer phase: POST to upload_url (rupload host) with - // file_url JSON body and OAuth header — not to the graph endpoint - // with the (legacy/wrong) video_file_chunk body param. + // Assert the transfer phase: POST raw bytes to upload_url (rupload + // host) with OAuth header and the required Offset + file_size + // headers Facebook's rupload validator demands. Http::assertSent(function ($request) { if (! str_contains($request->url(), 'rupload.facebook.com')) { return false; } - return $request['file_url'] === 'https://example.com/media/2026-01/reel.mp4' + return ($request->header('Offset')[0] ?? null) === '0' + && ($request->header('file_size')[0] ?? null) === (string) strlen('fake-video-binary-content') && str_starts_with($request->header('Authorization')[0] ?? '', 'OAuth '); }); }); @@ -423,6 +425,7 @@ 'upload_url' => 'https://rupload.facebook.com/video-upload/v25.0/reel_video_cleanup_123', ], 200) ->push(['id' => 'reel_cleanup_456', 'success' => true], 200), + '*example.com/media/*' => Http::response('fake-video', 200), '*rupload.facebook.com/*' => Http::response(['success' => true], 200), ]); From ee18b489e62e975229c3083c389374f3c5ddba28 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Tue, 12 May 2026 20:43:32 -0300 Subject: [PATCH 3/3] refactor(facebook): cleanup publishReel after review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five small fixes scoped to the publishReel method: 1. Replace generic \Exception on media download failure with a typed FacebookPublishException(ServerError). The generic exception was landing in the \Throwable catch in PublishToSocialPlatform with category 'unknown', defeating the whole point of the social exception hierarchy. 2. Replace handleApiError($startResponse) with a direct FacebookPublishException throw when video_id/upload_url are missing. The previous code passed a successful HTTP response into a method built for error responses — fromApiResponse would fall into the default arm and surface 'An unknown Facebook error occurred.' ironically reintroducing the same bad UX we just spent the day fixing. 3. Drop the four redundant Log::error calls before handleApiError. FacebookPublishException::fromApiResponse already pulls the FB error code/subcode/message into platformErrorCode + userMessage, and the downstream catch in PublishToSocialPlatform::handle logs the exception anyway (Nightwatch picks that up). Same pattern as the X cleanup in PR #29. 4. Stream the upload body via fopen() resource instead of file_get_contents(). Eliminates loading the whole video into memory for large reels. 5. Replace \@unlink with unlink + Log::warning. Surfaces temp-file cleanup failures instead of silently leaking files. Tests: - Strengthened the missing-upload_url test to assert the exception message and class. - Added a typed-exception test for the media-download failure path (would have caught the regression where we used a generic \Exception). - Full suite green: 1505 passed, 2 skipped, 0 failed. --- app/Services/Social/FacebookPublisher.php | 56 ++++++++++--------- .../Services/Social/FacebookPublisherTest.php | 38 ++++++++++++- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/app/Services/Social/FacebookPublisher.php b/app/Services/Social/FacebookPublisher.php index 14ecdcb1..abeabbf8 100644 --- a/app/Services/Social/FacebookPublisher.php +++ b/app/Services/Social/FacebookPublisher.php @@ -5,6 +5,7 @@ namespace App\Services\Social; use App\Enums\PostPlatform\ContentType; +use App\Exceptions\Social\ErrorCategory; use App\Exceptions\Social\FacebookPublishException; use App\Models\PostPlatform; use App\Services\Social\Concerns\HasSocialHttpClient; @@ -220,10 +221,6 @@ private function publishReel(string $pageId, string $accessToken, ?string $conte ]); if ($startResponse->failed()) { - Log::error('Facebook reel upload start failed', [ - 'status' => $startResponse->status(), - 'body' => $this->redactResponseBody($startResponse->body()), - ]); $this->handleApiError($startResponse); } @@ -232,10 +229,12 @@ private function publishReel(string $pageId, string $accessToken, ?string $conte $uploadUrl = data_get($startData, 'upload_url'); if (! $videoId || ! $uploadUrl) { - Log::error('Facebook reel start did not return video_id/upload_url', [ - 'body' => $this->redactResponseBody($startResponse->body()), - ]); - $this->handleApiError($startResponse); + throw new FacebookPublishException( + userMessage: 'Facebook did not return upload_url for reel start.', + category: ErrorCategory::ServerError, + platformErrorCode: null, + rawResponse: $startResponse->body(), + ); } // Phase 2 (transfer, local-file flow) — download our hosted @@ -252,29 +251,39 @@ private function publishReel(string $pageId, string $accessToken, ?string $conte ->get($media->url); if ($download->failed()) { - throw new \Exception('Failed to download reel media: HTTP '.$download->status()); + throw new FacebookPublishException( + userMessage: 'Could not download media for Facebook reel.', + category: ErrorCategory::ServerError, + platformErrorCode: (string) $download->status(), + rawResponse: null, + ); } $fileSize = filesize($tempFile); + $stream = fopen($tempFile, 'rb'); - $uploadResponse = Http::withHeaders([ - 'Authorization' => "OAuth {$accessToken}", - 'Offset' => '0', - 'file_size' => (string) $fileSize, - ]) - ->timeout(600) - ->withBody(file_get_contents($tempFile), $media->mime_type ?? 'video/mp4') - ->post($uploadUrl); + try { + $uploadResponse = Http::withHeaders([ + 'Authorization' => "OAuth {$accessToken}", + 'Offset' => '0', + 'file_size' => (string) $fileSize, + ]) + ->timeout(600) + ->withBody($stream, $media->mime_type ?? 'video/mp4') + ->post($uploadUrl); + } finally { + if (is_resource($stream)) { + fclose($stream); + } + } if ($uploadResponse->failed()) { - Log::error('Facebook reel upload transfer failed', [ - 'status' => $uploadResponse->status(), - 'body' => $this->redactResponseBody($uploadResponse->body()), - ]); $this->handleApiError($uploadResponse); } } finally { - @unlink($tempFile); + if (! unlink($tempFile)) { + Log::warning('Facebook reel temp file cleanup failed', ['path' => $tempFile]); + } } // Phase 3 (finish) — publish the reel. @@ -287,9 +296,6 @@ private function publishReel(string $pageId, string $accessToken, ?string $conte ]); if ($finishResponse->failed()) { - Log::error('Facebook reel finish failed', [ - 'body' => $this->redactResponseBody($finishResponse->body()), - ]); $this->handleApiError($finishResponse); } diff --git a/tests/Feature/Services/Social/FacebookPublisherTest.php b/tests/Feature/Services/Social/FacebookPublisherTest.php index 7f817d68..8e039512 100644 --- a/tests/Feature/Services/Social/FacebookPublisherTest.php +++ b/tests/Feature/Services/Social/FacebookPublisherTest.php @@ -233,7 +233,43 @@ ]); expect(fn () => $this->publisher->publish($this->postPlatform)) - ->toThrow(FacebookPublishException::class); + ->toThrow( + FacebookPublishException::class, + 'Facebook did not return upload_url for reel start.' + ); +}); + +test('facebook publisher fails reel publish with typed exception when media download fails', function () { + $this->postPlatform->update(['content_type' => ContentType::FacebookReel]); + + $this->post->update([ + 'media' => [ + [ + 'id' => 'test-media-reel', + 'path' => 'media/2026-01/reel.mp4', + 'url' => 'https://example.com/media/2026-01/reel.mp4', + 'mime_type' => 'video/mp4', + 'original_filename' => 'reel.mp4', + ], + ], + ]); + + // start succeeds, but the media URL returns 404 — should surface as + // a typed FacebookPublishException (ServerError) instead of leaking + // a generic Exception that would land in the 'unknown' bucket. + Http::fake([ + '*/page_123/video_reels' => Http::response([ + 'video_id' => 'reel_video_123', + 'upload_url' => 'https://rupload.facebook.com/video-upload/v25.0/reel_video_123', + ], 200), + '*example.com/media/*' => Http::response('', 404), + ]); + + expect(fn () => $this->publisher->publish($this->postPlatform)) + ->toThrow( + FacebookPublishException::class, + 'Could not download media for Facebook reel.' + ); }); test('facebook publisher can publish image story', function () {