Complete Reddit bridge

[Fizzadar]

No description provided.

[Fizzadar]

@indent review please

[indent]

Issues

All clear! No issues remaining. 🎉

10 issues already resolved

• With bridge.split_portals false, shared group portals with nil saved State (created by main, via CreateGroup, or before the first ChatResync) make savedRoomPortal error before sync state is merged. Sync then retries forever, and after the upgrade's full resync the whole account stops bridging. (fixed by commit 5846ee3)
  _{Found by Indent Review Agent}
• handleSync aborts the whole batch on any per-room or per-event error and runSync retries it forever, so one deterministic failure (unsupported redaction target, unknown chat type, non-t2_ user, permanent reaction image 404) leaves the login in TRANSIENT_DISCONNECT and stops all bridging. (fixed by commit 429c6fb)
  _{Found by Indent Review Agent}
• In FetchMessages, one event that fails conversion (unsupported msgtype, edit, media download failure, reaction image failure) returns an error for the whole page, so catch-up and backward backfill never make progress past it. (fixed by commit 429c6fb)
  _{Found by Indent Review Agent}
• Sync now advances the cursor even on transient failures (network, 5xx, 429, DB errors in room-state resolution or GetEvent), so those messages, redactions and unhides are lost permanently. Previously they were retried. (fixed by commit 04a7cd2)
  _{Found by Indent Review Agent}
• Backfill stores an error placeholder under the real message ID for transient media failures (download timeout, 5xx, 429, upload failure), so the image is permanently replaced and never retried. (fixed by commit 04a7cd2)
  _{Found by Indent Review Agent}
• Non-split deployments: DMs that main saved as shared (receiver-less) portals fail the ownership check on every sync and now silently stop bridging. (fixed by commit 91979f5)
  _{Found by Indent Review Agent}
• Every room with an unknown chat type triggers an unfiltered full account sync on each poll until its portal state is saved, which can cause load and rate limiting. (fixed by commit 91979f5)
  _{Found by Indent Review Agent}
• Only 404/410 are classified as permanent. Other permanent 4xx responses (403 on GetEvent or media, redirect rejections, upload rejections) are retried forever, so one such event stalls sync for the whole account and fails every backfill retry. (fixed by commit 91979f5)
  _{Found by Indent Review Agent}
• Every 403 is classified as permanent, but Reddit's edge returns temporary 403 blocks. During a block, redactions are dropped, reaction images fall back to text, and backfilled media or reactions are skipped or replaced, all permanently. (fixed by commit 144118b)
  _{Found by Indent Review Agent}
• Failures not classified as permanent retry the whole sync batch with no attempt or time limit, and the held cursor stops every chat on the account. There is no point at which the offending event is skipped. (fixed by commit 144118b)
  _{Found by Indent Review Agent}

CI Checks

All CI checks passed on da909c6.

Review agents

Select any unchecked box below to run or rerun that agent.

Passed (1)

• Indent Review Agent · The edge-403 nit is fixed; one residual nit: unclassified permanent failures retry with no limit.

Full results

Indent Review Agent

• Summary: The edge-403 nit is fixed; one residual nit: unclassified permanent failures retry with no limit.
• Last ran on commit: da909c65
• Latest result

  {
    "summary": "The edge-403 nit is fixed; one residual nit: unclassified permanent failures retry with no limit.",
    "findings": [
      {
        "fix": "Track consecutive failures of the same sync batch (same `since` cursor). After N attempts, or a time budget, switch that pass into skip mode: log and skip each room or event that still fails, instead of retrying the whole batch. That keeps edge blocks retryable for a while without letting one deterministic failure hold the account forever.",
        "line": 89,
        "path": "pkg/connector/chatsync.go",
        "issue": "The author's claim checks out against the code.\n- `isPermanentHTTPError` returns true for 404/410 always.\n- For any other 4xx it returns true only when `httpErr.RespError != nil`. In mautrix v0.30, `ParseErrorResponse` sets `RespError` to nil when the body has no `errcode` (client.go:766-775), so HTML or empty edge blocks retry.\n- 401/`M_UNKNOWN_TOKEN`/`M_MISSING_TOKEN` (via `isUnauthorized`), 408 and 429 are excluded. `M_LIMIT_EXCEEDED` normally arrives as 429.\n- Reaction images become unbridgeable only on 404/410 (`isGoneStatus`, emoji.go:75).\n- The redirect rejection is a plain error again, so it retries.\n\nThe remaining risk is that every failure the classifier doesn't recognise as permanent retries the whole sync batch with no upper limit. Because the cursor is held, one such failure stops every chat on the account until it clears. Examples:\n- An off-host or long redirect from i.redd.it for a catalogued reaction.\n- A permanent 4xx without a JSON errcode from the media CDN or i.redd.it.\n- A deterministic Beeper `Bot.UploadMedia` rejection of a reaction image.\nThere is no attempt count or time budget after which the offending event is skipped. This is unlikely with today's static reaction assets, so I'm reporting it as a nit.",
        "title": "Failures the classifier doesn't recognise as permanent still retry with no limit and hold the cursor",
        "severity": "nit"
      }
    ]
  }

[indent]

Review started on 18c7892. Findings will show up in the banner comment.