fix(relay): bounded recursion in sendTelegram on 429 rate limit by fuleinist · Pull Request #3338 · koala73/worldmonitor

fuleinist · 2026-04-23T11:53:34Z

Bug

Add _retryCount parameter (default 0) to sendTelegram and guard against unbounded recursion when Telegram returns sustained 429 responses.

Changes

_retryCount parameter (default 0) added to sendTelegram signature
Bail with warn log if _retryCount >= 1
Pass _retryCount + 1 on recursive call
Non-429 paths unaffected (no behavioral change for 200, 400, 403, 401)

Verification

Read sendTelegram signature — confirm it has a _retryCount parameter defaulting to 0
Confirm the recursive call passes _retryCount + 1
Confirm a guard bails (returns false + logs) when _retryCount >= 1
Confirm non-429 paths are unaffected

Replicate legacy NEG_TTL=30000 behaviour in list-military-flights fetchStaleFallback. After a failed live fetch, do not immediately re-read the stale Redis key — that just wastes an Upstash call if OpenSky/the relay is still down. Suppress stale-read attempts for 30 s after the last failed live fetch attempt. Fixes koala73#3277

Add _retryCount parameter (default 0) to sendTelegram and guard against unbounded recursion when Telegram returns sustained 429 responses. - Bail with warn log if _retryCount >= 1 - Pass _retryCount + 1 on recursive call - Fixes koala73#3060

vercel · 2026-04-23T11:53:38Z

@fuleinist is attempting to deploy a commit to the World Monitor Team on Vercel.

A member of the Team first needs to authorize it.

greptile-apps · 2026-04-23T11:55:46Z

Greptile Summary

This PR makes two targeted fixes: (1) bounds the previously unbounded recursion in sendTelegram when Telegram returns sustained 429s, by adding a _retryCount guard that bails after one retry; (2) fixes the negative-cache guard in listMilitaryFlights so lastStaleAttempt is only armed when the stale fallback actually returned empty, not unconditionally before the fetch. Both changes are minimal, correct, and address the stated bugs without altering non-429 or non-failure paths.

Confidence Score: 5/5

Both fixes are correct and minimal; no P0/P1 issues found — safe to merge.

The sendTelegram recursion bound is logically sound (guard fires at _retryCount >= 1, allowing exactly one retry). The negative-cache guard placement in listMilitaryFlights correctly addresses the previously flagged issue — lastStaleAttempt is only set when stale data was confirmed empty, so live-cache failures still serve valid stale data within the window. No regressions to non-429 or non-failure paths. All remaining findings are P2 or lower.

No files require special attention.

Important Files Changed

Filename	Overview
scripts/notification-relay.cjs	Adds `_retryCount` parameter to `sendTelegram` with a `>= 1` guard that terminates recursion after one retry on 429; non-429 paths are unaffected.
server/worldmonitor/military/v1/list-military-flights.ts	Adds module-level negative-cache guard (`lastStaleAttempt`) that is correctly armed only after stale fallback returns empty, not before the fetch.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant sendTelegram
    participant TelegramAPI

    Caller->>sendTelegram: sendTelegram(userId, chatId, text, _retryCount=0)
    sendTelegram->>TelegramAPI: POST /sendMessage
    TelegramAPI-->>sendTelegram: 429 Too Many Requests

    alt _retryCount >= 1
        sendTelegram-->>Caller: false (bail — retry limit reached)
    else _retryCount == 0
        sendTelegram->>sendTelegram: await retry_after delay
        sendTelegram->>sendTelegram: sendTelegram(userId, chatId, text, _retryCount=1)
        sendTelegram->>TelegramAPI: POST /sendMessage
        alt 429 again
            sendTelegram-->>Caller: false (bail — _retryCount >= 1)
        else 200 OK
            sendTelegram-->>Caller: true
        end
    end

_{Reviews (2): Last reviewed commit: "fix(military): move lastStaleAttempt ass..." | Re-trigger Greptile}

greptile-apps · 2026-04-23T11:55:50Z

+      lastStaleAttempt = now;
      const staleFlights = await fetchStaleFallback();
      if (staleFlights && staleFlights.length > 0) {
        return { flights: filterFlightsToBounds(staleFlights, requestBounds), clusters: [], pagination: undefined };


Negative-cache guard fires even when stale data is available

lastStaleAttempt is set unconditionally before calling fetchStaleFallback(). This means the first failed-live request per 30-second window gets stale data, but every subsequent failed request within that window hits the guard at line 276–279 and returns an empty list — even though the stale Redis key is present and unchanged. The goal of a negative-cache guard is to suppress lookups that returned nothing; it should only be armed when the stale fetch came back empty or null.

To fix, move lastStaleAttempt = now to after the empty-stale check:

const staleFlights = await fetchStaleFallback(); if (staleFlights && staleFlights.length > 0) { return { flights: filterFlightsToBounds(staleFlights, requestBounds), clusters: [], pagination: undefined }; } lastStaleAttempt = now; // only negative-cache when stale came back empty markNoCacheResponse(ctx.request); return { flights: [], clusters: [], pagination: undefined };

✅ Fixed in 914e156f.

The negative-cache guard is now armed only after fetchStaleFallback() returns empty — i.e., when we actually confirmed no stale data was available, not just after attempting the read.

lastStaleAttempt = now moved to after the stale-empty check, exactly as suggested. This ensures that if stale data exists and hasn't changed, subsequent requests within the NEG_TTL_MS window will still serve it on the next stale-read cycle, rather than being short-circuited by the guard before even attempting the fallback.

Only arm the negative-cache guard when stale data was actually absent, not when we merely attempted a stale read. Fixes comment: koala73#3338 (review)

fuleinist · 2026-04-24T01:55:55Z

PR Feedback Addressed ✅

Reviewed all comments from @greptile-apps[bot].

Changes Made

#	Reviewer	File	Comment	Resolution	Commit
1	greptile-apps[bot]	`server/worldmonitor/military/v1/list-military-flights.ts`	Negative-cache guard fires even when stale data is available	Fixed — `lastStaleAttempt = now` moved to after stale-empty check	`914e156f`

Files Changed

server/worldmonitor/military/v1/list-military-flights.ts — Move lastStaleAttempt = now to after the staleFlights.length > 0 check so the negative-cache guard only arms when stale data was confirmed absent, not merely attempted

Verification

tsc --noEmit passes
git push completed successfully

Fixes the issue where subsequent failed-live requests within the NEG_TTL_MS window were returning empty even though stale data was present and unchanged. Now the guard only short-circuits after an actual confirmed absence of stale data.

Ready for re-review. cc @greptile-apps[bot]

fuleinist added 2 commits April 23, 2026 07:54

github-actions Bot added the trust:safe Brin: contributor trust score safe label Apr 23, 2026

greptile-apps Bot reviewed Apr 23, 2026

View reviewed changes

fix(military): move lastStaleAttempt assignment after stale-empty check

914e156

Only arm the negative-cache guard when stale data was actually absent, not when we merely attempted a stale read. Fixes comment: koala73#3338 (review)

koala73 closed this Apr 29, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(relay): bounded recursion in sendTelegram on 429 rate limit#3338

fix(relay): bounded recursion in sendTelegram on 429 rate limit#3338
fuleinist wants to merge 3 commits into
koala73:mainfrom
fuleinist:fix/relay-duplicate-shadow-log-v2

fuleinist commented Apr 23, 2026

Uh oh!

vercel Bot commented Apr 23, 2026

Uh oh!

greptile-apps Bot commented Apr 23, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Apr 23, 2026

Uh oh!

fuleinist Apr 24, 2026

Uh oh!

fuleinist commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

fuleinist commented Apr 23, 2026

Bug

Changes

Verification

Uh oh!

vercel Bot commented Apr 23, 2026

Uh oh!

greptile-apps Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

fuleinist Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

fuleinist commented Apr 24, 2026

PR Feedback Addressed ✅

Changes Made

Files Changed

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Apr 23, 2026 •

edited

Loading