Skip to content

fix(channels): demote channel-message 404s to typed error (OPENHUMAN-TAURI-2Y)#1732

Merged
senamakel merged 3 commits into
tinyhumansai:mainfrom
oxoxDev:fix/telegram-channel-noise
May 15, 2026
Merged

fix(channels): demote channel-message 404s to typed error (OPENHUMAN-TAURI-2Y)#1732
senamakel merged 3 commits into
tinyhumansai:mainfrom
oxoxDev:fix/telegram-channel-noise

Conversation

@oxoxDev
Copy link
Copy Markdown
Contributor

@oxoxDev oxoxDev commented May 14, 2026

Summary

  • Add typed `BackendApiError::MessageNotFound { provider, message_id }` and surface it from `authed_json` whenever a 404 lands on `/channels//messages/`.
  • Four call sites in `src/openhuman/channels/bus.rs` (streaming edit, thinking edit, ephemeral delete, final edit) now downcast and recover in-flow instead of bubbling the 404 into `report_error`.
  • Targets `OPENHUMAN-TAURI-2Y` (~454 events) — by far the dominant telegram-channel Sentry issue. The other 8 telegram noise issues (502/503/504 shapes) are already filtered by upstream `is_transient_backend_api_failure` in `before_send`.

Problem

Telegram (and Discord/Slack) users can delete a message on the provider side between the backend storing the relay row and our streaming loop issuing an edit. The follow-up PATCH/DELETE/POST then 404s on `/channels//messages/`. The current `authed_json` non-transient non_2xx path treats this as a backend bug and emits `report_error` for every event, producing `OPENHUMAN-TAURI-2Y` at ~454 events/wk. From a UX perspective the right answer is to clear the stale id and fall back to atomic delivery — not to surface a Sentry error.

Solution

  • `src/api/rest.rs` — introduce `BackendApiError::MessageNotFound`, gate it behind a narrow `parse_message_path` helper (exact 4-segment shape, literal `channels` / `messages` anchors) so a 404 on any other route — auth probes, integration endpoints, mis-routed proxies — keeps its Sentry signal. Drop is emitted as `tracing::info` at the site for grep-visible triage without crossing the `report_error` threshold.
  • `src/openhuman/channels/bus.rs` — four downcast sites:
    • `flush_streaming_edit` clears `state.message_id`, latches `edit_disabled`, returns early so the next dirty-buffer tick falls back to atomic delivery.
    • `flush_thinking_message` clears `state.thinking_message_id` + latches `thinking_edit_disabled` (cloned borrow so the mutation is allowed).
    • `delete_channel_message` demotes the warn! to info! since "message already gone" is a no-op success.
    • `finalize_channel_reply` skips the orphan-draft delete (nothing to delete) and goes straight to `send_channel_reply` so the user still sees the canonical reply.
  • `src/api/rest_tests.rs` — two integration tests against the existing axum scaffolding: one proving telegram + discord 404 paths downcast correctly, one proving a 404 outside `/channels//messages/` falls through to the existing `report_error` path.

Phase A scope from the original Jules plan (transient HTTP `is_transient_provider_http_failure` + before_send wiring) was dropped — `is_transient_provider_http_failure` / `is_transient_backend_api_failure` / `is_transient_integrations_failure` are already shipped in `src/core/observability.rs` + bilateral before_send in `src/main.rs` + `app/src-tauri/src/lib.rs`. Duplicating it would have collided with the existing classifier names.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via `diff-cover`) meet the gate enforced by `.github/workflows/coverage.yml`. Run `pnpm test:coverage` and `pnpm test:rust` locally; PRs below 80% on changed lines will not merge.
  • N/A: behaviour-only change to existing channel-message routing — Coverage matrix updated — added/removed/renamed feature rows in `docs/TEST-COVERAGE-MATRIX.md` reflect this change
  • All affected feature IDs from the matrix are listed in the PR description under `## Related`
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • N/A: no release-cut surface touched — Manual smoke checklist updated if this touches release-cut surfaces (`docs/RELEASE-MANUAL-SMOKE.md`)
  • Linked issue closed via `Closes #NNN` in the `## Related` section

Impact

  • Desktop core binary (`openhuman-core`) and Tauri shell (`app/src-tauri`) both load the new `BackendApiError` variant through `openhuman_core`. No frontend / JS surface touched. No migration.
  • Sentry: expect `OPENHUMAN-TAURI-2Y` event volume to drop to ~0 once a build hits production (currently ~454 events/wk). Other 404s on non-channel paths keep their existing report path.
  • Performance: one extra `downcast_ref` per channel edit failure (negligible). One `parse_message_path` call per non-200 `authed_json` response (string split, allocation-free on the success path).

Related

  • Closes OPENHUMAN-TAURI-2Y

Summary by CodeRabbit

  • Bug Fixes
    • Gracefully handle messages that were deleted server-side: avoid repeated edit attempts, stop retry loops, and treat delete-of-already-gone messages as success.
    • If a final edit target is missing, send a fresh reply instead of failing or duplicating output; disable further edit attempts when appropriate.
  • Tests
    • Added tests verifying 404s for missing channel messages are classified and handled correctly, while other 404s remain unclassified.

Review Change Stack

@oxoxDev oxoxDev requested a review from a team May 14, 2026 12:03
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 19ea75b5-2aed-4bbd-89d0-4f60d311f186

📥 Commits

Reviewing files that changed from the base of the PR and between 02f9c97 and 28a024c.

📒 Files selected for processing (3)
  • src/api/rest.rs
  • src/api/rest_tests.rs
  • src/openhuman/channels/bus.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/api/rest_tests.rs
  • src/api/rest.rs
  • src/openhuman/channels/bus.rs

📝 Walkthrough

Walkthrough

Introduces BackendApiError::MessageNotFound and parses channel message paths; authed_json returns this error for 404s on /channels/<provider>/messages/<id>. Channel bus handlers and tests are updated to clear stale IDs, latch edit-disabled flags, log success-on-404, or send fresh replies when messages are gone.

Changes

Provider-side message deletion handling

Layer / File(s) Summary
Backend error type and message path parsing
src/api/rest.rs
Introduces BackendApiError enum with MessageNotFound { provider, message_id } variant and parse_message_path helper to extract provider and message ID from /channels/<provider>/messages/<id> paths.
HTTP 404 interception in authed_json
src/api/rest.rs
Updates BackendOAuthClient::authed_json to special-case HTTP 404 responses matching the channel message pattern, returning BackendApiError::MessageNotFound with parsed provider/message_id; other 404s continue through generic error reporting.
Error classification tests
src/api/rest_tests.rs
Adds reqwest::Method import and async integration tests verifying that 404s on /channels/<provider>/messages/<id> POST requests are classified as MessageNotFound with correct provider/message_id for Telegram and Discord, and that non-message-path 404s are not downcast.
Streaming lifecycle message cleanup
src/openhuman/channels/bus.rs
Updates progressive edit, thinking-message flush, message deletion, and finalization handlers to detect BackendApiError::MessageNotFound, clearing stale message IDs, latching edit-disabled flags, logging success-through-404 for deletes, and sending fresh atomic replies during finalization when the draft is already gone.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • tinyhumansai/openhuman#636: Both PRs modify src/openhuman/channels/bus.rs's streaming lifecycle (edit/thinking/delete/finalize) to gracefully handle missing backend message IDs via typed error variants and avoid retry loops on 404 conditions.
  • tinyhumansai/openhuman#1632: Modifies BackendOAuthClient::authed_json non-success HTTP handling; overlaps with this PR's 404 classification changes.

Suggested labels

working

Suggested reviewers

  • graycyrus

Poem

🐰 A message once lived, now it's quite gone,
The provider deleted it—oh what a con!
We parse the path, catch the 404 dance,
Clear stale IDs and stop the retry prance,
Fresh replies hop in to finish the chance! 🌱

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(channels): demote channel-message 404s to typed error' accurately and specifically summarizes the main change: introducing a typed BackendApiError for channel-message 404s and updating error handling to recover in-flow instead of reporting.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 14, 2026
oxoxDev and others added 3 commits May 14, 2026 17:52
When a user deletes their telegram/discord/slack message provider-side,
follow-up PATCH/DELETE/POST against the same id returns 404. The
existing authed_json path funnels every non-transient non_2xx into
report_error, which means OPENHUMAN-TAURI-2Y has accumulated ~454
events for a state that is fundamentally a UI-recovery cue, not a
backend bug.

Add a typed BackendApiError::MessageNotFound variant carrying the
provider segment + message id parsed from the URL, and short-circuit
the non_2xx branch when status == 404 AND the path matches
/channels/<provider>/messages/<id>. The path matcher is intentionally
narrow (exact 4-segment shape + literal 'channels' / 'messages'
anchors) so 404s on any other route — auth probes, integration
endpoints, mis-routed proxies — keep their Sentry signal. The drop is
emitted as tracing::info at the call site so triage sweeps stay
grep-visible without crossing the report_error threshold.

Refs OPENHUMAN-TAURI-2Y.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eNotFound recovery)

Four call sites in channels::bus consume the new
BackendApiError::MessageNotFound and recover in-flow instead of
counting it against MAX_EDIT_FAILURES or surfacing a generic warning:

  flush_streaming_edit     — clear state.message_id, latch
                             state.edit_disabled, return early so
                             subsequent dirty-buffer ticks fall back
                             to atomic delivery
  flush_thinking_message   — clear state.thinking_message_id, latch
                             state.thinking_edit_disabled (cloned
                             borrow so the mutation is allowed)
  delete_channel_message   — demote the warn! to info! since 'message
                             already gone' is a no-op success, not a
                             cleanup failure
  finalize_channel_reply   — skip the orphan-draft delete (there's
                             nothing to delete) and go straight to the
                             fresh atomic send so the user still sees
                             the canonical reply

Each branch keeps the existing non-404 error path intact, so genuine
backend failures retain their warn! signal.

Refs OPENHUMAN-TAURI-2Y.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…through

Two integration tests via the existing axum test scaffolding:

  authed_json_surfaces_message_not_found_on_404 — POST to both
    /channels/telegram/messages/1103 (Sentry shape) and
    /channels/discord/messages/abc (proves provider-agnostic
    parse_message_path)

  authed_json_404_outside_messages_path_still_reports — GET on
    /auth/profile returning 404 must NOT downcast to MessageNotFound
    (guards against accidentally swallowing a real routing bug)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@oxoxDev oxoxDev force-pushed the fix/telegram-channel-noise branch from 02f9c97 to 28a024c Compare May 14, 2026 12:24
@senamakel senamakel merged commit 72a365c into tinyhumansai:main May 15, 2026
23 of 24 checks passed
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
…TAURI-2Y) (tinyhumansai#1732)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants