Skip to content

Content: Fix unhandled rejection when scheduled publish fails - #4327

Open
github-actions[bot] wants to merge 4 commits into
devfrom
fix/4326-fix-schedule-publish-unhandled-rejection
Open

github-actions[bot] wants to merge 4 commits into
devfrom
fix/4326-fix-schedule-publish-unhandled-rejection

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Resolves #4326

Severity: low

Summary

  • MANAGER-UI-3EA: scheduling a publish that fails (e.g. the session went invalid mid-request) threw an unhandled promise rejection from SchedulePublish, even though the user already saw a failure notification.
  • dispatch(publish(...)) in src/shell/components/SchedulePublish/index.tsx only chained .finally(), and publish() in src/shell/store/content.js re-throws after notifying (line 908), so the rejection had nowhere to land. Added a .catch(() => {}) after the .finally() to swallow the already-handled error.

Classification

  • Root-cause certainty: PASS. The Sentry event's lastAction/notification text ("Error scheduling... version 20") matches exactly the notify+rethrow in publish()'s catch block (src/shell/store/content.js:898-909), and the only caller using .finally() without a .catch() is SchedulePublish/index.tsx:91-109. One clear fix, no design judgment involved.
  • Blast radius: PASS. The change is confined to src/shell/components/SchedulePublish/index.tsx — not auth.js, not permissions, not IndexedDB warm-cache hydration, not RTK Query base config, not webpack, not workflows.
  • Verifiable by inspection: PASS. Adding an empty .catch() after an existing .finally() doesn't change any notification or control flow the user sees; it only prevents the already-handled rejection from surfacing as unhandled.
  • Cohesion, not count: PASS. Single file touched, the exact call site causing the bug.

Test plan

  • With a Chrome devtools network override (or by expiring the session cookie mid-request), trigger a scheduled publish that returns a 4xx from the publishings endpoint and confirm the failure notification still appears with no "Uncaught (in promise)" console error.
  • Schedule a publish that succeeds and confirm onScheduleSuccess, loading state, and dialog close behavior are unchanged.

@github-actions github-actions Bot added the sentry-auto-fix PR auto-generated by the Sentry Handler workflow's auto-fix path label Sep 14, 2026
Comment thread src/shell/components/SchedulePublish/index.tsx Outdated
@github-actions

Copy link
Copy Markdown
Contributor Author

Code Review — 🔴 1 blocker(s) — see inline comments

@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Acceptance Criteria QA — ✅ PASS

Validates #4326: Error: invalid session

  1. ✅ Failure from the schedule-publish request no longer escapes as an unhandled promise rejection — publish() in src/shell/store/content.js rethrows after notifying (throw err at the end of its .catch), and the new .catch(() => {}) in SchedulePublish/index.tsx absorbs that rethrow at the call site.
  2. ✅ User still gets error feedback on a failed schedule — the added catch's comment relies on publish()'s own notify() dispatch, which fires before the rethrow, so no user-facing feedback is lost.
  3. ⚠️ Dialog no longer auto-closes on a failed schedule attempt (previously onClose() ran unconditionally in .finally, now it only runs in .then on success) — this looks like an intentional improvement but isn't confirmable as an explicit requirement since the issue body is just a bare Sentry stack trace with no described acceptance criteria.
Suggested Cypress coverage

cypress/e2e/content/actions.spec.js already covers the happy-path "Schedules a Publish for an item" flow (around line 278). Add a case that intercepts the POST .../publishings request and forces a 4xx/5xx or error-body response, then asserts: the error notification appears, the SchedulePublishModal stays open (rather than closing as it did before this fix), and the schedule button's loading state clears afterward so the user can retry. This would lock in the new .then/.catch/.finally split and guard against the unhandled-rejection regression.

@github-actions

Copy link
Copy Markdown
Contributor Author

Code Review — ✅ No blockers

@github-actions

Copy link
Copy Markdown
Contributor Author

Localization Reviewer — 🔴 2 blocker(s) — see inline comments

🔴 Blocking

  • src/shell/components/SchedulePublish/index.tsx:111 — TS2578: Unused '@ts-expect-error' directive.
  • src/shell/components/SchedulePublish/index.tsx:113 — TS2339: Property 'finally' does not exist on type '(dispatch: any, getState: any) => Promise'.

🟡 Advisory

None

Comment thread src/shell/components/SchedulePublish/index.tsx
Comment thread src/shell/components/SchedulePublish/index.tsx Outdated
finnar-bin and others added 2 commits September 18, 2026 10:50
Move onScheduleSuccess/onClose into .then() so they only run when
publish() resolves; .finally() now only clears loading state. Also
fixes the @ts-expect-error directive, which was suppressing the
closing paren line instead of the .then() line it was meant to cover.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor Author

Code Review — ✅ No blockers

@github-actions

Copy link
Copy Markdown
Contributor Author

Localization Reviewer — 🔴 2 blocker(s) — see inline comments

🔴 Blocking

  • src/shell/components/SchedulePublish/index.tsx:111 — TS2578: Unused '@ts-expect-error' directive.
  • src/shell/components/SchedulePublish/index.tsx:113 — TS2339: Property 'then' does not exist on type '(dispatch: any, getState: any) => Promise'.

🟡 Advisory

None

@github-actions

Copy link
Copy Markdown
Contributor Author

Adversarial Browser QA

🔴 PR's own diff introduces a TypeScript compile error that blocks clicks across the whole app

Type: regression from this PR
Steps:

  1. Navigate to http://8-acabf6a8d6-bj9tr2.manager.dev.zesty.io:8080/launchpad (or any route — the webpack dev overlay fires on initial bundle compile, independent of route).
  2. Observe the webpack-dev-server error overlay rendered in an <iframe> on top of the app: "Compiled with problems:".
  3. It lists two errors, both in the file this PR modifies, src/shell/components/SchedulePublish/index.tsx:
    • TS2578: Unused '@ts-expect-error' directive. at index.tsx(111,7)
    • TS2339: Property 'then' does not exist on type '(dispatch: any, getState: any) => Promise<any>'. at index.tsx(113,8)
  4. Navigate to /content/6-bab6b795f8-fbbz95 and, while the overlay is still up, click any content-item row in the list.
  5. Reloaded twice and repeated the click-through-overlay attempt twice — same two tsc errors and same blocked click both times.

Expected: The PR's change from .finally() to .then().catch().finally() should type-check; the pre-existing // @ts-expect-error untyped action comment (placed on the line before the outer dispatch(...) closing paren) should still suppress whatever untyped-dispatch error it was originally covering, and no overlay should appear.
Actual: Restructuring the promise chain moved the type error's location. The untyped dispatch(publish(...)) return type no longer errors where the @ts-expect-error comment sits (making the directive itself invalid per TS2578), while the new .then(...) call one line below is now the thing that doesn't type-check (TS2339, since the outer type has no .then member). This is a genuine tsc error introduced by the PR in the exact file it touches — CLAUDE.md calls this out directly: CI has no typecheck gate for this repo, so nothing but a manual npx tsc --noEmit catches it, and the PR does not appear to have run it. It's not just visual noise: the overlay <iframe> sits on top of the whole app and intercepts pointer events, so clicking anywhere on the page (e.g. a content-list row) fails outright until the overlay's own "Dismiss" button is clicked — reproduced directly, a page.click() on a content-list row retried for 5s and failed with "<iframe id=\"webpack-dev-server-client-overlay\">... intercepts pointer events" before timing out. Because this is a tsl (ts-loader) type-check error rather than a syntax error, webpack still emits a working bundle underneath, so the app isn't permanently broken — but the overlay reappears on every hot-reload while this branch is checked out, repeatedly blocking clicks anywhere on the page until manually dismissed.
Console/network: overlay text is the finding itself; separately confirmed it blocks a page.click() on an unrelated content-list row ("intercepts pointer events" in the Playwright retry log).
TypeScript compile error overlay caused by SchedulePublish's promise chain restructure

🟡 Unschedule Publish dialog closes as if successful even when the unschedule request fails

Type: pre-existing on the touched surface
Steps:

  1. Navigate to http://8-acabf6a8d6-bj9tr2.manager.dev.zesty.io:8080/content/6-bab6b795f8-fbbz95/7-c0cf88d6b3-g6zs8b ("No Relations" item) and schedule a publish via the PublishMenuButton caret → PublishScheduleButtonSchedulePublishButton, confirming the item now shows the "Scheduled" badge.
  2. In-page, monkey-patch window.fetch so any DELETE .../publishings/:id resolves with HTTP 500 instead of hitting the real API (simulates a failed unschedule without touching other traffic).
  3. Reopen the publish menu → click PublishScheduleButton again (now labeled "Unschedule Publish") → click UnschedulePublishButton in the dialog.
  4. Dialog closes immediately with no error shown, exactly as it would on success. The "Scheduled" badge remains on the item header (the server correctly kept the schedule since the DELETE was rejected), so the UI and actual state diverge for as long as the user trusts the dialog closing.
  5. Repeated steps 3–4 a second time on the same item — same result both times: dialog closes silently on the forced 500.

Expected: Given this PR's own reasoning for handleSchedulePublish — success callbacks should only fire in .then(), not unconditionally in .finally() — the sibling handleUnschedulePublish (src/shell/components/SchedulePublish/index.tsx:125-141) should show the same care: on failure, the dialog should stay open (or otherwise surface an error) rather than closing as if the unschedule succeeded.
Actual: handleUnschedulePublish still does dispatch(unpublish(...)).finally(() => { setIsLoading(false); onClose(); onUnscheduleSuccess?.(); })onClose() and onUnscheduleSuccess?.() run unconditionally. This works out in the UI today only because unpublish()'s own .catch() (src/shell/store/content.js:967-979) swallows the error and does not rethrow, so the returned promise always resolves — but that means the exact bug this PR fixes for scheduling is still live for unscheduling, one function below it in the same file. If onUnscheduleSuccess is wired to open a follow-up "Confirm Publish" step (as it is from the item header's "Publish Now" action on a scheduled item, ItemEditHeaderActions.tsx:814-817), a failed unschedule would proceed straight into a publish-confirmation flow for a version that was never actually unscheduled.
Console/network: no console errors or uncaught rejections from this sequence; the forced 500 is silent to the user beyond the dialog closing. (The in-page fetch patch is test-only instrumentation, not a product bug in itself — the product bug is that the UI has no visible reaction to the failure at all.)
Unschedule Publish dialog closed after a forced 500 on the DELETE call, item still shows Scheduled

Also checked and working correctly
  • Rapid double-submit on the "Schedule Publish" confirm button (SchedulePublishButton): only one POST .../publishings request fired; no duplicate schedule created.
  • Successful schedule-publish flow (item goes from draft → Scheduled) and the dialog closing correctly on success, per this PR's fix.
  • Opened the "QA Negative Test" model's content list and an item containing <b>bold</b> & <script>x</script> in a text field — renders as literal text in the grid, no script execution or broken layout observed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sentry-auto-fix PR auto-generated by the Sentry Handler workflow's auto-fix path

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: invalid session

1 participant