fix(social): unify token-expired handling across all publishers#29
Merged
Conversation
Three related fixes for the failure mode where a scheduled post errors out as 'An unknown X error occurred.' when a social account's refresh_token was already invalidated by the provider: 1. **PublishToSocialPlatform**: fail-fast when account status is `TokenExpired`. Previously the job tried to publish, the publisher internally tried to refresh, the provider rejected the rotated refresh_token, and the failure surfaced as a generic 'unknown' error instead of a clear 'reconnect your account' signal. 2. **XPublisher::refreshToken**: when the OAuth endpoint rejects the refresh_token (typically because it was rotated/revoked at X), log the raw response and throw `TokenExpiredException` instead of falling through to `XPublishException::fromApiResponse` which expects the tweet-API response shape (`type`/`title`/`detail`) and treats OAuth-style responses (`error`/`error_description`) as 'Unknown'. 3. **SocialAccount::markAsTokenExpired**: dispatch an in-app + email notification (`Type::AccountDisconnected`) when an account transitions from `Connected` → `TokenExpired`, mirroring the existing pattern in `markAsDisconnected`. Wrapped in a lock to prevent duplicate notifications on concurrent transitions. Accepts an optional `notify: false` so the batch verifier (`VerifyWorkspaceConnections`) can suppress per-account notifications and rely on its summary email.
Extends the X-only fix to LinkedIn, LinkedInPage, Pinterest, Threads, and TikTok. Each publisher's `refreshToken` now throws `TokenExpiredException` directly when the OAuth refresh endpoint rejects the refresh_token, instead of routing through `handleApiError` -> `<Platform>PublishException::fromApiResponse`. Why: `fromApiResponse` is designed for the publish API response shape and only converts certain status codes to `TokenExpiredException` (e.g., 401). OAuth refresh failures typically return 400 with an `error`/ `error_description` (or platform variant) body and were falling into the generic 'Unknown' bucket, masking the real cause and skipping the proper `markAsTokenExpired` + user-notification flow. Dropped the redundant `Log::error` before each `throw`: the downstream catch in `PublishToSocialPlatform::handle` already logs the exception (captured by Nightwatch), and the rich context now lives on the exception itself (`message` = provider's `error_description` / `error.message`, `platformErrorCode` = HTTP status). Tests: one regression-style test per publisher confirming refresh rejection becomes `TokenExpiredException` instead of a generic publish exception.
paulocastellano
changed the title
These tests use Http::fake, model factories, and DB — by Laravel/Pest convention that's a feature test, not a unit test. Moving them to the Feature suite to match the convention. No code changes. Tests still pass: 1493 passed, 2 skipped, 0 failed.
Two follow-up fixes from the code review:
1. **Unified lock key for markAsDisconnected / markAsTokenExpired.**
Both methods now use `social_account_status:{id}` instead of
different keys. Prevents the race where `markAsDisconnected` and
`markAsTokenExpired` could run concurrently on the same account
(publish-time vs verify-batch-time), causing overlapping updates and
duplicate notifications.
2. **i18n for notification title/body in markAsTokenExpired and
markAsDisconnected.** Strings were previously hardcoded in English.
Added `notifications.account_disconnected.{title,body}` and
`notifications.account_token_expired.{title,body}` in en, pt-BR, es.
Follows the project convention (e.g. `Mail/PostPublished`) of
concatenating the `@` prefix in PHP before passing the username to
the translation placeholder, instead of putting `@:account` in the
lang file.
…cted_at preservation, and i18n placeholder substitution Fills gaps surfaced in the code review: - 3 new `markAsDisconnected` tests (mirroring the existing TokenExpired trio). Previously the method had zero coverage despite this PR changing its lock key. - 1 test confirming `markAsTokenExpired($msg, notify: false)` skips notification dispatch (used by VerifyWorkspaceConnections batch path). - 1 test confirming `disconnected_at` is preserved when already set (the `?? now()` branch). - 2 end-to-end tests (one per method) that DO NOT fake the queue, so `SendNotification::handle()` runs synchronously. Asserts the Notification row is actually created in the DB with the correct i18n-substituted title/body, that NotificationCreated event is dispatched, and that AccountDisconnected mail is queued. Catches bugs where the i18n placeholder keys (`:platform`, `:account`) silently fail to substitute.
7 tasks
paulocastellano
added a commit
that referenced
this pull request
May 12, 2026
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.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Scheduled X post failed with the cryptic "An unknown X error occurred." despite the user having recently run
social:refresh-expiring-tokens. The audit revealed the same latent bug pattern in 6 publishers: when the OAuth refresh endpoint rejects the storedrefresh_token, the failure was misclassified as a generic publish error instead of a token-expired event — so the user never got a clear "reconnect your account" signal and the account never transitioned cleanly toStatus::TokenExpired.This PR fixes the entire class of bug, not just X.
The 4 fixes
1.
PublishToSocialPlatform— fail-fast onStatus::TokenExpiredMirrors the existing
Status::Disconnectedguard. When the account is already known to need reconnection, mark the platform as failed withposts.errors.account_token_expired+error_context.category = 'token_expired'and skip the publish attempt entirely. Applies to all platforms (status-based, not platform-specific).2. Publishers'
refreshToken→TokenExpiredException(XPublisher + 5 others)For every publisher with its own OAuth refresh logic (
X,LinkedIn,LinkedInPage,Pinterest,Threads,TikTok), when the refresh endpoint returns a failure response, throwTokenExpiredExceptiondirectly with:message: extracted from the provider'serror_description/error.messageplatformErrorCode: the HTTP statusThis bypasses each
<Platform>PublishException::fromApiResponse, which was designed for the publish-API response shape (type/title/detail, status 401 detection) and didn't reliably classify OAuth-format refresh failures (status 400 +error/error_description).The redundant
Log::errorcalls were dropped — the downstream catch inPublishToSocialPlatform::handlealready logs the exception (and Nightwatch captures it), and the actionable info now lives on the exception itself.InstagramandYouTubewere already doing this correctly; no changes there.3.
SocialAccount::markAsTokenExpired— notify the userMirrors the pattern in
markAsDisconnected:Connected→TokenExpired(no spam on subsequent failures)SendNotification(in-app + email) withType::AccountDisconnectedOptional
bool $notify = trueparameter letsVerifyWorkspaceConnectionssuppress per-account notifications when it's about to send a batched summary email.4. i18n
New key
posts.errors.account_token_expiredin en / pt-BR / es.Test plan
php artisan test --compact --parallel— 1493 passed, 2 skipped, 0 failed (+5 over main, all new)publish to social platform skips publishing when account token is expiredx publisher throws TokenExpiredException when refresh_token is rejected by Xlinkedin publisher throws TokenExpiredException when refresh_token is rejectedlinkedin page publisher throws TokenExpiredException when refresh_token is rejectedpinterest publisher throws TokenExpiredException when refresh_token is rejectedthreads publisher throws TokenExpiredException when refresh_token is rejectedtiktok publisher throws TokenExpiredException when refresh_token is rejectedSocialAccount::markAsTokenExpirednotification behaviorpublish to social platform attempts publishing when account token is expired(asserted the buggy behavior)posts.errors.account_token_expiredshows up (not "unknown")markAsTokenExpired→ confirm in-app notification + email arrives once per transition