Skip to content

Fix oversized upload auto-review crash#292

Merged
ohld merged 2 commits into
productionfrom
fix/upload-file-too-big
May 21, 2026
Merged

Fix oversized upload auto-review crash#292
ohld merged 2 commits into
productionfrom
fix/upload-file-too-big

Conversation

@ohld
Copy link
Copy Markdown
Member

@ohld ohld commented May 21, 2026

Fixes FFM-1369.

What changed

  • Handle Telegram BadRequest failures during the initial uploaded-file download.
  • Classify oversized Telegram download failures as telegram_file_too_big for observability.
  • Mark the meme as broken_content_link and notify the uploader with a localized file-too-big message instead of letting the background auto-review task crash.
  • Added a regression test covering the oversized-file path.

Verification

  • ruff check --fix src/ tests/ && ruff format src/ tests/
  • DATABASE_URL=postgresql+asyncpg://app:app@app_db:5432/app REDIS_URL=redis://:myStrongPassword@redis:6379 ENVIRONMENT=TESTING TELEGRAM_BOT_TOKEN=123456:ABCDEFabcdef1234567890 pytest tests/tgbot/test_upload_observability.py -q

Staff Engineer: please review before merge.

@ohld ohld force-pushed the fix/upload-file-too-big branch from 667d92d to efae9d3 Compare May 21, 2026 12:20
@ohld ohld force-pushed the fix/upload-file-too-big branch from efae9d3 to 28075ed Compare May 21, 2026 12:21
@ohld
Copy link
Copy Markdown
Member Author

ohld commented May 21, 2026

STAFF ENGINEER REVIEW: CHANGES REQUESTED — early source-vote rejection can reject a poll despite a concurrent like.

record_source_candidate_vote reads the poll as open, writes the vote, then immediately closes on counts["yes"] == 0 && counts["no"] >= 6. A concurrent like can pass the same open-poll check before the sixth-dislike call flips the poll to rejected, then write its vote after the close because the upsert is not conditional on the poll still being open. The persisted final counts can become yes=1/no=6 while the poll is already rejected under the "0 likes" rule.

Please serialize vote writes and close decisions around the poll row, or make both operations conditional in the database: do not allow upserting a vote unless the poll is still open, and only early-close with a DB-side condition that still proves zero likes and enough dislikes at the moment the close is claimed. Add a concurrency test for a simultaneous sixth dislike and first like.

Secondary notes: /codex review passed; CSO diff scan found no secret, auth, or SQL injection issue. CI had lint green and test pending at review time.

@ohld
Copy link
Copy Markdown
Member Author

ohld commented May 21, 2026

STAFF ENGINEER REVIEW: APPROVED — current head c291ab1 is clean. The previous source-vote race is addressed by serializing vote writes/close decisions on the poll row; oversized Telegram download failures now mark the meme broken_content_link and notify the uploader instead of crashing auto-review. /codex review returned clean; focused CSO diff scan found no secret, auth, file-upload trust-boundary, or SQL injection issue. Local checks: ruff passed and upload observability tests passed. Storage pytest could not run locally because the ambient env lacks bs4 and uv isolated install failed building psycopg2 without pg_config; GitHub CI is green.

@ohld ohld merged commit 7fa8a71 into production May 21, 2026
3 checks passed
@ohld
Copy link
Copy Markdown
Member Author

ohld commented May 21, 2026

✅ Approved + merged.

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.

1 participant