lvl0/fedi-feed-router

Fork 0

A failed publish silently retries instead of returning the article to the list #142

New issue

Closed

opened 2026-08-13 21:13:52 +02:00 by myrmidex · 2 comments

myrmidex commented

2026-08-13 21:13:52 +02:00

Owner

Problem

When publishing an approved article fails, the article vanishes from the UI and is retried in the background with no visible indication. The user approved it, it never appeared on the platform, and nothing in the interface says why or when it will be tried again.

Observed 2026-08-13: publishing failed with couldnt_find_community (stale channel id). The article left the pending list and the only trace was a logs row.

Current behaviour

PublishRouteArticleAction sets publish_status = ERROR and calls RouteArticle::recordPublishAttemptFailed(), which schedules a retry:

RETRY_BACKOFF_MINUTES = [5, 30, 120, 360]
MAX_PUBLISH_ATTEMPTS = 4

The Articles page's pending tab filters on approval_status = PENDING. A failed publish leaves approval_status = APPROVED and only changes publish_status, so the article is no longer pending and no longer shown. After four failed attempts it stops retrying and sits in ERROR indefinitely, still invisible.

Desired behaviour

A failed publish should put the article back in front of the user rather than disappearing into a background retry.

Open questions — resolved

Does automatic retry go away entirely, or stay as a backstop? → Removed entirely. A failure now waits for the user.
If the article returns to pending, what happens to approval_status? → It does not return to pending. approval_status and decided_at are never touched, so #83's approval-rate chart is unaffected. Failed articles surface in a dedicated tab instead.
What about the articles already sitting in ERROR? → Moot in practice: there were none left by implementation time (CleanupArticlesJob had pruned them). Any that do exist surface on the Failed tab with no reason text, since there is nothing to backfill from.

Acceptance criteria

A publish failure leaves the article visible and actionable in the UI
The failure reason is shown to the user, not only written to logs
Retry behaviour decided: removed, replaced by a manual per-row Retry action
The approval-rate stat (#83) is not distorted — approval_status/decided_at are never written on failure or retry, pinned by two tests
Existing publish_status = ERROR rows are surfaced rather than stranded — they appear on the Failed tab, though rows predating the migration show no reason text
Feature tests cover a failed publish returning the article to the user

Notes

PublishRouteArticleAction writes ERROR at three sites; all now route through recordPublishFailed().
Related: #131 "Detect and notify on failed queue jobs that have sat unretried" covers the queue-level version of this blind spot; this ticket is about the article-level UX.

## Problem When publishing an approved article fails, the article vanishes from the UI and is retried in the background with no visible indication. The user approved it, it never appeared on the platform, and nothing in the interface says why or when it will be tried again. Observed 2026-08-13: publishing failed with `couldnt_find_community` (stale channel id). The article left the pending list and the only trace was a `logs` row. ## Current behaviour `PublishRouteArticleAction` sets `publish_status = ERROR` and calls `RouteArticle::recordPublishAttemptFailed()`, which schedules a retry: - `RETRY_BACKOFF_MINUTES = [5, 30, 120, 360]` - `MAX_PUBLISH_ATTEMPTS = 4` The Articles page's pending tab filters on `approval_status = PENDING`. A failed publish leaves `approval_status = APPROVED` and only changes `publish_status`, so the article is no longer pending and no longer shown. After four failed attempts it stops retrying and sits in `ERROR` indefinitely, still invisible. ## Desired behaviour A failed publish should put the article back in front of the user rather than disappearing into a background retry. ## Open questions — resolved 1. **Does automatic retry go away entirely, or stay as a backstop?** → **Removed entirely.** A failure now waits for the user. 2. **If the article returns to pending, what happens to `approval_status`?** → **It does not return to pending.** `approval_status` and `decided_at` are never touched, so #83's approval-rate chart is unaffected. Failed articles surface in a dedicated tab instead. 3. **What about the articles already sitting in `ERROR`?** → Moot in practice: there were none left by implementation time (`CleanupArticlesJob` had pruned them). Any that do exist surface on the Failed tab with no reason text, since there is nothing to backfill from. ## Acceptance criteria - [x] A publish failure leaves the article visible and actionable in the UI - [x] The failure reason is shown to the user, not only written to `logs` - [x] Retry behaviour decided: **removed**, replaced by a manual per-row Retry action - [x] The approval-rate stat (#83) is not distorted — `approval_status`/`decided_at` are never written on failure or retry, pinned by two tests - [x] Existing `publish_status = ERROR` rows are surfaced rather than stranded — they appear on the Failed tab, though rows predating the migration show no reason text - [x] Feature tests cover a failed publish returning the article to the user ## Notes - `PublishRouteArticleAction` writes `ERROR` at three sites; all now route through `recordPublishFailed()`. - Related: #131 "Detect and notify on failed queue jobs that have sat unretried" covers the queue-level version of this blind spot; this ticket is about the article-level UX.

myrmidex added this to the v1.4.0 milestone 2026-08-13 21:13:52 +02:00

myrmidex added the

bug

label 2026-08-13 21:13:52 +02:00

myrmidex commented

2026-08-13 21:44:59 +02:00

Author

Owner

Delivered in c7b9cd6.

What shipped

route_articles.publish_error (nullable text) stores the failure reason. Migration 2024_01_01_000022 also drops publish_attempts and next_attempt_at.
Automatic retry removed. RETRY_BACKOFF_MINUTES, MAX_PUBLISH_ATTEMPTS, recordPublishAttemptFailed(), clearPublishAttempts() and hasExhaustedPublishAttempts() are gone, replaced by recordPublishFailed(string $reason) / clearPublishFailure().
A Failed tab on the Articles page with a red count badge, the failure reason shown on each card, and a per-row Retry action.

The part that needed care

scopeDueForPublishing() previously gated on attempt count and backoff time. It now gates on publish_status != ERROR, and that exclusion is load-bearing — it is not incidental to removing the backoff.

PublishNextArticleJob publishes one article per five-minute tick, taking the oldest approved-but-unpublished candidate. Remove the backoff without an exclusion and a permanently-failing article becomes the oldest candidate on every tick forever: retried constantly, and blocking every article behind it. PublishNextArticleJobTest::test_handle_publishes_a_later_article_when_the_oldest_has_failed pins that an older failed article is skipped and the next one publishes.

What was deliberately not touched

approval_status and decided_at are never written on failure or retry. Returning a failed article to PENDING would have been the obvious implementation, but it would clear decided_at — which #83's approval-rate chart buckets on — and retroactively change a past day's percentage. Two tests pin the invariant: RouteArticleRetryTest::test_a_failure_does_not_change_the_approval_decision and ArticlesTest::test_retrying_keeps_the_approval_decision_intact.

Safety

A manual retry cannot double-post. ArticlePublication rows are created only after a successful platform post, and the job's whereDoesntHave('article.articlePublications', …) guard keys off those rows rather than publish_status; there is a second check under a cache lock in ArticlePublishingService. retryPublish() scopes through RouteArticle::failed()->find() rather than findOrFail, so a replayed request or an id in any other state silently no-ops.

Verification

1196 tests / 2898 assertions green, Pint clean (341 files), PHPStan clean. code-reviewer (pre-commit) and pr-reviewer (full diff) both returned no critical/must-fix issues. Migration applied to dev and both new scopes confirmed executing against the real schema.

Not verified in a browser — dev currently has no failed articles, so the tab renders its empty state.

Left open deliberately

NotificationTypeEnum::PUBLISH_FAILED has no dedup, unlike FEED_STALE/FEED_EMPTY. Before this change one failure could fire up to four notifications; now it fires one, which is strictly better — but a user repeatedly retrying a permanently-broken channel gets one each time. Out of scope here; worth its own ticket if it becomes noticeable.
publish_status is not indexed. Pre-existing, but more load-bearing now that it filters every scheduler tick and every Articles page render. Premature at current table size.

Delivered in `c7b9cd6`. ## What shipped - **`route_articles.publish_error`** (nullable text) stores the failure reason. Migration `2024_01_01_000022` also drops `publish_attempts` and `next_attempt_at`. - **Automatic retry removed.** `RETRY_BACKOFF_MINUTES`, `MAX_PUBLISH_ATTEMPTS`, `recordPublishAttemptFailed()`, `clearPublishAttempts()` and `hasExhaustedPublishAttempts()` are gone, replaced by `recordPublishFailed(string $reason)` / `clearPublishFailure()`. - **A Failed tab** on the Articles page with a red count badge, the failure reason shown on each card, and a per-row Retry action. ## The part that needed care `scopeDueForPublishing()` previously gated on attempt count and backoff time. It now gates on `publish_status != ERROR`, and **that exclusion is load-bearing** — it is not incidental to removing the backoff. `PublishNextArticleJob` publishes one article per five-minute tick, taking the **oldest** approved-but-unpublished candidate. Remove the backoff without an exclusion and a permanently-failing article becomes the oldest candidate on every tick forever: retried constantly, and blocking every article behind it. `PublishNextArticleJobTest::test_handle_publishes_a_later_article_when_the_oldest_has_failed` pins that an older failed article is skipped and the next one publishes. ## What was deliberately not touched `approval_status` and `decided_at` are never written on failure or retry. Returning a failed article to PENDING would have been the obvious implementation, but it would clear `decided_at` — which #83's approval-rate chart buckets on — and retroactively change a past day's percentage. Two tests pin the invariant: `RouteArticleRetryTest::test_a_failure_does_not_change_the_approval_decision` and `ArticlesTest::test_retrying_keeps_the_approval_decision_intact`. ## Safety A manual retry cannot double-post. `ArticlePublication` rows are created only after a successful platform post, and the job's `whereDoesntHave('article.articlePublications', …)` guard keys off those rows rather than `publish_status`; there is a second check under a cache lock in `ArticlePublishingService`. `retryPublish()` scopes through `RouteArticle::failed()->find()` rather than `findOrFail`, so a replayed request or an id in any other state silently no-ops. ## Verification 1196 tests / 2898 assertions green, Pint clean (341 files), PHPStan clean. `code-reviewer` (pre-commit) and `pr-reviewer` (full diff) both returned no critical/must-fix issues. Migration applied to dev and both new scopes confirmed executing against the real schema. **Not verified in a browser** — dev currently has no failed articles, so the tab renders its empty state. ## Left open deliberately - **`NotificationTypeEnum::PUBLISH_FAILED` has no dedup**, unlike `FEED_STALE`/`FEED_EMPTY`. Before this change one failure could fire up to four notifications; now it fires one, which is strictly better — but a user repeatedly retrying a permanently-broken channel gets one each time. Out of scope here; worth its own ticket if it becomes noticeable. - **`publish_status` is not indexed.** Pre-existing, but more load-bearing now that it filters every scheduler tick and every Articles page render. Premature at current table size.

myrmidex closed this issue

2026-08-13 21:45:05 +02:00

myrmidex commented

2026-08-13 21:46:50 +02:00

Author

Owner

Browser-verified 2026-08-13: the Failed tab renders on /articles and correctly shows its empty state — dev has no articles in publish_status = ERROR.

Updates the "Not verified in a browser" caveat in the previous comment. The populated case (failure reason text, red badge count, Retry button) is still only covered by the feature tests, since reproducing it would mean deliberately breaking a channel's community id.

Browser-verified 2026-08-13: the Failed tab renders on `/articles` and correctly shows its empty state — dev has no articles in `publish_status = ERROR`. Updates the "Not verified in a browser" caveat in the previous comment. The populated case (failure reason text, red badge count, Retry button) is still only covered by the feature tests, since reproducing it would mean deliberately breaking a channel's community id.

myrmidex referenced this issue

2026-08-14 21:59:14 +02:00

Channel health status and a repair action for stale community ids #145

myrmidex referenced this issue

2026-08-15 00:02:58 +02:00

Release v1.4.0 #146

myrmidex referenced this issue

2026-08-15 10:35:50 +02:00

Channel health status and a repair action for stale community ids #145