fix(relay): prevent unbounded recursion in sendTelegram on sustained 429 by fuleinist · Pull Request #3478 · koala73/worldmonitor

fuleinist · 2026-04-27T19:13:57Z

Summary

Fix unbounded recursion in sendTelegram() when Telegram returns sustained HTTP 429 responses. Under rate-limiting bursts, the relay could recurse infinitely and crash the Railway process.

Problem

In scripts/notification-relay.cjs, the 429 handler called sendTelegram() recursively with no counter:

if (res.status === 429) {
  const wait = ((body.parameters?.retry_after ?? 5) + 1) * 1000;
  await new Promise(r => setTimeout(r, wait));
  return sendTelegram(userId, chatId, text); // "single retry" — but no guard
}

The comment says "single retry" but there was no enforcement.

Solution

Add _retryCount parameter (default 0) to sendTelegram()
Add guard at 429: if _retryCount >= 1, log and return false immediately
Pass incremented count: recursive call passes (_retryCount ?? 0) + 1
Non-429 paths (200, 400, 403, 401) are completely unaffected

Changes

scripts/notification-relay.cjs: 2 edits to sendTelegram()
tests/notification-relay-telegram-retry.test.mjs: new regression test (static analysis, matches repo test patterns)

Testing

New test: 4 assertions (param signature, guard+return, incremented call, single call site) — all pass
All 27 existing relay tests — pass
Verification steps from issue confirmed:
- sendTelegram has _retryCount parameter defaulting to 0
- Guard bails (returns false + logs) when _retryCount >= 1
- Recursive call passes _retryCount + 1
- Non-429 paths unaffected

Fixes #3060

The 429 handler in sendTelegram() called itself unconditionally with no retry counter. Under sustained Telegram rate limiting, this could recurse indefinitely and crash the Railway relay process. Changes: - Add _retryCount parameter (default 0) to sendTelegram() - Add guard: if (_retryCount ?? 0) >= 1, log and return false immediately - Pass (_retryCount ?? 0) + 1 on the recursive 429 call (single retry only) - Non-429 paths (200, 400, 403, 401) are unaffected Testing: - Add regression test (static analysis of source, matching repo patterns) - All 4 new assertions pass: param signature, guard + return false, incremented recursive call, single call site - All 27 existing relay tests continue to pass Fixes koala73#3060

vercel · 2026-04-27T19:14:02Z

@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-27T19:16:10Z

Greptile Summary

This PR adds a _retryCount parameter to sendTelegram() and a guard that returns false after one 429 retry, correctly preventing the unbounded recursion described in issue #3060. The fix is minimal and confined to the 429 branch; all other status-code paths are unaffected. The accompanying regression test validates the fix via static source-text analysis rather than real function execution, making it fragile to any reformatting of the guard block.

Confidence Score: 4/5

Safe to merge; the runtime fix is correct and all findings are style/quality P2s.

No P0 or P1 issues found. All three comments are P2: redundant null-coalescing, unconsumed response body on the early-bail path, and fragile static-analysis tests. None affect the correctness of the recursion guard itself.

tests/notification-relay-telegram-retry.test.mjs — the static-analysis test approach should be replaced with a proper mocked-fetch test before it becomes a maintenance burden.

Important Files Changed

Filename	Overview
scripts/notification-relay.cjs	Adds `_retryCount = 0` parameter to `sendTelegram` and a guard that bails on the second 429; logic is correct but `?? 0` is redundant and the unconsumed response body on the bail path is a minor connection-pool concern.
tests/notification-relay-telegram-retry.test.mjs	New regression test uses static source-text parsing (regex/string matching) instead of actually invoking `sendTelegram` with a mocked `fetch`; all four assertions are tightly coupled to exact source formatting and will break on any minor refactoring.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["sendTelegram(userId, chatId, text, _retryCount=0)"] --> B{TELEGRAM_BOT_TOKEN set?}
    B -- No --> C[return false]
    B -- Yes --> D[POST /sendMessage]
    D --> E{res.status}
    E -- 403/400 --> F[log + optionally deactivate channel\nreturn false]
    E -- 429 --> G{_retryCount >= 1?}
    G -- Yes --> H[log warn\nreturn false]
    G -- No --> I[read retry_after from body\nwait delay]
    I --> J["sendTelegram(..., _retryCount + 1)"]
    J --> E
    E -- 401 --> K[log error\nreturn false]
    E -- other non-ok --> L[log warn\nreturn false]
    E -- 200 ok --> M[log delivered\nreturn true]

_{Reviews (1): Last reviewed commit: "fix(relay): prevent unbounded recursion ..." | Re-trigger Greptile}

greptile-apps · 2026-04-27T19:16:15Z

+    if ((_retryCount ?? 0) >= 1) {
+      console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);
+      return false;
+    }
    const body = await res.json().catch(() => ({}));
    const wait = ((body.parameters?.retry_after ?? 5) + 1) * 1000;
    await new Promise(r => setTimeout(r, wait));
-    return sendTelegram(userId, chatId, text); // single retry
+    return sendTelegram(userId, chatId, text, (_retryCount ?? 0) + 1); // single retry


Redundant null-coalescing on _retryCount

_retryCount is declared with a default value of 0 in the function signature, so it can never be null or undefined inside the function body. The ?? 0 guards on lines 296 and 303 are dead code — they add noise without providing any safety benefit.

Suggested change

if ((_retryCount ?? 0) >= 1) {

console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);

return false;

}

const body = await res.json().catch(() => ({}));

const wait = ((body.parameters?.retry_after ?? 5) + 1) * 1000;

await new Promise(r => setTimeout(r, wait));

return sendTelegram(userId, chatId, text); // single retry

return sendTelegram(userId, chatId, text, (_retryCount ?? 0) + 1); // single retry

if (_retryCount >= 1) {

console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);

return false;

}

const body = await res.json().catch(() => ({}));

const wait = ((body.parameters?.retry_after ?? 5) + 1) * 1000;

await new Promise(r => setTimeout(r, wait));

return sendTelegram(userId, chatId, text, _retryCount + 1); // single retry

greptile-apps · 2026-04-27T19:16:17Z

+    if ((_retryCount ?? 0) >= 1) {
+      console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);
+      return false;
+    }


Response body left unconsumed on early bail

When the guard fires (_retryCount >= 1), the function returns false before res.json() is called. The 429 response body is never consumed. In Node.js's undici-backed fetch, an unconsumed response body can hold the underlying TCP socket open until the body stream is garbage-collected, potentially exhausting the connection pool under heavy rate-limiting. Adding res.body?.cancel() before the return false keeps the connection lifecycle clean.

Suggested change

if ((_retryCount ?? 0) >= 1) {

console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);

return false;

}

if ((_retryCount ?? 0) >= 1) {

res.body?.cancel();

console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);

return false;

}

greptile-apps · 2026-04-27T19:16:19Z

+  const openIdx = src.indexOf('{', idx);
+  let depth = 1;
+  let i = openIdx + 1;
+  while (i < src.length && depth > 0) {
+    if (src[i] === '{') depth++;
+    else if (src[i] === '}') depth--;
+    i++;
+  }
+  return src.slice(idx, i);
+}
+
+describe('notification-relay sendTelegram retry discipline', () => {
+  const fn = extractSendTelegram(relaySrc);
+
+  it('has a _retryCount parameter defaulting to 0', () => {
+    assert.match(
+      fn,
+      /async function sendTelegram\s*\(\s*\w+,\s*\w+,\s*\w+,\s*_retryCount\s*=\s*0\s*\)/,
+      'sendTelegram must accept _retryCount = 0 parameter',
+    );
+  });
+
+  it('bails and returns false when _retryCount >= 1 (no infinite recursion)', () => {
+    // There must be a guard that returns false when retry count is exceeded.
+    // The guard code is: if ((_retryCount ?? 0) >= 1) { console.warn(...); return false; }
+    assert.match(
+      fn,
+      /_retryCount.*?\)\s*>=\s*1/,
+      'sendTelegram must guard against _retryCount >= 1 to prevent infinite recursion',
+    );
+
+    // The guard must return false (not recurse again)
+    assert.ok(
+      /if\s*\([^)]*_retryCount[^)]*\)\s*>=\s*1/.test(fn),
+      'guard if-statement with _retryCount >= 1 check not found',
+    );
+    assert.ok(
+      fn.includes('return false'),
+      'guard must return false when retry limit is exceeded',
+    );
+
+    // The return false must be INSIDE the guard if-block, not somewhere else
+    const guardBlockIdx = fn.indexOf('if ((_retryCount ?? 0) >= 1)');
+    assert.ok(guardBlockIdx !== -1, 'guard block not found');
+    const afterGuard = fn.slice(guardBlockIdx);
+    const returnIdx = afterGuard.indexOf('return false');
+
+    // Find the matching closing brace of the guard block (not the first '}')
+    const openBraceIdx = afterGuard.indexOf('{');
+    let braceDepth = 0;
+    let closingBraceIdx = -1;
+    for (let j = openBraceIdx; j < afterGuard.length; j++) {
+      if (afterGuard[j] === '{') braceDepth++;
+      else if (afterGuard[j] === '}') {
+        braceDepth--;
+        if (braceDepth === 0) { closingBraceIdx = j; break; }
+      }
+    }
+
+    assert.ok(
+      returnIdx !== -1 && returnIdx < closingBraceIdx,
+      'return false must appear before the closing brace of the guard block',
+    );
+  });
+
+  it('passes incremented retry count on the recursive 429 call', () => {
+    // The recursive call inside the 429 block must pass an incremented retry count.
+    // Since regex can't easily match nested parens, we verify:
+    // 1. sendTelegram is called recursively (the only call site in the 429 block)
+    // 2. _retryCount is passed as an argument
+    // 3. + 1 is present in the function (the increment)
+    const recursiveCallIdx = fn.indexOf('return sendTelegram(');
+    assert.ok(recursiveCallIdx !== -1, 'recursive sendTelegram call not found');
+    const afterCall = fn.slice(recursiveCallIdx);
+    assert.ok(
+      /_retryCount/.test(afterCall),
+      'recursive call must pass _retryCount parameter',
+    );
+    assert.ok(
+      /\+\s*1/.test(afterCall),
+      'recursive call must increment retry count with + 1',
+    );
+  });
+
+  it('does not recurse for any status code other than 429', () => {
+    // Count all sendTelegram( calls inside the function body.
+    // There should be exactly 1: the recursive one in the 429 handler.
+    const bodyOnly = fn.slice(fn.indexOf('{') + 1, fn.lastIndexOf('}'));
+    const allCalls = bodyOnly.match(/sendTelegram\s*\(/g) || [];
+    assert.equal(
+      allCalls.length, 1,
+      `sendTelegram body should contain exactly 1 self-call (the 429 retry); found ${allCalls.length}`,
+    );
+  });
+});


Static source-text analysis is brittle

The tests never actually call sendTelegram — they parse the source file as a raw string and pattern-match against it. Any innocent refactoring (adding a space, simplifying ?? 0, renaming the guard variable) will break all four assertions with confusing errors even when runtime behaviour is correct. Line 73 hard-codes the exact token 'if ((_retryCount ?? 0) >= 1)', so the simplification suggested above would immediately break the test suite. A mock-fetch integration test would be far more durable.

- Drop redundant `?? 0` — the `_retryCount = 0` default already guarantees the value is a number inside the function body. - Cancel the 429 response body on the bail path. Without this the undici socket can be held open until the body stream is GC'd, which under sustained rate limiting could pressure the connection pool. - Replace the static-source-text test with a proper integration test that invokes `sendTelegram` against a mocked `globalThis.fetch`. The new test asserts call count and return value across 200, 429→200, sustained 429 (bail), 403, and 401, plus body-cancel hygiene on the bail path. Source formatting changes that don't alter behaviour will no longer break it. Required surgery in the relay module: - Wrap the SIGTERM handler + `subscribe()` invocation in `if (require.main === module)` so requiring the script from a test does not start the poll loop. - Export `{ sendTelegram }` for direct invocation in tests. The relay's third-party deps (`resend`, `convex/browser`) live in `scripts/package.json` and are only installed in the Railway container, so the test stubs them at the loader level via `Module._load`. All 147 existing relay tests pass; 6 new assertions added.

vercel · 2026-04-29T03:07:07Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
worldmonitor	Ready	Preview, Comment	Apr 29, 2026 3:09am

…429 (koala73#3478) * fix(relay): prevent unbounded recursion in sendTelegram on sustained 429 The 429 handler in sendTelegram() called itself unconditionally with no retry counter. Under sustained Telegram rate limiting, this could recurse indefinitely and crash the Railway relay process. Changes: - Add _retryCount parameter (default 0) to sendTelegram() - Add guard: if (_retryCount ?? 0) >= 1, log and return false immediately - Pass (_retryCount ?? 0) + 1 on the recursive 429 call (single retry only) - Non-429 paths (200, 400, 403, 401) are unaffected Testing: - Add regression test (static analysis of source, matching repo patterns) - All 4 new assertions pass: param signature, guard + return false, incremented recursive call, single call site - All 27 existing relay tests continue to pass Fixes koala73#3060 * fix(relay): address review feedback on sendTelegram 429 guard - Drop redundant `?? 0` — the `_retryCount = 0` default already guarantees the value is a number inside the function body. - Cancel the 429 response body on the bail path. Without this the undici socket can be held open until the body stream is GC'd, which under sustained rate limiting could pressure the connection pool. - Replace the static-source-text test with a proper integration test that invokes `sendTelegram` against a mocked `globalThis.fetch`. The new test asserts call count and return value across 200, 429→200, sustained 429 (bail), 403, and 401, plus body-cancel hygiene on the bail path. Source formatting changes that don't alter behaviour will no longer break it. Required surgery in the relay module: - Wrap the SIGTERM handler + `subscribe()` invocation in `if (require.main === module)` so requiring the script from a test does not start the poll loop. - Export `{ sendTelegram }` for direct invocation in tests. The relay's third-party deps (`resend`, `convex/browser`) live in `scripts/package.json` and are only installed in the Railway container, so the test stubs them at the loader level via `Module._load`. All 147 existing relay tests pass; 6 new assertions added. --------- Co-authored-by: Elie Habib <[email protected]>

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

greptile-apps Bot reviewed Apr 27, 2026

View reviewed changes

koala73 merged commit a750cfb into koala73:main Apr 29, 2026
9 of 10 checks passed

vercel Bot deployed to Preview April 29, 2026 03:09 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(relay): prevent unbounded recursion in sendTelegram on sustained 429#3478

fix(relay): prevent unbounded recursion in sendTelegram on sustained 429#3478
koala73 merged 2 commits into
koala73:mainfrom
fuleinist:fix/sendTelegram-unbounded-recursion-429

fuleinist commented Apr 27, 2026

Uh oh!

vercel Bot commented Apr 27, 2026

Uh oh!

greptile-apps Bot commented Apr 27, 2026

Uh oh!

greptile-apps Bot Apr 27, 2026

Uh oh!

greptile-apps Bot Apr 27, 2026

Uh oh!

greptile-apps Bot Apr 27, 2026

Uh oh!

vercel Bot commented Apr 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

fuleinist commented Apr 27, 2026

Summary

Problem

Solution

Changes

Testing

Uh oh!

vercel Bot commented Apr 27, 2026

Uh oh!

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

vercel Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel Bot commented Apr 29, 2026 •

edited

Loading