longPoll: a 409 Conflict permanently stops the polling loop (#1350)

[yagop]

Closes #1350

• T1 longPoll: treat a 409 poll conflict as recoverable - back off (its own longer delay) and resume, bounded by a max consecutive-conflict count that then throws; log + onError, counter resets on any successful poll
• T2 Unit tests: resume after a transient 409, throw after N consecutive conflicts, and respect retry: false
• T3 Regenerate doc/api.md for the new LongPollOptions fields
• T4 run(): a fatal poll-stop must not be silent - the managed runner surfaces it to stderr before re-throwing, so a dropped/late-observed rejection can't leave the process alive with dead polling; unit test
• T5 startWebhook: cap graceful shutdown so an idle keep-alive (or stuck) connection can't leave a stopped-but-never-exiting process - drop idle connections, force-close past shutdownTimeoutMs; unit test + docs
• T6 Bump the documented minimum to Node 18.2 (engines, docs) - closeIdleConnections/closeAllConnections need 18.2, so >=18 could still hang (Codex P2)
• T7 Make startWebhook's shutdown idempotent: a shuttingDown guard schedules a single force-timer, so repeated signals can't leak a timer that fires after resolve (Codex)
• T8 Fix the off-by-one wording in the conflict-retry comment (N retries, throw on conflict N+1) (Codex)
• T9 Close test gaps: startWebhook signal-driven shutdown (idle close, force-close on timeout, repeated signals, handler cleanup) and longPoll conflict counter resets on a successful (incl. empty) poll (Codex)
• T10 Simplify: trim the verbose polling/shutdown comments and JSDoc, collapse the conflict/plain log into one line (no behavior change)
• T11 Refactor: move 409 classification (isPollConflict + HTTP_STATUS_CONFLICT) into errors.ts next to isTransientError, so all error taxonomy lives in one module
• T12 Refactor: extract the shared SIGINT/SIGTERM install-run-cleanup into src/node/withShutdownSignals, used by both run() and startWebhook() (one place for the signal set)
• T13 Refactor: extract gracefulClose(server, timeoutMs) in server.ts (close + drop idle + bounded force-close) so shutdown mechanics are self-contained and directly unit-testable
• T14 Handle request aborts from a forced shutdown: createWebhookServer now .catches the handler promise, so a socket destroyed mid-body (ECONNRESET from readBody) can't become an unhandled rejection that crashes the process mid-shutdown; regression test (Codex P2)
• T15 Install startWebhook's shutdown handlers only after the server is listening, so an early signal can't call server.close() on a not-yet-listening server (ERR_SERVER_NOT_RUNNING) - no try/catch needed (Copilot)
• T16 longPoll: reset the conflict counter on a non-conflict transient error so the bound is on truly consecutive 409s (409 -> 5xx -> 409 no longer counts as two); test (Copilot)
• T17 Stop mutating process-global signal listeners in the shutdown test: snapshot + diff to invoke startWebhook's own handler directly, avoiding flakiness under concurrent test runs (Copilot)

[yagop]

😺 Michi — all tasks done, CI green, marked ready for review.

Done

• T1 (702d821): longPoll treats a 409 as recoverable for polling - it backs off with its own longer delay (conflictRetryDelayMs, default 5s) and resumes without advancing the offset, bounded by maxConflictRetries (default 10) consecutive conflicts before it throws. The streak resets on any successful poll; onError + a log() line make it observable. Truly fatal errors (401/400/404) still throw and stop the loop, unchanged.
• T2 (23636ab): unit tests - resume after a 409 then yield (offset not advanced), throw after N consecutive 409s (with onError count), and 409 fatal under retry: false.
• T3 (fa1c6b3): regenerated doc/api.md for the two new LongPollOptions fields.

CI: GitHub Actions (typecheck, build, unit on Bun + Node 22/24/26) and AppVeyor all pass; E2E skipped by design. Full local gate (npm run check, 225 tests) green.

Waiting for your merge. 🐾

[yagop]

😺 Michi — added T4 for the second half of #1350 (the "process lingers dead" symptom); CI green.

Investigation: the core pump does not itself hang — verified empirically: startPolling rejects in ~4ms on a fatal poll error with isRunning() -> false, and run() removes its SIGINT/SIGTERM listeners + calls bot.close() in a finally, then re-throws (process exits once the rejection is observed). A process lingering dead after a fatal stop therefore requires an external open handle (a session-store pool, an HTTP server) keeping Node alive while polling is dead. Two fixes:

• T1 fixes the actual incident: a stray getUpdates (debug curl / overlapping deploy) returns 409, which no longer stops polling — it backs off and resumes, so nothing ever strands.
• T4 (77833cd) closes the residual for any fatal poll-stop (conflict bound exceeded, 401, ...): the managed run() runner now surfaces it to stderr before re-throwing, so a dropped or late-observed rejection can't leave the bot silently not-polling. The error is still re-thrown unchanged for awaiting callers. Unit test covers both the fatal (writes + re-throws) and clean-stop (silent) paths.

Note: preventing the lingering itself is inherently the caller's open resource (or T1) — a library can't force-close handles it doesn't own — so T4 makes the failure unmissable rather than force-killing the process. Happy to instead add an onStop(err) hook or an opt-in exit-on-fatal if you'd prefer a different policy.

CI: all GitHub Actions + AppVeyor green; E2E skipped by design. Full local gate (npm run check, 227 tests) + Node 22/24/26 unit run green.

Waiting for your merge. 🐾

[yagop]

😺 Michi — added T5, the real "not stopping" bug, found by tracing the mitubo-bot commits; CI green.

The production hang wasn't the poll loop — it was webhook-server shutdown hanging on a stuck connection during Fly scale-to-zero idle-exit (fixed downstream in mitubo-bot 1a09e2a2 with a server.stop() timeout). ntba's own managed startWebhook (src/node/server.ts) had the identical gap: node:http's server.close() waits for every existing connection to end, so one idle keep-alive socket keeps the "close" event — and the startWebhook promise — pending forever. On SIGTERM the process stops serving but never exits: "no work, but not stopping."

T5 (ed16561): shutdown now drops idle connections immediately (closeIdleConnections()) and force-closes anything still busy past shutdownTimeoutMs (new option, default 10s, mirroring mitubo-bot) via closeAllConnections(), both optional-chained for cross-runtime safety. Tests: close() completes despite an idle keep-alive socket, and startWebhook rejects + cleans up its signal handlers on a listen error. Docs regenerated for the new option.

CI: all GitHub Actions + AppVeyor green; E2E skipped by design. Full local gate (npm run check, 229 tests) + Node 22/24/26 unit run green.

Waiting for your merge. 🐾

[yagop]

😺 Michi — applied the Codex (gpt-6-astra) review; CI green.

• [P2] Shutdown could still hang on Node 18.0/18.1 (closeIdleConnections/closeAllConnections are 18.2+): T6 (ab28496) bumps the documented minimum to Node 18.2 (engines: >=18.2.0, CLAUDE.md), so the helpers are guaranteed on Node; the optional-chaining stays only as a safe no-op for non-Node runtimes, now noted in the comment.
• Repeated signals leave a timer behind: T7 (8b16bba) makes stop() idempotent via a shuttingDown guard, so a second signal can't schedule a second force-timer that outlives resolve.
• Off-by-one in the conflict-retry comment: T8 (35ecef0) reworded to "retry on up to N consecutive conflicts; the next one throws."
• Test-coverage gaps: T9 (b462eca) adds a longPoll test that a successful (empty) poll resets the conflict counter, and a startWebhook test that drives the real signal path — installs the handler, drops an idle keep-alive connection on SIGTERM, is idempotent under a repeated signal, and cleans up its listeners.

One deliberate scope note: I did not add an explicit force-closeAllConnections-on-timeout test — it needs a reliably-stuck (never-completing) socket, which is flaky to construct deterministically. The timeout path is exercised (the timer is created/cleared) and the force-close is a single guarded call; happy to add a child-process integration test if you'd prefer belt-and-suspenders.

CI: all GitHub Actions + AppVeyor green; E2E skipped by design. Full local gate (npm run check, 231 tests) + Node 22/24/26 + typecheck:test green.

Waiting for your merge. 🐾

[Copilot]

🟡 Changes recommended

There are verified edge-case correctness issues (conflict retry streak semantics and a potential ERR_SERVER_NOT_RUNNING crash path in gracefulClose) plus a test that mutates global signal handlers in a way that can be flaky under concurrent test execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses Telegram getUpdates polling resilience by treating 409 Conflict as a recoverable polling-specific condition (with bounded retries + backoff), and strengthens Node runner shutdown behavior to avoid silent dead processes or hanging webhook shutdowns.

Changes:

• Update longPoll to retry on 409 Conflict with dedicated delay and a max consecutive-conflict bound; add isPollConflict/HTTP_STATUS_CONFLICT to centralize error taxonomy.
• Make managed Node runners (run, startWebhook) more operationally robust via shared shutdown-signal handling and non-hanging webhook shutdown (gracefulClose).
• Add unit tests covering 409 conflict recovery/bounds, fatal poll-stop surfacing to stderr, and webhook shutdown edge cases.

File summaries

File	Description
test/unit/server.test.ts	Adds webhook shutdown and signal-handling/idempotency tests.
test/unit/run.test.ts	Tests that run() surfaces fatal poll-stops to stderr and rethrows.
test/unit/longpoll.test.ts	Adds coverage for 409 conflict retry/backoff/bounds and counter reset.
src/node/signals.ts	Introduces shared SIGINT/SIGTERM install/remove helper for Node runners.
src/node/server.ts	Adds gracefulClose, makes webhook handler promise rejection safe, uses shared shutdown-signal plumbing.
src/node/run.ts	Uses shared shutdown-signal plumbing; logs fatal poll-stop to stderr before rethrow.
src/core/longpoll.ts	Implements 409 conflict retry strategy + new LongPollOptions fields.
src/core/errors.ts	Adds HTTP 409 constant and isPollConflict classifier alongside isTransientError.
package.json	Bumps Node engine minimum to >=18.2.0.
doc/api.md	Regenerates API docs for new exports/options and updated docstrings.
CLAUDE.md	Updates documented minimum Node version to 18.2+.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yagop]

😺 Michi — applied the Copilot review; CI green.

• [server.ts] gracefulClose could throw ERR_SERVER_NOT_RUNNING if a signal arrived before listen() finished. T15 (bac47f9) fixes it structurally rather than with a try/catch: startWebhook now listens first (phase 1) and only installs the SIGINT/SIGTERM handlers once the server is actually listening (phase 2). So stop() can never run server.close() on a not-yet-listening server, and gracefulClose stays guard-free. A signal during the brief startup window keeps Node's default (exit), which is correct - nothing is listening to close.
• [longpoll.ts] conflict bound wasn't truly "consecutive": a non-conflict transient between two 409s (409 -> 5xx -> 409) still accumulated. T16 (3a4783c) resets the counter on any non-conflict transient, so only genuinely consecutive 409s count toward maxConflictRetries; added a test (409 -> 500 -> 409 with maxConflictRetries: 1 no longer throws).
• [test] global signal-listener mutation: T17 (5fcc057) stops removeAllListeners/emit on the process; the test now snapshots the SIGTERM listeners, lets startWebhook add its own, and invokes that handler directly - no global state touched, so it can't disturb other tests or the runner.

CI: all GitHub Actions + AppVeyor green; E2E skipped by design. Full local gate (npm run check, 233 tests) + Node 22/24/26 + typecheck:test green.

Waiting for your merge. 🐾