Skip to content

fix(platform): return retry guidance for rate-limited video ingest - #4194

Open
yannickmonney wants to merge 1 commit into
mainfrom
fix/video-ingest-rate-limit
Open

yannickmonney wants to merge 1 commit into
mainfrom
fix/video-ingest-rate-limit

Conversation

@yannickmonney

@yannickmonney yannickmonney commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Exhausting the organization's shared upload allowance returns HTTP 429 with Retry-After and structured retry metadata. The video caller now shows localized EN/DE/FR guidance with the rounded-up delay and stops the remaining links in that paste after one quota toast. Earlier accepted videos stay accepted, and unrelated refusals still allow later links.

The route retains the member, body, thread and project guards. Regressions cover a foreign thread before any allowance charge, expected refusals without fault logging, and a real limiter fault with fault logging. The rebased fixtures match main's membership and budget readers.

Closes #3932.

Verification on the prepared source:

  • 252 focused server, locale and guard tests; 18 UI tests.
  • Five real Chromium cases using the existing UI regressions: EN/DE/FR/de-CH quota guidance and an earlier successful link. The browser adapter runs real axe-core with WCAG 2.1 AA and contrast enabled.
  • Full platform typecheck, type-aware lint, scoped SAST, conflict and manual-register checks pass; independent source review accepted.
  • No live PostgreSQL concurrency or external video-provider claim.

The full monorepo check did not pass: it stopped with 45 of 52 tasks successful. Its daemon, sandbox and connector timing failures pass separately; main also has a repeatable REST error-code classification failure and unused exports being repaired in #4625. Native CI and queue checks remain required.

Current-main rebase

Replayed the previously accepted source 1f27135ee85e53bf698e9a159f5f53a1bcf13204 onto main d1373d84cd56972501403f62145ec52e6f65d44a, including the merged shared CI repair in #4625. The accepted feature payload and all current-main changes are preserved in one atomic commit. Configured commit and conflict checks pass; earlier behavioral proof remains recorded above. All seven native required checks and full merge-group validation remain required for this new source.

Maintenance replay: preserves the accepted feature payload on current main 7d178ca. Includes the merged #4649 Knip cleanup and the exact independently accepted one-line shared CLI inventory repair from #4654 (252f0df), which is still pending native merge on main. The fixed suite inventory keeps its discovery and source/compiled phase guards. Existing feature proof is retained; no fresh full-feature/full-workspace test or hosted-green claim. Native required checks remain mandatory.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

Independent review of PR #4194 at 215b8cad49963c6d7b3730f60d802cedfa564a2c: changes required (one MEDIUM). The head also needs a rebase.

Runtime (mine)

  • Negative control: with main's video_links/routes.ts, the head's routes.rate-limit.test.ts fails 1 of 6 (the 429 mapping). At the head it passes 6/6.
  • CI at this head was terminal and green: 51 success, 11 skipped, Trivy neutral, and Backend integration 1517/1517.

MEDIUM: the retry guidance never reaches the app, the door's only caller, and its toast regresses to a raw code.

  • The response: a 429 now answers {error: 'RATE_LIMITED', code: 'RATE_LIMITED', data: {retryAfterMs}}.
  • What the app shows:
    • use-chat-video-links.ts:92-97 looks up videoLink.errors.${code}, and there is no videoLink.errors.RATE_LIMITED key in EN, DE or FR. The existing rateLimited key means the video platform itself is throttling.
    • It then falls back to failureDetail. Because the message equals the code, the reviewer traced the toast to "Couldn't add this video" with the description RATE_LIMITED, untranslated, and no wait time.
    • Before this PR, the 500 gave the translated generic line.
    • The paste loop doesn't stop after the 429, so one multi-link paste can raise up to three identical toasts.
  • What the PR body says: "no … locale, UI … change is needed".
  • Fix: add videoLink.errors.RATE_LIMITED in EN/DE/FR, ideally with the seconds from data.retryAfterMs. There is precedent in projects.errors.RATE_LIMITED and map-upload-error.ts.
  • Test: in use-chat-video-links.test.tsx, a pasted link answered with that 429 shows the translated sentence, never the bare code, and gives one toast.

Merge gate: the head now conflicts with main 403a9a537 in tests/manual/reference/automation.md:17, where #4197's row went in at the same place. Rebuild on main keeping both rows, below the table's delimiter line.

LOW

  1. The guard order isn't pinned by a test. The code does check the thread (routes.ts:79-86) before charging (:87), but the fixture's thread lookup always succeeds. Moving the charge first passes all six tests, and would then answer a foreign thread 429 instead of 404. Test: a foreign thread with an exhausted limiter expects 404 and no charge.
  2. Fault reporting isn't asserted. console.error is mocked but never checked. Assert it is not called for the 429 and is called for the synthetic fault.
  3. INFO:
    • The door's other three 429s (in-flight cap, budget, retry cooldown) carry no Retry-After and no code.
    • The charge runs before the playlist, dedupe and budget checks.
    • The file:upload key is shared with document uploads.
    • /retry charges no rate limit, although its docstring says it does.
    • The register row is filed under [chat], where other video-link rows use [video-links].

Verified correct

  • Only RateLimitExceededError maps to 429. Domain errors keep their status, and everything else goes to the shared handler as 503 or a reported 500.
  • The body leaks nothing: it carries a fixed code plus the retry delay, not the rule name.
  • Retry-After is well formed: delta-seconds, rounded up, at least 1, bounded to 1–60 for this window. A refused charge doesn't extend the window.
  • The shape matches the sibling app doors, and this is the only door that creates video jobs.

Next: I'll re-review the repaired, rebased head and require green exact-head CI.

@yannickmonney
yannickmonney force-pushed the fix/video-ingest-rate-limit branch from 215b8ca to b42b443 Compare October 9, 2026 02:40
@yannickmonney
yannickmonney disabled auto-merge October 9, 2026 02:55
@yannickmonney
yannickmonney force-pushed the fix/video-ingest-rate-limit branch from b42b443 to 1f27135 Compare October 9, 2026 06:56
@yannickmonney
yannickmonney force-pushed the fix/video-ingest-rate-limit branch 2 times, most recently from 5f6d5a6 to 2467997 Compare October 9, 2026 14:59
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026
@yannickmonney
yannickmonney force-pushed the fix/video-ingest-rate-limit branch from 2467997 to bb3e4cb Compare October 9, 2026 23:49
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@yannickmonney
yannickmonney removed this pull request from the merge queue due to a manual request Oct 10, 2026
@yannickmonney
yannickmonney force-pushed the fix/video-ingest-rate-limit branch from bb3e4cb to a19b4b0 Compare October 10, 2026 09:41
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: rate-limited video ingest returns 500 without retry guidance

1 participant