refactor(facebook): cleanup publishReel after review
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.
This commit is contained in:
parent
1d5d6ec063
commit
ee18b489e6
2 changed files with 68 additions and 26 deletions
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 () {
|
||||
|
|
|
|||
Loading…
Reference in a new issue