Commit Graph
10 Commits
Author SHA1 Message Date
YoursFunny 62507d3a01 perf: bound the memory photo preparation holds, not just its count
`PREP_SLOTS` caps how many items are prepared at once (6) but says nothing
about what they hold: one photo's decode buffer can be up to
`MAX_DECODE_BYTES` (512 MiB) and that guard is *per photo*, so six of them —
an album of large scans, two chats at once — could peak near 3 GiB on a host
sized for a fraction of it. The upload fallback is the only path that
allocates like this; nothing downstream notices until the kernel does.

Photo preparation now charges a process-wide memory budget
(`MEMORY_UNITS` × 64 MiB = 512 MiB) for what it actually holds: the
downloaded bytes plus the decode buffer the *header* predicts — the same
prediction the per-photo guards apply, now shared (`decode_bytes`,
`decode_budget_bytes`) so the reservation and the guard cannot drift. A photo
that is already within Telegram's limits is billed only its download, so an
ordinary 10-image album still runs several at a time; two photos near the
per-photo cap serialize (each takes the whole budget). The request is clamped
to the budget so a single huge photo runs alone instead of waiting for
permits that cannot exist.

Verified with a throwaway harness against a locally served 9999x9999 PNG
(126 KB on the wire, ~100 MB decoded) driven through the real
`prepare_upload_item`: with the budget held the preparation waits — "after
1516ms the prep is still waiting on the budget" — and finishes in 11.8s the
moment it is released, so the accounting binds in the real path and not just
in the semaphore.

Kept as permanent tests instead: the unit math (rounding, clamp, and that a
max-size photo still gets the whole budget rather than waiting forever), the
budget sharing (huge decodes cannot overlap, ordinary ones do not queue), and
the prediction agreeing with the processing decision (over-sized PNG/JPEG
charged, within-limits and unknown formats free).

`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean (122 bot + 91 x-media).
2026-09-21 14:31:50 +08:00
YoursFunny 362eb9e729 perf: bound upload-fallback preparation process-wide
Each batch's items were prepared under their own `Semaphore::new(3)`, which
is not a memory bound: 8 URL workers and 4 queue workers can each be inside
a batch, so a burst could have two dozen downloads in flight at once, each
buffering a whole photo before it is processed. Nothing else on the media
path bounds them — the send itself is paced by the rate limiter, but the
download and the decode happen before it is charged.

One process-wide `PREP_SLOTS` (6) replaces the per-batch semaphore, and the
photo download gets its own cap: `MAX_PHOTO_DOWNLOAD_BYTES` (32 MiB) for the
transfer, with `MAX_DECODE_BYTES` (512 MiB) left as the pre-allocation guard
on a single decoded buffer. A photo over the download cap degrades to its
smaller URL exactly as one over the decode budget does
(`FallbackError::MediaTooLarge` → `fallback_url`) — never an error.

Verified with the same throwaway proxy harness: 4 concurrent 10-item batches
against a server that holds every response 150 ms peak at exactly 6
concurrent downloads (the per-batch three allowed 12) with all 40 items
prepared.

`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
2026-09-21 04:10:28 +08:00
YoursFunny 39dbd0f3a2 test: drop redundant tests, make the vacuous ones real
Audit of all 205 tests (five read-only passes plus a line-by-line
re-check). Ten test functions were removed or merged and eight
subsumed assertion blocks trimmed; the suite is down to 180 tests with
no loss of mutation coverage, and four tests that were passing for
nothing now fail when the code they name is broken.

Redundant (deleted or merged):
- twitter: `syndication_text_only_has_no_media` (re-asserts its own
  empty fixture), `..._keeps_multibyte_text` (both transforms are
  no-ops for that text), `..._strips_trailing_short_link_without_entities`
  (same branch as `..._media_short_link`, which now also covers the
  real multibyte tweet), `..._regardless_of_index_units` (its
  `display_text_range` rationale outlived the function it described).
- pixiv: `test_fetch` (a bare `is_ok()` on the illustration
  `download_media_pixiv_original_with_referer` already asserts and
  downloads, and the only network touch in a plain `cargo test`),
  `startup_validation_only_disables_on_a_definitive_failure` (four rows
  that are a subset of the retry-policy table; the `validate()` branch
  it was named for is not asserted at all).
- bilibili: `from_item_legacy_draw_shape_still_parses` (its fixture is
  the same legacy `draw` shape `from_item_maps_draw_images_and_topic`
  builds, with a subset of its assertions).
- site/mod.rs: two `Ok(None)` cases merged into one test.
- urls.rs: `cache_hit_success_keeps_the_cache_entry` (the `/test` test
  asserts the same two things under stricter settings), plus a
  `assert_ne!` loop that re-states the mapping assertions above it.
- send/mod.rs: `media_group_success_and_forward_ok` (the forward half is
  covered by `post_send_forwards_immediately_when_configured`; the
  `is_ok()` half cannot see the returned file ids), and two boundary
  rows implied by the constant they sit next to.
- commands.rs: the parse tail that `every_documented_invocation_parses`
  already covers per README form, and three `debug_report` rows the
  escaping test pins with stronger input.

Passing for nothing (now real):
- `truncate_caption_does_not_split_an_html_entity` — the cut lands
  inside the entity, so `!contains("&amp")` never fired; it now asserts
  the exact output in both directions and fails when the guard in
  `truncate_caption` is deleted (verified).
- `pipeline_resizes_oversized_jpeg` — magic bytes and a non-empty buffer
  pass for a copy-through; it now decodes the output's headers and
  fails when the JPEG branch skips the resize (verified).
- `live_validate_with_bogus_token_fails` — expected `PixivError::Api`,
  which the status check before the body read made unreachable; a bogus
  token is a 4xx. Confirmed against the live endpoint: the old
  assertion fails with `got Err(Status(400))`, the new one passes.
- bsky `live_fetch_with_photos` — its URL is a text-only post and it had
  a byte-identical twin, so no live test pinned media; it now points at a
  labelled post with photos and asserts media + the label (live-verified).

Also fixed, found by turning the runtime-sweep test into a real one:
the 30 s lease-expiry sweep recovered crashed rows but never woke a
worker, so a recovered task waited for the next unrelated enqueue (every
worker is parked on `notify` when no row is pending). `recover_update`
now reports its count, `recover_expired` wakes a worker when it changed
something, and `runtime_sweep_recovers_expired_lease` drives the spawned
loop with a paused clock instead of calling the recovery by hand — it
fails on both the missing wake-up and a sweep that recovers nothing.

Verified: `cargo fmt --check`, `cargo clippy --workspace --all-targets
--locked -- -D warnings`, `cargo test --workspace --locked` (180 passed,
14 ignored) and `cargo test -p x-media -- --ignored live` (13 passed).
2026-09-21 00:16:11 +08:00
YoursFunny 4cdf618c25 fix(retry): fence the queue lease, clean up after a kill, name dead-lettered posts
P2 (hardening) of the retry audit, closing the report's remaining findings.

- Lease fencing. `lease_next` now stamps a random `lease_token`, and every
  write-back a worker makes (the 30s heartbeat, `delete_row`, `reschedule`,
  `mark_done`) is guarded by it. A lease that expired while its holder was
  stalled and was then re-leased used to let *both* holders write the same row:
  one duplicated the send, the other silently discarded the new holder's retry
  (a 0-row update was not even logged). Now a worker that no longer holds the
  lease drops its attempt at the next heartbeat and writes nothing. Reaching
  existing databases needed a migration chain, which `db.rs` had been
  pre-committed to: `MIGRATIONS` + `migrate` track `PRAGMA user_version`, with
  `schema_init` as the version-0 baseline. Verified on a database created
  before this change: user_version 0 -> 1, column added, rows intact.
- Dead-letter notifications no longer mislabel an unparsable payload. A row
  whose payload no longer deserializes as a `Task` (an older version's shape,
  corruption) used to skip the cache invalidation *and* report "Forward failed
  permanently" for a send task, because both were derived from the parsed
  value. The identity now comes off the raw JSON, so the stale link-cache entry
  is dropped and the message names the post.
- Temp files are marked and swept. Every temp file/dir the project creates now
  carries `x_media::TEMP_FILE_PREFIX`, and startup removes entries with that
  prefix older than an hour — a killed process leaves its downloads (up to
  hundreds of MB) behind because no destructor runs, and the age gate keeps the
  sweep away from a second instance's in-flight files. Verified live: the log
  reports the sweep, an aged leftover goes, a fresh prefixed file and an
  unrelated file stay.
2026-09-20 21:34:45 +08:00
YoursFunny 89c4642e1c fix(lint): resolve clippy warnings from rust 1.98
- photo.rs: chunks_exact(4)/(2) -> as_chunks::<N>().0
  (chunks_exact_to_as_chunks, the new lint prefers the
  compile-time-checked slice split)
- send.rs: box the Task inside SendError so the error fits the
  result_large_err limit (Task is ~400 bytes; the error now moves
  through Result as a pointer); unbox with *task at the two
  enqueue_retry call sites (handlers/urls.rs, handlers/callback.rs)

cargo clippy --workspace --all-targets is now warning-free; the
remaining proc-macro-error2 future-incompat note is upstream
(teloxide -> aquamarine) and unfixable locally. Full test suite passes.
2026-09-07 17:00:38 +08:00
YoursFunny 6b3e61881d logging: re-level, redact user data at info, and key links by post id
P0 — level rework + redaction:
- info now carries only lifecycle, per-post business results (sent /
  forwarded / copied / template applied), admin actions and anomalies
  (upload fallback, retry enqueue; dead-letter stays error).
- Per-request detail moved to debug: message/command logging, URL
  extraction, fetching/fetched, link-cache hits, media-group batch sends,
  queue processing (enqueue/processing/completed/rescheduled), photo
  processing (downscale/transcode), inline queries, sensitive-tweet note.
- Full user-submitted URLs and message text now appear only at debug; at
  info and above links are printed via the normalized cache key.

P1 — request correlation:
- handlers::log_key() maps a URL to its normalized post key
  (twitter:<id> / pixiv:<id> / bsky:<handle>/<rkey>). The whole lifecycle
  of one link (fetch -> send -> cache -> fallback) now logs [key=...], so
  multi-worker logs can be correlated by grepping the key.

Convention documented in AGENTS.md.
2026-08-13 23:34:06 +08:00
YoursFunny 1747d321d8 perf(photo): stop re-reading the downloaded temp file
download_to_temp buffered the full bytes, wrote them to a temp file, and
prepare_photo then read the whole file back from disk. The bytes are
already in memory — pass them through (download_to_temp now returns
(file, bytes)) so photo processing never touches the disk for input.
Adds the bytes dependency to xmedia-bot (already in the lock via x-media).
2026-08-13 22:24:58 +08:00
YoursFunny d61dba5096 send: stream media downloads with hard size caps
download_to_temp now uses download_media_limited: non-photos abort the
moment the 10 MiB upload cap is crossed mid-stream (no more full-body
buffering before the size check), photos cap at the 512 MiB decode
budget, and the ugoira frame zip gets a 512 MiB cap. MediaTooLarge
routes to the existing smaller-URL fallback.
2026-08-08 20:03:14 +08:00
YoursFunny 3d6f8548c3 style: cargo fmt across the workspace
Apply rustfmt to the 11 files that had drifted (86 hunks): x-media
site modules (bsky/pixiv/twitter) and xmedia-bot (config/main/
photo/send). Formatting only - no semantic changes; full test suite
still green.
2026-08-07 16:12:01 +08:00
YoursFunny bc954e6e0b fix: handle oversized photos Telegram rejects with pure Rust processing 2026-08-06 10:17:29 +08:00