fix: per-bot queue fixes — outer loop transient retry, getFileInfo logging, test file, safety net comment, empty-bot guard
- Add MAX_OUTER_RETRIES constant and transientAttempts counter for outer-loop retry - Restore getFileInfo transient retry logging with bot identity and fileId - Create test/bot-pool.test.ts with 4 tests for core BotPool behavior - Add empty-bots guard in selectBot() returning null - Add safety net comment and improved logging for outer 429 catch Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -48,6 +48,7 @@ const isTransientError = (error: unknown): boolean => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
const MAX_TRANSIENT_RETRIES = 3;
|
const MAX_TRANSIENT_RETRIES = 3;
|
||||||
|
const MAX_OUTER_RETRIES = 10;
|
||||||
const TELEGRAM_API_TIMEOUT_MS = 120_000;
|
const TELEGRAM_API_TIMEOUT_MS = 120_000;
|
||||||
const PER_BOT_CONCURRENCY = 1;
|
const PER_BOT_CONCURRENCY = 1;
|
||||||
|
|
||||||
@@ -83,6 +84,8 @@ export class BotPool implements ITelegramService {
|
|||||||
* or in the skip set.
|
* or in the skip set.
|
||||||
*/
|
*/
|
||||||
private selectBot(skipIndexes?: Set<number>): BotEntry | null {
|
private selectBot(skipIndexes?: Set<number>): BotEntry | null {
|
||||||
|
if (this.bots.length === 0) return null;
|
||||||
|
|
||||||
let best: BotEntry | null = null;
|
let best: BotEntry | null = null;
|
||||||
let bestPending = Infinity;
|
let bestPending = Infinity;
|
||||||
|
|
||||||
@@ -133,9 +136,10 @@ export class BotPool implements ITelegramService {
|
|||||||
): Promise<ForwardResult> {
|
): Promise<ForwardResult> {
|
||||||
let lastError: unknown;
|
let lastError: unknown;
|
||||||
const attemptedIndexes = new Set<number>();
|
const attemptedIndexes = new Set<number>();
|
||||||
|
let transientAttempts = 0;
|
||||||
|
|
||||||
// Outer retry loop — up to 10 attempts across all bots
|
// Outer retry loop — up to MAX_OUTER_RETRIES attempts across all bots
|
||||||
for (let attempt = 0; attempt < 10; attempt++) {
|
for (let attempt = 0; attempt < MAX_OUTER_RETRIES; attempt++) {
|
||||||
const bot = this.selectBot(attemptedIndexes);
|
const bot = this.selectBot(attemptedIndexes);
|
||||||
|
|
||||||
if (!bot) {
|
if (!bot) {
|
||||||
@@ -221,7 +225,19 @@ export class BotPool implements ITelegramService {
|
|||||||
const retryAfterMatch = errorStr.match(/retry after (\d+)/i);
|
const retryAfterMatch = errorStr.match(/retry after (\d+)/i);
|
||||||
|
|
||||||
if (retryAfterMatch) {
|
if (retryAfterMatch) {
|
||||||
// Bot was rate-limited — already marked, try next bot
|
// 429 catch in outer block: serves as a safety net for errors that
|
||||||
|
// contain "retry after N" wording but were rethrown from the inner
|
||||||
|
// queue task's fallback path (e.g., non-429 errors with similar text).
|
||||||
|
logger.warn('Retry-after pattern caught in outer loop (safety net)', {
|
||||||
|
fileName,
|
||||||
|
error: errorStr,
|
||||||
|
});
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Transient error at the queue level — retry on next bot
|
||||||
|
if (transientAttempts < MAX_TRANSIENT_RETRIES && isTransientError(error)) {
|
||||||
|
transientAttempts++;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -267,6 +283,10 @@ export class BotPool implements ITelegramService {
|
|||||||
}
|
}
|
||||||
if (retry < MAX_TRANSIENT_RETRIES && isTransientError(error)) {
|
if (retry < MAX_TRANSIENT_RETRIES && isTransientError(error)) {
|
||||||
const backoffMs = Math.min(1000 * 2 ** (retry + 1), 5_000);
|
const backoffMs = Math.min(1000 * 2 ** (retry + 1), 5_000);
|
||||||
|
logger.warn(
|
||||||
|
`Transient error getting file info, retrying bot ${bot.token.slice(0, 8)}... (${retry + 1}/${MAX_TRANSIENT_RETRIES})`,
|
||||||
|
{ telegramFileId, error: errorStr, backoffMs },
|
||||||
|
);
|
||||||
await sleep(backoffMs);
|
await sleep(backoffMs);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,131 @@
|
|||||||
|
import { afterEach, beforeEach, describe, expect, it, mock } from 'bun:test';
|
||||||
|
|
||||||
|
process.env.BOT_TOKEN = 'bot1:token';
|
||||||
|
process.env.ADDITIONAL_BOT_TOKENS = 'bot2:token,bot3:token';
|
||||||
|
process.env.STORAGE_CHANNEL_ID = '-1001234567890';
|
||||||
|
process.env.BASE_URL = 'https://example.com';
|
||||||
|
process.env.DATABASE_URL = 'sqlite://test.db';
|
||||||
|
process.env.PORT = '3000';
|
||||||
|
|
||||||
|
// Track mock queue instances for per-bot assertions
|
||||||
|
const queueInstances: Array<{
|
||||||
|
concurrency: number;
|
||||||
|
add: ReturnType<typeof mock>;
|
||||||
|
pending: number;
|
||||||
|
size: number;
|
||||||
|
}> = [];
|
||||||
|
|
||||||
|
// Mock PQueue so we can verify concurrency
|
||||||
|
const mockAdd = mock(function addFn(this: any, fn: () => Promise<any>) {
|
||||||
|
return Promise.resolve().then(() => fn());
|
||||||
|
});
|
||||||
|
|
||||||
|
mock.module('p-queue', () => {
|
||||||
|
return {
|
||||||
|
default: mock(function MockQueue(this: any, opts?: { concurrency?: number }) {
|
||||||
|
const instance = {
|
||||||
|
concurrency: opts?.concurrency ?? 1,
|
||||||
|
add: mockAdd,
|
||||||
|
pending: 0,
|
||||||
|
size: 0,
|
||||||
|
};
|
||||||
|
queueInstances.push(instance);
|
||||||
|
return instance;
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
// Mock Telegraf — use a class so `new Telegraf(token)` works correctly
|
||||||
|
const mockTelegramInstances: Record<
|
||||||
|
string,
|
||||||
|
{
|
||||||
|
token: string;
|
||||||
|
sendDocument: ReturnType<typeof mock>;
|
||||||
|
sendPhoto: ReturnType<typeof mock>;
|
||||||
|
getFile: ReturnType<typeof mock>;
|
||||||
|
}
|
||||||
|
> = {};
|
||||||
|
|
||||||
|
class MockTelegraf {
|
||||||
|
token: string;
|
||||||
|
telegram: {
|
||||||
|
token: string;
|
||||||
|
sendDocument: ReturnType<typeof mock>;
|
||||||
|
sendPhoto: ReturnType<typeof mock>;
|
||||||
|
getFile: ReturnType<typeof mock>;
|
||||||
|
};
|
||||||
|
|
||||||
|
constructor(token: string) {
|
||||||
|
this.token = token;
|
||||||
|
this.telegram = {
|
||||||
|
token,
|
||||||
|
sendDocument: mock(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
message_id: 1,
|
||||||
|
document: { file_id: `file_${token}`, file_unique_id: `uniq_${token}` },
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
sendPhoto: mock(() =>
|
||||||
|
Promise.resolve({
|
||||||
|
message_id: 1,
|
||||||
|
photo: [{ file_id: `photo_${token}`, file_unique_id: `photo_uniq_${token}` }],
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
getFile: mock(() =>
|
||||||
|
Promise.resolve({ file_size: 100, mime_type: 'text/plain', file_path: 'path' }),
|
||||||
|
),
|
||||||
|
};
|
||||||
|
mockTelegramInstances[token] = this.telegram;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
mock.module('telegraf', () => ({
|
||||||
|
Telegraf: MockTelegraf,
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('BotPool', () => {
|
||||||
|
let BotPool: typeof import('../src/infrastructure/telegram/bot-pool').BotPool;
|
||||||
|
let botPool: import('../src/infrastructure/telegram/bot-pool').BotPool;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
mockAdd.mockClear();
|
||||||
|
queueInstances.length = 0;
|
||||||
|
for (const token of Object.keys(mockTelegramInstances)) {
|
||||||
|
const tg = mockTelegramInstances[token];
|
||||||
|
if (tg) {
|
||||||
|
tg.sendDocument?.mockClear();
|
||||||
|
tg.getFile?.mockClear();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const mod = await import('../src/infrastructure/telegram/bot-pool');
|
||||||
|
BotPool = mod.BotPool;
|
||||||
|
botPool = new BotPool();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
// No module cache cleanup needed — Bun handles import caching correctly
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should have correct bot count', () => {
|
||||||
|
expect(botPool.size).toBe(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should have correct effective concurrency', () => {
|
||||||
|
// 3 bots * 1 concurrency per bot
|
||||||
|
expect(botPool.getEffectiveConcurrency()).toBe(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should forward files through the queue', async () => {
|
||||||
|
const result = await botPool.forwardToStorage(Buffer.from('test data'), 'test.txt', 'document');
|
||||||
|
expect(result.telegramFileId).toBeDefined();
|
||||||
|
expect(result.storageMessageId).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should use per-bot queues with concurrency=1', () => {
|
||||||
|
// Each bot gets its own PQueue instance with concurrency=1
|
||||||
|
expect(queueInstances.length).toBe(3);
|
||||||
|
for (const qi of queueInstances) {
|
||||||
|
expect(qi.concurrency).toBe(1);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user