Skip to content

fix(telegram): retry stale/missed notification on transient network error - #40

Merged
robster7674 merged 2 commits into
glucodroidfrom
fix/telegram-stale-retry
Jun 8, 2026
Merged

fix(telegram): retry stale/missed notification on transient network error#40
robster7674 merged 2 commits into
glucodroidfrom
fix/telegram-stale-retry

Conversation

@robster7674

@robster7674 robster7674 commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

Addresses greptile P1 on PR #36: when `postEdit()` returned `null` (transient network error), the stale/missed alert was silently dropped with no retry path.

What changed

Merges the missed-threshold reschedule and a 60 s transient-retry delay into a single `earliestNextDelayMs` accumulator. On a transient error the scheduler re-fires in 60 s; if a missed-threshold reschedule is pending for another recipient, the earlier of the two wins.

The initial fix used a `retryFor: Set` filter to avoid re-sending to recipients that already succeeded — but greptile correctly flagged that this permanently suppressed the STALE→MISSED promotion for those recipients when another recipient kept failing transiently.

Final approach: drop `scheduleInternal`/`retryFor` entirely. Instead, `STALE_THROTTLE_MS = 70 s > TRANSIENT_RETRY_DELAY_MS = 60 s` means a recipient that just posted STALE is naturally skipped on the 60 s retry cycle (throttled), but re-evaluated when the missed-threshold timer fires minutes later.

private const val TRANSIENT_RETRY_DELAY_MS = 60_000L
// Must exceed TRANSIENT_RETRY_DELAY_MS: a recipient that just posted STALE is
// throttled on the 60 s retry cycle but re-evaluated when the missed-threshold
// timer fires (which can be minutes later).
private const val STALE_THROTTLE_MS = 70_000L

All recipients are re-evaluated on every invocation; the throttle naturally suppresses recently-succeeded ones.

Test plan

  • Simulate a network outage while the stale timer is pending — verify a retry fires ~60 s after connectivity returns
  • Verify missed-threshold scheduling still works normally when the stale post succeeds
  • Verify a recipient that keeps failing transiently does not block STALE→MISSED promotion for a successful recipient

🤖 Generated with Claude Code

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the scheduleInternal(retryFor: Set<String>) approach — which permanently blocked STALE→MISSED promotion for successful recipients while another recipient kept failing — with a simpler throttle constant (STALE_THROTTLE_MS = 70_000L > TRANSIENT_RETRY_DELAY_MS = 60_000L). All recipients are re-evaluated on every invocation; the throttle naturally suppresses a recipient that just posted STALE during the 60 s retry window while still allowing re-evaluation when the throttle expires (~120 s after the last successful post).

  • Removes scheduleInternal/retryFor overload; schedule() is now the single entry point, simplifying the call graph.
  • Increases the stale throttle from 30 s to 70 s so that a just-succeeded recipient is safely skipped on the 60 s retry cycle but re-evaluated on the next iteration.
  • On transient null from postEdit, sets earliestNextDelayMs = TRANSIENT_RETRY_DELAY_MS (60 s) in the same accumulator used for STALE→MISSED reschedule, so whichever is earlier wins.

Confidence Score: 5/5

The change is safe to merge — it correctly fixes the STALE→MISSED promotion suppression and simplifies the scheduling API with no regressions on the happy path.

The new throttle-based approach correctly bounds the suppression window to at most one extra 60 s retry cycle instead of indefinitely, and the retryFor overload that caused the indefinite suppression is gone. The only finding is a style suggestion to derive STALE_THROTTLE_MS from TRANSIENT_RETRY_DELAY_MS to protect the invariant against future edits.

No files require special attention; the single changed file is straightforward and correct.

Important Files Changed

Filename Overview
Common/src/main/java/tk/glucodata/TelegramStaleCheckWork.kt Removes scheduleInternal/retryFor overload and replaces the per-recipient filter with STALE_THROTTLE_MS = 70_000L; the new throttle correctly prevents duplicate STALE posts on the 60 s retry cycle while allowing STALE→MISSED promotion once the throttle window expires. Logic is sound with one style nit: STALE_THROTTLE_MS should be derived from TRANSIENT_RETRY_DELAY_MS to make the invariant self-enforcing.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([schedule fires]) --> B[load config / recipients]
    B --> C{for each recipient}
    C --> D{elapsedMs >= staleThreshold?}
    D -- No --> E[continue]
    D -- Yes --> F{>= missedThreshold?}
    F -- Yes --> G[status = MISSED]
    F -- No --> H[status = STALE]
    G --> I{lastStaleMs > 0 AND now - lastStaleMs < STALE_THROTTLE_MS 70s?}
    H --> I
    I -- Yes throttled --> E
    I -- No --> J[postEdit]
    J -- true 2xx --> K[recordStaleAt now]
    K --> L{status == STALE?}
    L -- Yes --> M[earliestNextDelayMs = min remaining+SLACK]
    L -- No MISSED --> N[no reschedule needed]
    J -- false 4xx --> O[clearRecipientState]
    J -- null transient --> P[earliestNextDelayMs = min 60 s]
    M --> C
    N --> C
    O --> C
    P --> C
    C -- done --> Q{earliestNextDelayMs < MAX_VALUE?}
    Q -- Yes --> R[schedule context delay]
    Q -- No --> S([exit no retry])
Loading

Reviews (4): Last reviewed commit: "fix(telegram): restore transient retry w..." | Re-trigger Greptile

Comment thread Common/src/main/java/tk/glucodata/TelegramStaleCheckWork.kt
Comment thread Common/src/main/java/tk/glucodata/TelegramStaleCheckWork.kt
Comment thread Common/src/main/java/tk/glucodata/TelegramStaleCheckWork.kt
@robster7674

Copy link
Copy Markdown
Owner Author

Root cause identified and fixed directly on glucodroid (563126f). The transient-retry logic from this branch has been merged into glucodroid — closing to keep the PR list clean.

@robster7674 robster7674 closed this Jun 8, 2026
@robster7674 robster7674 reopened this Jun 8, 2026
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

Rob and others added 2 commits June 8, 2026 19:24
…sed approach

The scheduleInternal/retryFor mechanism introduced in the previous commit
permanently suppressed missed-threshold promotion for recipients that
succeeded while another recipient kept failing transiently: the guard
`if (retryFor.isNotEmpty() && recipient !in retryFor) continue`
skipped all evaluation for unaffected recipients on every 60 s retry,
so their STALE→MISSED transition was never triggered.

Fix: drop scheduleInternal and retryFor entirely. All recipients are
re-evaluated on every invocation. The new STALE_THROTTLE_MS = 70 s
(> TRANSIENT_RETRY_DELAY_MS = 60 s) ensures a recipient that just
posted STALE is naturally skipped during the 60 s retry cycle, while
still being re-evaluated when the missed-threshold timer fires (which
can be several minutes later).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…filtering

The previous commit was modified by the pre-commit hook and lost two
critical pieces: the transient-error retry scheduling and the raised
throttle constant.

This commit restores both:
- TRANSIENT_RETRY_DELAY_MS = 60 s (retry interval on network failure)
- STALE_THROTTLE_MS = 70 s (> retry interval, so a recipient that just
  posted STALE is throttled on the 60 s retry cycle but re-evaluated
  when the missed-threshold timer fires minutes later)
- null branch reschedules at TRANSIENT_RETRY_DELAY_MS instead of
  silently dropping the notification

No retryFor/scheduleInternal — all recipients are re-evaluated on every
invocation; the throttle naturally suppresses recently-succeeded ones.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@robster7674
robster7674 force-pushed the fix/telegram-stale-retry branch from 21f613b to 01ea0c7 Compare June 8, 2026 19:25
@robster7674
robster7674 merged commit e2aa8be into glucodroid Jun 8, 2026
5 checks passed
robster7674 added a commit that referenced this pull request Jun 9, 2026
…AY_MS

Addresses greptile style nit from PR #40: STALE_THROTTLE_MS is expressed as
TRANSIENT_RETRY_DELAY_MS + 10_000L, making the throttle > retry-delay
invariant structurally self-enforcing. Drops the now-redundant usage-site
comment that restated the relationship.

No behavioral change; value remains 70 000 ms.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
robster7674 added a commit that referenced this pull request Jun 9, 2026
…AY_MS

Addresses greptile style nit from PR #40: STALE_THROTTLE_MS is expressed as
TRANSIENT_RETRY_DELAY_MS + 10_000L, making the throttle > retry-delay
invariant structurally self-enforcing. Drops the now-redundant usage-site
comment that restated the relationship.

No behavioral change; value remains 70 000 ms.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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