run(): a failed polling pump leaves the process alive and silent (#1351)

doc/api.md

@@ -437,6 +437,7 @@ bound for long-lived bots that talk to many distinct chats.
 
 | Method | Params | Returns | Description |
 | --- | --- | --- | --- |
+| `close` | - | void | Close the client, but only if this store opened it (a passed-in `client` and the shared `redis` are the caller's / runtime's). Closing ends this store's life - a later use throws and you construct a new one. With a shared or caller-supplied client this is a no-op. |
 | `delete` | `key`: string | Promise<void> | - |
 | `read` | `key`: string | Promise<string \| undefined> | The stored string for `key`, or `undefined` when there is none. |
 | `touch` | `key`: string, `ttlSeconds`: number | Promise<void> | Refresh a key's expiry without rewriting it - what the middleware calls when an update changed nothing, so an active chat is not evicted mid-conversation. |
@@ -1109,12 +1110,13 @@ out, whether the loop stopped or threw.
 A fatal poll-stop is also written to stderr before being re-thrown: a bare
 rejection can be dropped (fire-and-forget, or a swallowing `.catch`), which
 would leave the process alive but no longer polling - the silent hang #1350
-describes.
+describes. Pass `exitOnError: true` to also exit the process non-zero after
+teardown, so a supervisor restarts the bot instead of relying on the caller.
 
 | Param | Type |
 | --- | --- |
 | `bot` | [Bot](#bot) |
-| `options?` | [LongPollOptions](#longpolloptions) |
+| `options` | [RunOptions](#runoptions) |
 
 **Returns:** Promise<void>
 
@@ -1136,9 +1138,10 @@ comparison always inspects every position, so it leaks no information about
 ### `startWebhook()`
 
 Managed webhook runner: create a `node:http` webhook server, start listening,
-and resolve when it shuts down. Installs `SIGINT`/`SIGTERM` handlers that close
-the server for a graceful exit (cleaned up in a `finally`), mirroring `run()` for
-long polling. Rejects if the server fails (e.g. the port is in use).
+and resolve when it shuts down. Once listening, installs `SIGINT`/`SIGTERM`
+handlers that close the server for a graceful exit (cleaned up in a `finally`),
+mirroring `run()` for long polling. Rejects if the server fails to start (e.g.
+the port is in use).
 
 Shutdown cannot hang: `server.close()` waits for existing connections to end,
 so we also drop idle keep-alive sockets at once and force-close anything still
@@ -6344,6 +6347,7 @@ type RedisSessionStorageOptions = {
   client?: RedisClient;
   prefix?: string;
   ttlSeconds?: number;
+  url?: string;
 };
 ```
 
@@ -7211,6 +7215,14 @@ type RichTextUrl = {
 };
 ```
 
+### `RunOptions`
+
+```ts
+type RunOptions = [LongPollOptions](#longpolloptions) & {
+  exitOnError?: boolean;
+};
+```
+
 ### `SavePreparedInlineMessageParams`
 
 ```ts

examples/01-polling-bot.ts

@@ -47,4 +47,7 @@ bot.catch((err, ctx) => {
 });
 
 // `run()` resolves when the process receives SIGINT/SIGTERM and the loop drains.
-await run(bot);
+// `exitOnError` makes a fatal poll-stop exit non-zero (after logging) so a
+// supervisor (systemd, Docker, pm2, ...) restarts the bot instead of leaving a
+// live-but-silent process.
+await run(bot, { exitOnError: true });

examples/06-keyboards.ts

@@ -53,4 +53,4 @@ bot.on("callback_query", async (ctx) => {
   if (data) await ctx.reply(`You pressed: ${data}`);
 });
 
-await run(bot);
+await run(bot, { exitOnError: true });

examples/09-middleware.ts

@@ -60,4 +60,4 @@ bot.on("message", (ctx) => {
   return ctx.reply(`You've sent ${session.count} message(s) this session.`);
 });
 
-await run(bot);
+await run(bot, { exitOnError: true });

examples/12-conversation.ts

@@ -74,4 +74,4 @@ bot.on("message", (ctx) => {
   }
 });
 
-await run(bot);
+await run(bot, { exitOnError: true });

examples/16-sessions.ts

@@ -84,4 +84,4 @@ bot.on("message", async (ctx, next) => {
   await ctx.reply(`Thanks ${data.name}! Saved ${data.email}. Send /me to see it, /start to redo.`);
 });
 
-await run(bot);
+await run(bot, { exitOnError: true });

examples/17-callback-tracking.ts

@@ -160,4 +160,4 @@ bot.on("callback_query", async (ctx, next) => {
   await ctx.reply(`Ran ${pending.op} on ${pending.targets.join(", ")} (asked at ${pending.requestedAt}).`);
 });
 
-await run(bot);
+await run(bot, { exitOnError: true });

src/bun/redis-storage.ts

@@ -13,12 +13,14 @@
  * `./node` (a CI guard enforces it).
  */
 
-import { redis, type RedisClient } from "bun";
+import { RedisClient, redis } from "bun";
 import type { SessionStore, SessionWriteOptions } from "../core/session.js";
 
 export type RedisSessionStorageOptions = {
   /** Bun `RedisClient` to use. Defaults to Bun's shared `redis` (REDIS_URL / VALKEY_URL). */
   client?: RedisClient;
+  /** Connect a client the store owns (and closes on teardown) to this URL, instead of the shared `redis`. Ignored when `client` is given. */
+  url?: string;
   /** Prefix prepended to every key. Default `"session:"`. */
   prefix?: string;
   /**
@@ -30,25 +32,57 @@ export type RedisSessionStorageOptions = {
 
 export class RedisSessionStorage implements SessionStore {
   private readonly client: RedisClient;
+  /** True when this store opened the client itself, so `close()` may close it. */
+  private readonly owned: boolean;
+  /** Set once `close()` closed a client this store owned - the store is then spent. */
+  private closed = false;
   private readonly prefix: string;
   private readonly ttlSeconds?: number;
 
   constructor(options: RedisSessionStorageOptions = {}) {
-    this.client = options.client ?? redis;
+    this.owned = options.client === undefined && options.url !== undefined;
+    this.client =
+      options.client ?? (options.url !== undefined ? this.createClient(options.url) : redis);
     this.prefix = options.prefix ?? "session:";
     this.ttlSeconds = options.ttlSeconds;
   }
 
+  /** Construct the owned client for a `url`. A seam so tests can supply a fake. */
+  protected createClient(url: string): RedisClient {
+    return new RedisClient(url);
+  }
+
+  private open(): RedisClient {
+    if (this.closed) {
+      throw new Error("RedisSessionStorage: this store was closed; construct a new one");
+    }
+    return this.client;
+  }
+
+  /**
+   * Close the client, but only if this store opened it (a passed-in `client` and
+   * the shared `redis` are the caller's / runtime's). Closing ends this store's
+   * life - a later use throws and you construct a new one. With a shared or
+   * caller-supplied client this is a no-op.
+   */
+  close(): void {
+    if (this.owned && !this.closed) {
+      this.closed = true;
+      this.client.close();
+    }
+  }
+
   async read(key: string): Promise<string | undefined> {
-    return (await this.client.get(this.prefix + key)) ?? undefined;
+    return (await this.open().get(this.prefix + key)) ?? undefined;
   }
 
   async write(key: string, value: string, options?: SessionWriteOptions): Promise<void> {
+    const client = this.open();
     const k = this.prefix + key;
-    await this.client.set(k, value);
+    await client.set(k, value);
     const ttl = options?.ttlSeconds ?? this.ttlSeconds;
     if (ttl !== undefined) {
-      await this.client.expire(k, ttl);
+      await client.expire(k, ttl);
     }
   }
 
@@ -57,10 +91,10 @@ export class RedisSessionStorage implements SessionStore {
    * an update changed nothing, so an active chat is not evicted mid-conversation.
    */
   async touch(key: string, ttlSeconds: number): Promise<void> {
-    await this.client.expire(this.prefix + key, ttlSeconds);
+    await this.open().expire(this.prefix + key, ttlSeconds);
   }
 
   async delete(key: string): Promise<void> {
-    await this.client.del(this.prefix + key);
+    await this.open().del(this.prefix + key);
   }
 }

src/node/run.ts

@@ -14,6 +14,22 @@ import type { Bot } from "../core/bot.js";
 import type { LongPollOptions } from "../core/longpoll.js";
 import { withShutdownSignals } from "./signals.js";
 
+/** Write to stderr and resolve once the chunk is flushed - a following `process.exit()` would otherwise truncate it. */
+function writeStderr(line: string): Promise<void> {
+  return new Promise((resolve) => {
+    process.stderr.write(line, () => resolve());
+  });
+}
+
+export type RunOptions = LongPollOptions & {
+  /**
+   * On a fatal poll-stop, exit the process with a non-zero code (after teardown
+   * and the stderr log) so a supervisor restarts the bot. Default false - `run`
+   * re-throws instead, leaving the exit policy to the caller.
+   */
+  exitOnError?: boolean;
+};
+
 /**
  * Start the bot's long-poll loop and resolve when it stops. Installs
  * `SIGINT`/`SIGTERM` handlers that trigger `bot.stop()` for a clean shutdown,
@@ -25,24 +41,40 @@ import { withShutdownSignals } from "./signals.js";
  * A fatal poll-stop is also written to stderr before being re-thrown: a bare
  * rejection can be dropped (fire-and-forget, or a swallowing `.catch`), which
  * would leave the process alive but no longer polling - the silent hang #1350
- * describes.
+ * describes. Pass `exitOnError: true` to also exit the process non-zero after
+ * teardown, so a supervisor restarts the bot instead of relying on the caller.
  */
-export async function run(bot: Bot, options?: LongPollOptions): Promise<void> {
+export async function run(bot: Bot, options: RunOptions = {}): Promise<void> {
+  const { exitOnError = false, ...pollOptions } = options;
   // Only a call that actually owns the pump may close. `startPolling` refuses
   // when another run is already active (as a rejection - it is async - so the
   // loser cannot be told apart after the fact); checking first is what keeps this
   // call from closing stores under the run that is still using them.
   const owned = !bot.isRunning();
+  let failed = false;
   try {
     return await withShutdownSignals(
       () => bot.stop(),
-      () => bot.startPolling(undefined, options),
+      () => bot.startPolling(undefined, pollOptions),
     );
   } catch (err) {
-    // Never let a fatal poll-stop be silent (see the doc comment above).
-    process.stderr.write(`node-telegram-bot-api: polling stopped on a fatal error: ${String(err)}\n`);
+    // Only the call that owns the pump reports and acts on the stop; a call that
+    // lost the "already running" race just re-throws to its own caller - it must
+    // not log a misleading "polling stopped" line or exit a healthy pump's process.
+    if (owned) {
+      failed = true;
+      // Await the flush so the exit below can't truncate this diagnostic.
+      await writeStderr(`node-telegram-bot-api: polling stopped on a fatal error: ${String(err)}\n`);
+    }
     throw err;
   } finally {
-    if (owned) await bot.close();
+    try {
+      if (owned) await bot.close();
+    } finally {
+      // Nested so it runs even when teardown throws: `exitOnError` must not be
+      // defeated by the very stuck resource it exists to escape. Gated on
+      // `failed` (which implies `owned`), so a losing double-run never exits.
+      if (failed && exitOnError) process.exit(1);
+    }
   }
 }

test/unit/bun/redis-storage.test.ts

@@ -8,12 +8,13 @@ import { RedisSessionStorage } from "../../../src/bun/redis-storage.js";
 // Node runner skips it; `bun test test/unit` runs it.
 
 /** Minimal in-memory stand-in for the RedisClient methods the store uses. */
-function fakeRedis(): RedisClient & { store: Map<string, string>; expires: Map<string, number> } {
+function fakeRedis(): RedisClient & { store: Map<string, string>; expires: Map<string, number>; closes: number } {
   const store = new Map<string, string>();
   const expires = new Map<string, number>();
   const client = {
     store,
     expires,
+    closes: 0,
     async get(key: string): Promise<string | null> {
       return store.has(key) ? (store.get(key) as string) : null;
     },
@@ -28,12 +29,29 @@ function fakeRedis(): RedisClient & { store: Map<string, string>; expires: Map<s
       expires.set(key, seconds);
       return 1;
     },
+    close(): void {
+      client.closes += 1;
+    },
   };
-  return client as unknown as RedisClient & { store: Map<string, string>; expires: Map<string, number> };
+  return client as unknown as RedisClient & { store: Map<string, string>; expires: Map<string, number>; closes: number };
 }
 
 const envelope = JSON.stringify({ v: 1, data: { n: 1 } });
 
+/**
+ * A store that takes the `{ url }` (owned) path but whose owned client is `client`.
+ * `createClient` is overridden via a closure (not an instance field), so the fake
+ * is available during `super()` - a field initializer would run too late.
+ */
+function ownedStore(client: RedisClient): RedisSessionStorage {
+  class Owned extends RedisSessionStorage {
+    protected override createClient(): RedisClient {
+      return client;
+    }
+  }
+  return new Owned({ url: "redis://fake" });
+}
+
 describe("RedisSessionStorage", () => {
   test("prefixes keys and round-trips the encoded string", async () => {
     const client = fakeRedis();
@@ -78,4 +96,29 @@ describe("RedisSessionStorage", () => {
     await new RedisSessionStorage({ client, ttlSeconds: 3600 }).write("k", envelope, { ttlSeconds: 60 });
     assert.equal(client.expires.get("session:k"), 60);
   });
+
+  test("close() does not close an injected client (the caller owns it) and the store stays usable", async () => {
+    const client = fakeRedis();
+    const store = new RedisSessionStorage({ client });
+    store.close();
+    assert.equal(client.closes, 0); // not ours to close
+    await store.write("k", envelope); // still usable
+    assert.equal(await store.read("k"), envelope);
+  });
+
+  test("a url-owned client is closed on close(), idempotently, and reuse throws", async () => {
+    const client = fakeRedis();
+    const store = ownedStore(client);
+
+    await store.write("k", envelope); // usable while open
+    assert.equal(await store.read("k"), envelope);
+
+    store.close();
+    assert.equal(client.closes, 1); // owned -> closed
+    store.close();
+    assert.equal(client.closes, 1); // idempotent, not double-closed
+
+    await assert.rejects(store.read("k"), /was closed/);
+    await assert.rejects(store.write("k", envelope), /was closed/);
+  });
 });