feat: Implement LRU-Based Account Rotation for Telegram and Twitter with Redis Integration #11
Conversation
…gration - Added TelegramAccountManager and TwitterAccountManager for managing account credentials and usage tracking. - Refactored fetchTelegramMessages and fetchHomeTimeline to utilize account rotation logic. - Introduced batch API key usage tracking for multiple accounts. - Created demo scripts for simulating account rotation for both Telegram and Twitter. - Updated moveEnvToRedis script to handle new environment variable mappings. - Enhanced error handling and logging for better debugging and monitoring.
WalkthroughImplements per-account rotation for Telegram and Twitter: new Redis-backed base and platform account managers, per-account credential handling and client caching, updated APIs to select/mark least-recently-used accounts, adjusted function signatures, batch usage utilities, and rotation demo/test scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Cron as Cron (5m)
participant App as index.ts
participant Mgr as TelegramAccountManager
participant Cache as ClientCache
participant TGClient as TelegramClient
participant API as Telegram API
Cron->>App: Trigger rotation cycle
App->>Mgr: getEarliestUsedAccount()
Mgr-->>App: TelegramAccount
App->>Cache: get/create client(account)
Cache-->>App: TelegramClient
App->>TGClient: ensure logged in / start if needed
App->>API: Api.messages.GetHistory(peer: entity, limit: 10)
API-->>App: messages
App->>Mgr: markAccountAsUsed(accountId)
App-->>Cron: log results
sequenceDiagram
autonumber
actor Cron as Cron (5m)
participant Api as twitterApi.ts
participant Mgr as TwitterAccountManager
participant TW as Twitter API
Cron->>Api: fetchHomeTimeline(seenIds)
Api->>Mgr: getEarliestUsedAccount()
Mgr-->>Api: TwitterAccount
Api->>TW: GET HomeTimeline (headers from account.credentials)
TW-->>Api: timeline tweets
Api->>Mgr: markAccountAsUsed(accountId)
Api-->>Cron: log tweets count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/utils/redisUtils.ts (1)
5-17: Unify Redis config and use isOpen instead of a sticky flag.Hard-coded localhost and a manual
redisConnectedboolean are brittle and inconsistent withmoveEnvToRedis.ts(which usesprocess.env.REDIS_URL). Use env URL with a fallback and rely onredisClient.isOpen.-// Singleton Redis client -const redisClient = createClient({ url: 'redis://localhost:6379' }); -let redisConnected = false; +// Singleton Redis client +const redisUrl = process.env.REDIS_URL ?? 'redis://localhost:6379'; +const redisClient = createClient({ url: redisUrl }); async function ensureRedisConnected() { - if (!redisConnected) { - await redisClient.connect(); - redisConnected = true; - } + if (!redisClient.isOpen) { + await redisClient.connect(); + } }src/fetchTelegramMessages.ts (1)
45-53: Don’t log raw message content by default.Printing each message body risks PII leakage. Gate under a debug flag or remove.
- const formatted = { id, content, channelId }; - out.push(formatted); - console.log(formatted); + const formatted = { id, content, channelId }; + out.push(formatted); + if (process.env.DEBUG_TELEGRAM === '1') { + console.log(formatted); + }src/twitterApi.ts (1)
189-201: Avoid side effects on import; gate main() and cron behind CLI checkRunning main() and scheduling cron at module load makes this file unusable as a library and risks duplicate jobs if imported elsewhere. Guard with a CLI entrypoint.
Apply this diff:
-// Run once at startup -main(); - -// Schedule to run every 5 minutes - automatically rotates to earliest used account -cron.schedule('*/5 * * * *', async () => { - console.log('Refetching Twitter timeline with account rotation...'); - try { - const timeline = await fetchHomeTimeline(); - console.log(`Fetched ${timeline.length} tweets`); - } catch (err) { - console.error('Scheduled Twitter timeline fetch failed:', err); - } -}); +// Run only when executed directly +if (require.main === module) { + // Run once at startup + main(); + // Schedule to run every 5 minutes - automatically rotates to earliest used account + cron.schedule('*/5 * * * *', async () => { + console.log('Refetching Twitter timeline with account rotation...'); + try { + const timeline = await fetchHomeTimeline(); + console.log(`Fetched ${timeline.length} tweets`); + } catch (err) { + console.error('Scheduled Twitter timeline fetch failed:', err); + } + }); +}
🧹 Nitpick comments (28)
src/utils/redisUtils.ts (1)
96-131: Make batch usage retrieval concurrent and robust (avoid per-key awaits; handle NaN).The current loop performs sequential
hGetAll. Pipeline withPromise.all(ormulti) and guard numeric parsing.export async function getBatchApiKeyUsage( accountIds: string[], platform: 'telegram' | 'twitter' ): Promise<Array<{ accountId: string; total_requests: number; last_request: string | null }>> { if (platform !== 'twitter' && platform !== 'telegram') { throw new Error('getBatchApiKeyUsage: platform must be "twitter" or "telegram"'); } - const results: Array<{ accountId: string; total_requests: number; last_request: string | null }> = []; - try { await ensureRedisConnected(); - for (const accountId of accountIds) { - if (!accountId?.trim()) continue; - - let key: string; - if (platform === 'twitter') { - key = `twitter_accounts:${accountId}`; - } else { - key = `telegram_accounts:${accountId}`; - } - - const data = await redisClient.hGetAll(key); - results.push({ - accountId, - total_requests: data.total_requests ? parseInt(data.total_requests) : 0, - last_request: data.last_request ? data.last_request : null, - }); - } + const prefix = platform === 'twitter' ? 'twitter_accounts:' : 'telegram_accounts:'; + const ids = accountIds.filter(id => id?.trim()); + const keys = ids.map(id => `${prefix}${id}`); + const replies = await Promise.all(keys.map(k => redisClient.hGetAll(k))); + return ids.map((accountId, i) => { + const data = replies[i] ?? {}; + const n = Number(data.total_requests); + return { + accountId, + total_requests: Number.isFinite(n) ? n : 0, + last_request: data.last_request ?? null, + }; + }); } catch (err) { console.error('getBatchApiKeyUsage: Redis operation failed:', err); } - return results; + return []; }src/utils/moveEnvToRedis.ts (4)
17-21: Exclude REDIS_URL from being written back to Redis.Storing the store’s connection string inside the store is unnecessary and risky. Exclude it alongside
ENCRYPTION_KEY.- const EXCLUDE = new Set(['ENCRYPTION_KEY']); + const EXCLUDE = new Set(['ENCRYPTION_KEY', 'REDIS_URL']);
28-32: Type the key map to prevent accidental key mismatches.Mark the map as readonly to tighten typings.
- const telegramKeyMap = { + const telegramKeyMap = { 'API_ID': 'TELEGRAM_API_ID', 'API_HASH': 'TELEGRAM_API_HASH', 'TG_CHANNEL': 'TELEGRAM_TG_CHANNEL' - }; + } as const;
51-61: Replace any[] with typed arrays (aligns with PR objective).Use
Record<string, string>[]for stronger type safety.- if (Object.keys(twitterAccount).length) { - let twitterArr: any[] = []; + if (Object.keys(twitterAccount).length) { + let twitterArr: Record<string, string>[] = []; const existing = await redisClient.get('twitter-accounts'); if (existing) { try { - twitterArr = JSON.parse(existing); + twitterArr = JSON.parse(existing) as Record<string, string>[]; } catch { } } twitterArr.push(twitterAccount); await redisClient.set('twitter-accounts', JSON.stringify(twitterArr)); }
62-72: Apply the same typing to Telegram accounts.- if (Object.keys(telegramAccount).length) { - let telegramArr: any[] = []; + if (Object.keys(telegramAccount).length) { + let telegramArr: Record<string, string>[] = []; const existing = await redisClient.get('telegram-accounts'); if (existing) { try { - telegramArr = JSON.parse(existing); + telegramArr = JSON.parse(existing) as Record<string, string>[]; } catch { } } telegramArr.push(telegramAccount); await redisClient.set('telegram-accounts', JSON.stringify(telegramArr)); }src/tests/rotationDemo.ts (1)
84-89: Avoid brittle string matching for control flow; prefer typed errors.Checking
error.message.includes('No Twitter accounts found')is fragile. HavetwitterAccountManagerthrow a dedicatedNoAccountsError(or error code) and test viainstanceofor a stablecode.src/tests/telegramRotationDemo.ts (1)
87-92: Prefer typed errors over string matching.Replace
error.message.includes('No Telegram accounts found')with a dedicated error type or code from the manager.src/twitterApi.ts (5)
16-16: Add request timeouts to external callsfetch calls have no timeout; a stuck connection can hang the job. Use AbortController with a configurable timeout.
-const res = await fetch(url, { method: "GET", headers }); +const controller = new AbortController(); +const to = setTimeout(() => controller.abort(), Number(process.env.HTTP_TIMEOUT_MS ?? 15000)); +let res: Response; +try { + res = await fetch(url, { method: "GET", headers, signal: controller.signal }); +} finally { + clearTimeout(to); +}-const response = await fetch(url, { - method: "POST", - headers, - body: JSON.stringify(body), -}); +const controller = new AbortController(); +const to = setTimeout(() => controller.abort(), Number(process.env.HTTP_TIMEOUT_MS ?? 15000)); +let response: Response; +try { + response = await fetch(url, { + method: "POST", + headers, + body: JSON.stringify(body), + signal: controller.signal, + }); +} finally { + clearTimeout(to); +}Additionally, declare once near the top if preferred:
// optionally at top-level const DEFAULT_HTTP_TIMEOUT_MS = Number(process.env.HTTP_TIMEOUT_MS ?? 15000);Also applies to: 104-110
36-36: Externalize brittle GraphQL queryIdHardcoding the queryId is fragile. Make it configurable and default to the current value.
-const queryId = "wEpbv0WrfwV6y2Wlf0fxBQ"; +const queryId = process.env.TW_HOME_TIMELINE_QID ?? "wEpbv0WrfwV6y2Wlf0fxBQ";Please verify the current valid queryId in your environment before deploy.
Also applies to: 101-101
62-62: Remove redundant fallbackseenTweetIds already defaults to [], so the extra || [] is unnecessary.
- seenTweetIds: seenTweetIds || [], + seenTweetIds,
171-174: Avoid logging PII in routine runsLogging Account info (screenName/userId) may be sensitive. Gate behind a DEBUG flag or redact.
Example:
if (process.env.DEBUG === '1') { console.log('Account info:', viewer); }
39-46: Guard for empty account pool and mark usage only after successful parsegetEarliestUsedAccount may throw when no accounts exist. Ensure clear error and avoid partial usage updates.
- Confirm twitterAccountManager.getEarliestUsedAccount() throws a descriptive error when the pool is empty.
- Current placement of markAccountAsUsed happens after response OK; good. Keep it that way to avoid penalizing failed calls.
Also applies to: 154-158
src/tests/testRotation.ts (2)
120-131: Prefer returning non-zero exit code without abrupt process.exit in shared test utilitiesIf this script is ever imported, process.exit(1) can terminate parent processes. Since this is mainly a CLI, it’s acceptable; consider returning a status and exiting only under the CLI guard.
Pattern:
if (require.main === module) { const ok = await main(); process.exit(ok ? 0 : 1); }
88-107: Reuse existing Redis connection to reduce overhead (optional)You spin up a dedicated Redis client to read twitter-accounts, while twitterAccountManager maintains its own client. For tests, it’s fine; consider delegating this check to the manager or sharing a client to reduce connection churn.
src/tests/testTelegramRotation.ts (3)
129-139: CLI exit vs. library importSame note as Twitter tests: exiting the process is fine for a CLI, but return a status from main() and only exit under the direct-execution guard.
30-36: Avoid printing decrypted channel names in shared logsChannel identifiers can be sensitive. Consider gating with DEBUG and/or redacting.
90-108: Reduce duplicate Redis connections (optional)Like the Twitter test, you create a new Redis client while telegramAccountManager manages one. It’s okay for a test, but you can reuse/ask the manager for a lightweight “exists” check.
src/services/telegramAccountManager.ts (4)
30-35: Make Redis connect idempotent under concurrencyTwo concurrent calls can both attempt connect() before isConnected flips, causing noisy errors. Track a connecting promise.
- private async ensureConnected(): Promise<void> { - if (!this.isConnected) { - await this.redisClient.connect(); - this.isConnected = true; - } - } + private connecting?: Promise<void>; + private async ensureConnected(): Promise<void> { + if (this.isConnected) return; + if (!this.connecting) { + this.connecting = (async () => { + await this.redisClient.connect(); + this.isConnected = true; + })().finally(() => { + this.connecting = undefined; + }); + } + await this.connecting; + }
43-53: Graceful handling when no accounts are presentThrowing is fine for callers expecting it; consider returning an empty array from fetchAllAccounts() and throwing only in getEarliestUsedAccount() with a clear message.
- if (!raw) { - throw new Error('No Telegram accounts found in Redis'); - } + if (!raw) { + return []; + } ... - if (accounts.length === 0) { - throw new Error('No valid Telegram accounts could be decrypted'); - } + if (accounts.length === 0) { + return []; + }Then in getEarliestUsedAccount():
if (accounts.length === 0) { throw new Error('No usable Telegram accounts found. Did you run moveEnvToRedis and set ENCRYPTION_KEY?'); }Also applies to: 86-91
112-115: Avoid noisy console logs in library codePrefer a passed-in logger or debug gating to keep services quiet in production.
71-79: Consider sharing Redis connection with usage utilsfetchAllAccounts uses this.redisClient, but getApiKeyUsage/trackApiKeyUsage use a separate global client. For efficiency and fewer sockets, consider allowing redisUtils to accept an injected client.
Happy to draft a small adapter in redisUtils to accept an optional client instance.
src/index.ts (2)
35-41: Fail fast in non-interactive environments without a sessionInteractive prompts will hang cron workers. With the per-account session check in place, also gate start() to avoid hanging.
- await client.start({ + await client.start({ phoneNumber: async () => await input.text("Enter your phone number: "), password: async () => await input.text("Enter 2FA password (if enabled): "), phoneCode: async () => await input.text("Enter code you received: "), onError: (err) => console.log(err), });Add above call:
if (!isInteractive && !sessionStr) { throw new Error(`Non-interactive login not possible for ${account.accountId}; provide ${sessionEnvKey}.`); }
75-87: Cron: handle per-run exceptions without process-wide impact and consider jitterCurrent schedule is fine; consider adding random jitter to distribute load and avoid synchronized spikes if multiple workers run.
src/services/twitterAccountManager.ts (5)
99-108: Harden sort against invalid timestamps; treat invalid as “never used”Date parsing can yield NaN; compare using a safe helper.
- accounts.sort((a, b) => { - if (!a.lastUsed && !b.lastUsed) return 0; - if (!a.lastUsed) return -1; // a comes first (never used) - if (!b.lastUsed) return 1; // b comes first (never used) - - // Both have lastUsed dates, compare them - return new Date(a.lastUsed).getTime() - new Date(b.lastUsed).getTime(); - }); + const ts = (s?: string) => { + const t = s ? Date.parse(s) : NaN; + return Number.isFinite(t) ? t : -Infinity; // never/invalid used -> earliest + }; + accounts.sort((a, b) => ts(a.lastUsed) - ts(b.lastUsed));
30-35: Connect race: two concurrent callers can both call connect()Guard with a connecting promise to avoid duplicate connects.
private connecting?: Promise<void>; private async ensureConnected(): Promise<void> { if (this.isConnected) return; if (!this.connecting) { this.connecting = this.redisClient.connect().then(() => { this.isConnected = true; }).finally(() => { this.connecting = undefined; }); } await this.connecting; }
48-54: Type the account payload and improve parse diagnosticsPrefer a typed interface over Record to catch missing fields at compile time; include original parse error for debugging.
type EncryptedTwitterAccount = { TWITTER_AUTH_TOKEN: string; TWITTER_BEARER: string; TWITTER_CSRF_TOKEN: string; // optional: stable account_label for identity across token rotations }; let encryptedAccounts: EncryptedTwitterAccount[]; try { encryptedAccounts = JSON.parse(raw); } catch (e) { throw new Error(`Failed to parse Twitter accounts from Redis: ${(e as Error).message}`); }
57-67: Validate required fields before decryptingEarly-validate presence of expected keys; skip entries with clear diagnostics instead of throwing decrypt errors.
if (!encryptedAccount.TWITTER_AUTH_TOKEN || !encryptedAccount.TWITTER_BEARER || !encryptedAccount.TWITTER_CSRF_TOKEN) { console.warn(`Skipping account index ${i}: missing required encrypted fields`); continue; }
136-141: Tolerate disconnect errors and pending connectsWrap quit() and ignore “The client is closed”/ECONNRESET noise; also handle in-flight connect.
async disconnect(): Promise<void> { try { if (this.isConnected) await this.redisClient.quit(); } catch {} this.isConnected = false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/fetchTelegramMessages.ts(2 hunks)src/index.ts(2 hunks)src/services/telegramAccountManager.ts(1 hunks)src/services/twitterAccountManager.ts(1 hunks)src/tests/rotationDemo.ts(1 hunks)src/tests/telegramRotationDemo.ts(1 hunks)src/tests/testRotation.ts(1 hunks)src/tests/testTelegramRotation.ts(1 hunks)src/twitterApi.ts(4 hunks)src/utils/moveEnvToRedis.ts(3 hunks)src/utils/redisUtils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/tests/rotationDemo.ts (1)
src/services/twitterAccountManager.ts (1)
twitterAccountManager(145-145)
src/tests/telegramRotationDemo.ts (1)
src/services/telegramAccountManager.ts (1)
telegramAccountManager(145-145)
src/utils/moveEnvToRedis.ts (1)
src/lib/encryption.ts (1)
encrypt(20-26)
src/services/twitterAccountManager.ts (2)
src/lib/encryption.ts (1)
decrypt(29-41)src/utils/redisUtils.ts (2)
getApiKeyUsage(60-88)trackApiKeyUsage(18-46)
src/tests/testTelegramRotation.ts (1)
src/services/telegramAccountManager.ts (1)
telegramAccountManager(145-145)
src/tests/testRotation.ts (1)
src/services/twitterAccountManager.ts (1)
twitterAccountManager(145-145)
src/services/telegramAccountManager.ts (2)
src/lib/encryption.ts (1)
decrypt(29-41)src/utils/redisUtils.ts (2)
getApiKeyUsage(60-88)trackApiKeyUsage(18-46)
src/fetchTelegramMessages.ts (2)
src/services/telegramAccountManager.ts (1)
TelegramAccount(5-14)src/utils/redisUtils.ts (1)
trackApiKeyUsage(18-46)
src/index.ts (2)
src/services/telegramAccountManager.ts (2)
TelegramAccount(5-14)telegramAccountManager(145-145)src/fetchTelegramMessages.ts (1)
fetchTelegramMessages(7-62)
src/twitterApi.ts (1)
src/services/twitterAccountManager.ts (2)
TwitterAccount(5-14)twitterAccountManager(145-145)
🔇 Additional comments (4)
src/tests/rotationDemo.ts (1)
12-96: Solid demo; clear rotation flow and proper cleanup.Shows initial/final state, selects earliest-used, marks as used, and always disconnects. LGTM.
src/tests/telegramRotationDemo.ts (1)
12-96: Good demo parity with Twitter; clean lifecycle handling.Clear rotation illustration and guaranteed disconnect. LGTM.
src/services/twitterAccountManager.ts (2)
40-47: Confirm Redis key for account listKey name is 'twitter-accounts' here; usage hashes use 'twitter_accounts:'. Verify this is intentional and documented to avoid future drift.
144-145: Singleton lifecycleEnsure all tests/services call twitterAccountManager.disconnect() on shutdown to avoid dangling connections; consider a factory for DI in tests.
| if (clientMap.has(account.accountId)) { | ||
| return clientMap.get(account.accountId)!; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reuse only live clients; evict stale ones
If a client was disconnected, you’ll return a dead client. Check connectivity before reuse.
- if (clientMap.has(account.accountId)) {
- return clientMap.get(account.accountId)!;
- }
+ const cached = clientMap.get(account.accountId);
+ if (cached && (cached as any).connected !== false) {
+ return cached;
+ }
+ if (cached && (cached as any).connected === false) {
+ clientMap.delete(account.accountId);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (clientMap.has(account.accountId)) { | |
| return clientMap.get(account.accountId)!; | |
| } | |
| const cached = clientMap.get(account.accountId); | |
| if (cached && (cached as any).connected !== false) { | |
| return cached; | |
| } | |
| if (cached && (cached as any).connected === false) { | |
| clientMap.delete(account.accountId); | |
| } |
🤖 Prompt for AI Agents
In src/index.ts around lines 14 to 16, the code returns a cached client without
verifying its connectivity; change the logic to check the client's live status
before reusing it (e.g., call the client's connection/ready/ping method or
inspect a connected/readyState property), and if the client is not connected
remove it from clientMap and proceed to create or fetch a new live client;
ensure the connectivity check is synchronous/awaited as needed and that evicted
clients are properly cleaned up (closed) before replacement.
…ession handling and usage tracking
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/index.ts (1)
14-16: Reuse only live clients; evict stale onesCheck connectivity before returning cached clients; evict disconnected ones.
- if (clientMap.has(account.accountId)) { - return clientMap.get(account.accountId)!; - } + const cached = clientMap.get(account.accountId); + if (cached && (cached as any).connected !== false) { + return cached; + } + if (cached && (cached as any).connected === false) { + clientMap.delete(account.accountId); + }src/services/twitterAccountManager.ts (1)
158-160: Do not expose decrypted credentials from a “…Usage” methodReturn a usage-only projection; provide a separate method when credentials are explicitly needed.
- async getAllAccountsUsage(): Promise<TwitterAccount[]> { - return await this.fetchAllAccounts(); - } + async getAllAccountsUsage(): Promise<Array<{ accountId: string; lastUsed?: string; totalRequests?: number }>> { + const accounts = await this.fetchAllAccounts(); + return accounts.map(({ accountId, lastUsed, totalRequests }) => ({ accountId, lastUsed, totalRequests })); + } + + // Optional: expose full info when explicitly required + async getAllAccountsWithCredentials(): Promise<TwitterAccount[]> { + return await this.fetchAllAccounts(); + }
🧹 Nitpick comments (5)
src/services/telegramAccountManager.ts (1)
112-115: Gate selected-account logs behind a debug flagAvoid emitting identifiers in prod logs.
- console.log(`Selected Telegram account: ${selectedAccount.accountId}`); - console.log(`Last used: ${selectedAccount.lastUsed || 'Never'}`); - console.log(`Total requests: ${selectedAccount.totalRequests || 0}`); + if (process.env.DEBUG_TELEGRAM === '1') { + console.debug(`Selected Telegram account: ${selectedAccount.accountId}`); + console.debug(`Last used: ${selectedAccount.lastUsed || 'Never'}`); + console.debug(`Total requests: ${selectedAccount.totalRequests || 0}`); + }src/fetchTelegramMessages.ts (2)
2-2: Remove unused importtrackApiKeyUsage is not used anymore.
-import { trackApiKeyUsage } from './utils/redisUtils';
54-58: Do not log message contents by defaultGate message logs behind DEBUG_TELEGRAM or drop them.
- console.log(formatted); + if (process.env.DEBUG_TELEGRAM === '1') { + console.debug(formatted); + } @@ - console.log("No messages property found in response:", messages); + if (process.env.DEBUG_TELEGRAM === '1') { + console.debug("No messages property found in response:", messages); + }src/tests/telegramRotationDemo.ts (1)
23-28: Reduce sensitive console output in demoEven in demos, consider gating channel/account prints to DEBUG_TELEGRAM to avoid leaking identifiers in CI logs.
- initialAccounts.forEach((account, index) => { + initialAccounts.forEach((account, index) => { console.log(` ${index + 1}. ${account.accountId}`); console.log(` Last used: ${account.lastUsed || 'Never'}`); console.log(` Total requests: ${account.totalRequests || 0}`); - console.log(` Channel: ${account.credentials.TELEGRAM_TG_CHANNEL}`); + if (process.env.DEBUG_TELEGRAM === '1') { + console.log(` Channel: ${account.credentials.TELEGRAM_TG_CHANNEL}`); + } }); @@ - console.log(` Channel: ${selectedAccount.credentials.TELEGRAM_TG_CHANNEL}`); + if (process.env.DEBUG_TELEGRAM === '1') { + console.log(` Channel: ${selectedAccount.credentials.TELEGRAM_TG_CHANNEL}`); + } @@ - console.log(` Channel: ${account.credentials.TELEGRAM_TG_CHANNEL}`); + if (process.env.DEBUG_TELEGRAM === '1') { + console.log(` Channel: ${account.credentials.TELEGRAM_TG_CHANNEL}`); + }Also applies to: 39-43, 71-75
src/services/twitterAccountManager.ts (1)
21-23: Align Redis URL defaults with Telegram managerFallback to localhost to ease local dev and keep parity.
- this.redisClient = createClient({ - url: redisUrl || process.env.REDIS_URL - }); + this.redisClient = createClient({ + url: redisUrl || process.env.REDIS_URL || 'redis://localhost:6379' + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/fetchTelegramMessages.ts(1 hunks)src/index.ts(2 hunks)src/services/telegramAccountManager.ts(1 hunks)src/services/twitterAccountManager.ts(1 hunks)src/tests/telegramRotationDemo.ts(1 hunks)src/tests/testTelegramRotation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tests/testTelegramRotation.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/services/twitterAccountManager.ts (1)
src/lib/encryption.ts (1)
decrypt(29-41)
src/index.ts (2)
src/services/telegramAccountManager.ts (2)
TelegramAccount(5-14)telegramAccountManager(162-162)src/fetchTelegramMessages.ts (1)
fetchTelegramMessages(7-62)
src/services/telegramAccountManager.ts (2)
src/lib/encryption.ts (1)
decrypt(29-41)src/utils/redisUtils.ts (2)
getApiKeyUsage(60-88)trackApiKeyUsage(18-46)
src/fetchTelegramMessages.ts (1)
src/services/telegramAccountManager.ts (1)
TelegramAccount(5-14)
src/tests/telegramRotationDemo.ts (1)
src/services/telegramAccountManager.ts (1)
telegramAccountManager(162-162)
| await fetchTelegramMessages(client, account); | ||
|
|
||
| // Show usage statistics for all accounts | ||
| const allAccounts = await telegramAccountManager.getAllAccountsUsage(); | ||
| console.log('All Telegram accounts usage:'); | ||
| allAccounts.forEach((acc, index) => { | ||
| console.log(` Account ${index + 1} (${acc.accountId}):`); | ||
| console.log(` Total requests: ${acc.totalRequests}`); | ||
| console.log(` Last used: ${acc.lastUsed || 'Never'}`); |
There was a problem hiding this comment.
Rotation does not update usage — account will never advance
You never call markAccountAsUsed; LRU will keep choosing the same account.
await fetchTelegramMessages(client, account);
+ await telegramAccountManager.markAccountAsUsed(account.accountId);
@@
await fetchTelegramMessages(client, account);
- console.log(`Fetched messages using account: ${account.accountId}`);
+ await telegramAccountManager.markAccountAsUsed(account.accountId);
+ console.log(`Fetched messages using account: ${account.accountId}`);Also applies to: 92-94
🤖 Prompt for AI Agents
In src/index.ts around lines 71 to 79 (and similarly around 92 to 94), the code
prints per-account usage but never updates each account's usage state, so the
LRU rotation won't advance; call
telegramAccountManager.markAccountAsUsed(account.accountId) (or the appropriate
method) after selecting or displaying an account to update its
lastUsed/totalRequests fields and persist the change; ensure you await the
markAccountAsUsed call where necessary and handle any errors so the usage store
reflects the update immediately.
| import { createClient } from 'redis'; | ||
| import { decrypt } from '../lib/encryption'; | ||
| import { getApiKeyUsage, trackApiKeyUsage } from '../utils/redisUtils'; | ||
|
|
There was a problem hiding this comment.
Unify Redis usage: stop mixing this class’s client with utils’ global client
fetchAllAccounts reads via this.redisClient but usage is read/written via utils’ separate client, risking split state across different Redis URLs. Keep all reads/writes on this.redisClient.
-import { getApiKeyUsage, trackApiKeyUsage } from '../utils/redisUtils';
+// use class-local Redis ops for consistency
@@
- // Get usage statistics from Redis
- const usage = await getApiKeyUsage({ accountId, platform: 'telegram' });
+ // Get usage statistics from Redis (same client)
+ const usage = await this.getApiKeyUsageLocal(accountId);
@@
- async markAccountAsUsed(accountId: string): Promise<void> {
- await this.trackApiKeyUsageLocal(accountId);
- // Best-effort unlock; lock has TTL as a safety net
- try { await this.redisClient.del(`lock:telegram:${accountId}`); } catch { }
- }
+ async markAccountAsUsed(accountId: string): Promise<void> {
+ await this.trackApiKeyUsageLocal(accountId);
+ try { await this.redisClient.del(`lock:telegram:${accountId}`); } catch { }
+ }
@@
- private async trackApiKeyUsageLocal(accountId: string): Promise<void> {
- await trackApiKeyUsage({ accountId, platform: 'telegram' });
- }
+ private async trackApiKeyUsageLocal(accountId: string): Promise<void> {
+ await this.ensureConnected();
+ const key = `telegram_accounts:${accountId}`;
+ const now = new Date().toISOString();
+ await this.redisClient
+ .multi()
+ .hIncrBy(key, 'total_requests', 1)
+ .hSet(key, { last_request: now, account_id: accountId })
+ .exec();
+ }
+
+ private async getApiKeyUsageLocal(accountId: string): Promise<{ total_requests: number; last_request: string | null }> {
+ await this.ensureConnected();
+ const key = `telegram_accounts:${accountId}`;
+ const data = await this.redisClient.hGetAll(key);
+ return {
+ total_requests: data?.total_requests ? parseInt(data.total_requests, 10) : 0,
+ last_request: data?.last_request ?? null,
+ };
+ }Also applies to: 71-79, 122-126, 131-133
🤖 Prompt for AI Agents
In src/services/telegramAccountManager.ts around lines 1-4, 71-79, 122-126 and
131-133, the code mixes this class's this.redisClient with the utils module's
global Redis client (getApiKeyUsage/trackApiKeyUsage), which can cause split
state; update the file so all Redis operations use this.redisClient only: either
(preferred) change calls to getApiKeyUsage/trackApiKeyUsage to pass
this.redisClient (and update those helpers to accept a client param), or replace
those helper calls with equivalent operations directly on this.redisClient
(using await this.redisClient.get/ set/ hget/hset/ incr as needed), remove
reliance on the global client, and ensure the client passed is the same
this.redisClient for reads and writes throughout the listed line ranges.
| async getEarliestUsedAccount(): Promise<TelegramAccount> { | ||
| const accounts = await this.fetchAllAccounts(); | ||
|
|
||
| // Sort accounts by last_request timestamp (earliest first) | ||
| // Accounts with no last_request (never used) come first | ||
| accounts.sort((a, b) => { | ||
| if (!a.lastUsed && !b.lastUsed) return 0; | ||
| if (!a.lastUsed) return -1; // a comes first (never used) | ||
| if (!b.lastUsed) return 1; // b comes first (never used) | ||
|
|
||
| // Both have lastUsed dates, compare them | ||
| return new Date(a.lastUsed).getTime() - new Date(b.lastUsed).getTime(); | ||
| }); | ||
|
|
||
| const selectedAccount = accounts[0]; | ||
|
|
||
| console.log(`Selected Telegram account: ${selectedAccount.accountId}`); | ||
| console.log(`Last used: ${selectedAccount.lastUsed || 'Never'}`); | ||
| console.log(`Total requests: ${selectedAccount.totalRequests || 0}`); | ||
|
|
||
| return selectedAccount; | ||
| } |
There was a problem hiding this comment.
Add an atomic claim/lock to prevent same-account selection under concurrency
Multiple workers can select the same LRU account; you also attempt to delete a non-existent lock in markAccountAsUsed. Claim with SET NX PX and fall through to the next candidate.
- const selectedAccount = accounts[0];
-
- console.log(`Selected Telegram account: ${selectedAccount.accountId}`);
- console.log(`Last used: ${selectedAccount.lastUsed || 'Never'}`);
- console.log(`Total requests: ${selectedAccount.totalRequests || 0}`);
-
- return selectedAccount;
+ for (const acc of accounts) {
+ const lockKey = `lock:telegram:${acc.accountId}`;
+ const ok = await this.redisClient.set(lockKey, '1', { NX: true, PX: 15000 });
+ if (ok === 'OK') {
+ if (process.env.DEBUG_TELEGRAM === '1') {
+ console.debug(`[TelegramAccountManager] Selected account=${acc.accountId} lastUsed=${acc.lastUsed ?? 'Never'} totalRequests=${acc.totalRequests ?? 0}`);
+ }
+ return acc;
+ }
+ }
+ throw new Error('No available Telegram accounts to claim (all locked).');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getEarliestUsedAccount(): Promise<TelegramAccount> { | |
| const accounts = await this.fetchAllAccounts(); | |
| // Sort accounts by last_request timestamp (earliest first) | |
| // Accounts with no last_request (never used) come first | |
| accounts.sort((a, b) => { | |
| if (!a.lastUsed && !b.lastUsed) return 0; | |
| if (!a.lastUsed) return -1; // a comes first (never used) | |
| if (!b.lastUsed) return 1; // b comes first (never used) | |
| // Both have lastUsed dates, compare them | |
| return new Date(a.lastUsed).getTime() - new Date(b.lastUsed).getTime(); | |
| }); | |
| const selectedAccount = accounts[0]; | |
| console.log(`Selected Telegram account: ${selectedAccount.accountId}`); | |
| console.log(`Last used: ${selectedAccount.lastUsed || 'Never'}`); | |
| console.log(`Total requests: ${selectedAccount.totalRequests || 0}`); | |
| return selectedAccount; | |
| } | |
| async getEarliestUsedAccount(): Promise<TelegramAccount> { | |
| const accounts = await this.fetchAllAccounts(); | |
| // Sort accounts by last_request timestamp (earliest first) | |
| // Accounts with no last_request (never used) come first | |
| accounts.sort((a, b) => { | |
| if (!a.lastUsed && !b.lastUsed) return 0; | |
| if (!a.lastUsed) return -1; // a comes first (never used) | |
| if (!b.lastUsed) return 1; // b comes first (never used) | |
| // Both have lastUsed dates, compare them | |
| return new Date(a.lastUsed).getTime() - new Date(b.lastUsed).getTime(); | |
| }); | |
| for (const acc of accounts) { | |
| const lockKey = `lock:telegram:${acc.accountId}`; | |
| const ok = await this.redisClient.set(lockKey, '1', { NX: true, PX: 15000 }); | |
| if (ok === 'OK') { | |
| if (process.env.DEBUG_TELEGRAM === '1') { | |
| console.debug(`[TelegramAccountManager] Selected account=${acc.accountId} lastUsed=${acc.lastUsed ?? 'Never'} totalRequests=${acc.totalRequests ?? 0}`); | |
| } | |
| return acc; | |
| } | |
| } | |
| throw new Error('No available Telegram accounts to claim (all locked).'); | |
| } |
…ion storage and retrieval
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/index.ts (1)
87-99: Align lock TTL to job duration, add jitter/backoff, and fix Telegram locking
- Observations: Twitter and the shared BaseAccountManager acquire locks with PX:15000 (15s). src/services/twitterAccountManager.ts and src/services/BaseAccountManager.ts use
set(..., { NX: true, PX: 15000 }). src/services/telegramAccountManager.ts's getEarliestUsedAccount does NOT acquire a lock (it returns accounts[0]); markAccountAsUsed still deleteslock:telegram:${accountId}.- Risk: If client creation/fetching or downstream processing exceeds 15s another worker can steal the same account mid-run; Telegram selection is racy because it has no acquisition step.
- Required fixes:
- Increase lock TTL to cover worst-case job runtime (or implement lease-renewal).
- Use an owner token when setting locks and release only if the token matches (or use a safe Lua script / Redlock) to avoid accidental unlocks.
- Replace immediate "all locked -> throw" behavior with a retry loop using exponential backoff + random jitter to avoid thundering-herd and improve resilience.
- Add equivalent locking to Telegram selection (or make selection atomic) so markAccountAsUsed's unlock matches an actual acquisition.
- Locations to change: src/services/BaseAccountManager.ts (getEarliestUsedAccount lock set), src/services/twitterAccountManager.ts (getEarliestUsedAccount lock set), src/services/telegramAccountManager.ts (getEarliestUsedAccount — add atomic lock acquisition; markAccountAsUsed unlock semantics).
♻️ Duplicate comments (3)
src/index.ts (3)
14-16: Ensure cached Telegram client is live; evict/reconnect stale clientsReusing a disconnected client will fail unpredictably. Check connectivity, try reconnect, else evict and recreate.
- if (clientMap.has(account.accountId)) { - return clientMap.get(account.accountId)!; - } + const cached = clientMap.get(account.accountId); + if (cached) { + try { + if ((cached as any).connected === true) return cached; + await (cached as any).connect?.(); + return cached; + } catch { + try { await (cached as any).disconnect?.(); } catch {} + clientMap.delete(account.accountId); + } + }
70-74: LRU will not advance without marking usageCall
markAccountAsUsedafter a successful fetch; otherwise the same account is repeatedly selected.const client = await createTelegramClient(account); - await fetchTelegramMessages(client, account); + await fetchTelegramMessages(client, account); + await telegramAccountManager.markAccountAsUsed(account.accountId);const client = await createTelegramClient(account); - await fetchTelegramMessages(client, account); + await fetchTelegramMessages(client, account); + await telegramAccountManager.markAccountAsUsed(account.accountId); console.log(`Fetched messages using account: ${account.accountId}`);Also applies to: 94-96
52-57: Don’t print raw session; persist to Redis and log a masked noticeThe
export ${sessionKey}=...is both an invalid env var (contains “:”) and leaks a secret. Save to Redis and avoid dumping the full token.- console.log(`Logged in successfully for account: ${account.accountId}`); - const saved = client.session.save(); - if (process.env.PRINT_TG_SESSION === "1" && isInteractive) { - // Emit an export-ready line deliberately, instead of dumping secrets in logs - console.log(`export ${sessionKey}="${saved}"`); - } + console.log(`Logged in successfully for account: ${account.accountId}`); + const saved = client.session.save(); + try { await (telegramAccountManager as any).redisClient.set(sessionKey, saved); } catch {} + if (process.env.PRINT_TG_SESSION === "1" && isInteractive) { + const preview = saved.slice(0, 8) + "…"; + console.log(`Saved session for ${account.accountId} to Redis key "${sessionKey}" (${preview})`); + }
🧹 Nitpick comments (4)
src/index.ts (1)
31-33: Don’t reach into manager internals; guard Redis connect with isOpenAccessing
telegramAccountManager.redisClientviaas anybreaks encapsulation and can double-connect. Expose a session helper on the manager, or minimally guardconnect().- const redisClient = (telegramAccountManager as any).redisClient; - await redisClient.connect?.(); + const redisClient = (telegramAccountManager as any).redisClient; + if (redisClient?.isOpen !== true) { + await redisClient.connect(); + }src/services/BaseAccountManager.ts (3)
37-42: Be resilient to invalid lastUsed values
new Date(invalid).getTime()yieldsNaN. Treat invalid/missing as oldest.- return new Date(a.lastUsed).getTime() - new Date(b.lastUsed).getTime(); + const ta = Number.isFinite(Date.parse(a.lastUsed!)) ? Date.parse(a.lastUsed!) : 0; + const tb = Number.isFinite(Date.parse(b.lastUsed!)) ? Date.parse(b.lastUsed!) : 0; + return ta - tb;
21-23: Constructor logging shouldn’t rely on abstract field
this.platformmay be undefined duringsuper(); prefer constructor name.- this.redisClient.on('error', (err) => { - console.error(`Redis Client Error in ${this.platform}AccountManager:`, err); - }); + this.redisClient.on('error', (err) => { + console.error(`Redis Client Error in ${this.constructor.name}:`, err); + });
61-66: Use client state instead of shadowisConnected
Rely onredisClient.isOpento avoid drift between flags.- if (this.isConnected) { - await this.redisClient.quit(); - this.isConnected = false; - } + if (this.redisClient?.isOpen) { + await this.redisClient.quit(); + this.isConnected = false; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/index.ts(2 hunks)src/services/BaseAccountManager.ts(1 hunks)src/utils/moveEnvToRedis.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/moveEnvToRedis.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (2)
src/services/telegramAccountManager.ts (2)
TelegramAccount(5-14)telegramAccountManager(162-162)src/fetchTelegramMessages.ts (1)
fetchTelegramMessages(7-62)
🔇 Additional comments (1)
src/index.ts (1)
23-28: API credentials validation looks good
Input validation forapiIdandapiHashis correct and provides account-scoped error messages.
| if (!sessionStr && !isInteractive) { | ||
| throw new Error(`Missing session in Redis for ${account.accountId} (key: ${sessionKey}). Generate and store a session string before running cron.`); | ||
| } | ||
| sessionStr = sessionStr || ""; | ||
| const stringSession = new StringSession(sessionStr); | ||
| const client = new TelegramClient(stringSession, apiId, apiHash, { |
There was a problem hiding this comment.
Non‑interactive runs may hang on prompts; prefer connect+health‑check
When isInteractive is false and a bad/expired session is loaded, start() will try to prompt and hang. Use connect() and a light auth check; only fall back to start() if interactive.
- await client.start({
- phoneNumber: async () => await input.text("Enter your phone number: "),
- password: async () => await input.text("Enter 2FA password (if enabled): "),
- phoneCode: async () => await input.text("Enter code you received: "),
- onError: (err) => console.log(err),
- });
+ if (sessionStr) {
+ await client.connect();
+ try {
+ await client.getMe(); // basic auth/health check
+ } catch (e) {
+ if (!isInteractive) {
+ throw new Error(`Session for ${account.accountId} is invalid/expired; re-auth interactively to refresh.`);
+ }
+ await client.start({
+ phoneNumber: async () => await input.text("Enter your phone number: "),
+ password: async () => await input.text("Enter 2FA password (if enabled): "),
+ phoneCode: async () => await input.text("Enter code you received: "),
+ onError: (err) => console.log(err),
+ });
+ }
+ } else {
+ await client.start({
+ phoneNumber: async () => await input.text("Enter your phone number: "),
+ password: async () => await input.text("Enter 2FA password (if enabled): "),
+ phoneCode: async () => await input.text("Enter code you received: "),
+ onError: (err) => console.log(err),
+ });
+ }Also applies to: 45-50
| const ok = await this.redisClient.set(lockKey, '1', { NX: true, PX: 15000 }); | ||
| if (ok === 'OK') { | ||
| console.debug(`[${this.platform}AccountManager] Selected account=${acc.accountId} lastUsed=${acc.lastUsed ?? 'Never'} totalRequests=${acc.totalRequests ?? 0}`); |
There was a problem hiding this comment.
Unsafe lock release; use token‑based delete (Redlock semantics)
Plain DEL can remove another process’s lock if TTL expired and was reacquired. Store a random token as the lock value and conditionally delete if the token matches.
+ private lockTokens = new Map<string, string>();
@@
- const ok = await this.redisClient.set(lockKey, '1', { NX: true, PX: 15000 });
+ const token = crypto.randomUUID();
+ const ok = await this.redisClient.set(lockKey, token, { NX: true, PX: 15000 });
if (ok === 'OK') {
+ this.lockTokens.set(acc.accountId, token);
console.debug(`[${this.platform}AccountManager] Selected account=${acc.accountId} lastUsed=${acc.lastUsed ?? 'Never'} totalRequests=${acc.totalRequests ?? 0}`);
return acc;
}
@@
- await this.trackApiKeyUsageLocal(accountId);
- try { await this.redisClient.del(`lock:${this.platform}:${accountId}`); } catch { }
+ await this.trackApiKeyUsageLocal(accountId);
+ const lockKey = `lock:${this.platform}:${accountId}`;
+ const token = this.lockTokens.get(accountId) ?? '';
+ // Delete only if value matches our token
+ try {
+ await this.redisClient.eval(
+ 'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end',
+ { keys: [lockKey], arguments: [token] }
+ );
+ } catch {}
+ this.lockTokens.delete(accountId);Alternatively, use the redlock package for robustness.
npm i redlock
Also applies to: 54-57
🤖 Prompt for AI Agents
In src/services/BaseAccountManager.ts around lines 45-47 (and similarly at
54-57), the code uses a plain DEL to release a lock which can remove another
process’s lock after TTL expiry; change the lock to store a
cryptographically-random token as the value when acquiring (SET key token NX PX
...), and when releasing perform an atomic check-and-delete (compare current
value to token and delete only if equal) — implement this using a small Redis
EVAL Lua script or replace the custom locking with the redlock package (npm i
redlock) to get correct token-based release semantics.
…countManager for improved Redis integration and connection handling
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/services/telegramAccountManager.ts (1)
1-4: Stop mixing Redis clients; use this.redisClient everywhere (duplicate).Replace utils getApiKeyUsage/trackApiKeyUsage with class‑local helpers using this.redisClient to avoid split state across Redis URLs.
Apply these diffs:
@@ -import { getApiKeyUsage, trackApiKeyUsage } from '../utils/redisUtils'; +// use class-local Redis ops for consistency@@ - // Get usage statistics from Redis - const usage = await getApiKeyUsage({ accountId, platform: 'telegram' }); + // Get usage statistics from Redis (same client) + const usage = await this.getApiKeyUsageLocal(accountId);@@ - protected async trackApiKeyUsageLocal(accountId: string): Promise<void> { - await trackApiKeyUsage({ accountId, platform: 'telegram' }); - } + protected async trackApiKeyUsageLocal(accountId: string): Promise<void> { + await this.ensureConnected(); + const key = `${this.usageKeyPrefix}:${accountId}`; + const now = new Date().toISOString(); + await this.redisClient + .multi() + .hIncrBy(key, 'total_requests', 1) + .hSet(key, { last_request: now, account_id: accountId }) + .exec(); + }Add this helper (outside the shown ranges):
private async getApiKeyUsageLocal(accountId: string): Promise<{ total_requests: number; last_request: string | null }> { await this.ensureConnected(); const key = `${this.usageKeyPrefix}:${accountId}`; const data = await this.redisClient.hGetAll(key); return { total_requests: data?.total_requests ? parseInt(data.total_requests, 10) : 0, last_request: data?.last_request ?? null, }; }Run to confirm no remaining utils usage in services:
#!/bin/bash rg -nP --type=ts -C2 '\b(getApiKeyUsage|trackApiKeyUsage)\b' src/servicessrc/services/twitterAccountManager.ts (1)
110-112: Do not expose decrypted credentials from getAllAccountsUsage (duplicate).Return a usage-only projection and add a separate API for credentials.
- async getAllAccountsUsage(): Promise<TwitterAccount[]> { - return await this.fetchAllAccounts(); - } + async getAllAccountsUsage(): Promise<Array<{ accountId: string; lastUsed?: string; totalRequests?: number }>> { + const accounts = await this.fetchAllAccounts(); + return accounts.map(({ accountId, lastUsed, totalRequests }) => ({ accountId, lastUsed, totalRequests })); + }Add (outside shown range):
async getAllAccountsWithCredentials(): Promise<TwitterAccount[]> { return await this.fetchAllAccounts(); }src/services/BaseAccountManager.ts (2)
45-47: Acquire locks with per-account random tokens (Redlock semantics).Use a random token as the lock value and store it for safe release.
- const lockKey = `lock:${this.platform}:${acc.accountId}`; - const ok = await this.redisClient.set(lockKey, '1', { NX: true, PX: 15000 }); + const lockKey = `lock:${this.platform}:${acc.accountId}`; + const token = (globalThis.crypto?.randomUUID?.() ?? require('crypto').randomUUID()); + const ok = await this.redisClient.set(lockKey, token, { NX: true, PX: 15000 }); if (ok === 'OK') { - console.debug(`[${this.platform}AccountManager] Selected account=${acc.accountId} lastUsed=${acc.lastUsed ?? 'Never'} totalRequests=${acc.totalRequests ?? 0}`); + this.lockTokens.set(acc.accountId, token); + console.debug(`[${this.platform}AccountManager] Selected account=${acc.accountId} lastUsed=${acc.lastUsed ?? 'Never'} totalRequests=${acc.totalRequests ?? 0}`); return acc; }Add class field (outside shown range):
private lockTokens = new Map<string, string>();
55-59: Release locks safely and always (token check + finally).- async markAccountAsUsed(accountId: string): Promise<void> { - await this.ensureConnected(); - await this.trackApiKeyUsageLocal(accountId); - try { await this.redisClient.del(`lock:${this.platform}:${accountId}`); } catch { } - } + async markAccountAsUsed(accountId: string): Promise<void> { + await this.ensureConnected(); + const lockKey = `lock:${this.platform}:${accountId}`; + const token = this.lockTokens.get(accountId) ?? ''; + try { + await this.trackApiKeyUsageLocal(accountId); + } finally { + try { + await this.redisClient.eval( + 'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end', + { keys: [lockKey], arguments: [token] } + ); + } catch {} + this.lockTokens.delete(accountId); + } + }
🧹 Nitpick comments (11)
src/services/telegramAccountManager.ts (4)
36-41: Harden JSON parsing and validate shape.- try { - encryptedAccounts = JSON.parse(raw); - } catch (e) { - throw new Error('Failed to parse Telegram accounts from Redis'); - } + try { + encryptedAccounts = JSON.parse(raw); + if (!Array.isArray(encryptedAccounts)) { + throw new Error('telegram-accounts must be a JSON array'); + } + } catch (e) { + throw new Error('Failed to parse Telegram accounts from Redis'); + }
45-67: Reduce N+1 Redis roundtrips when loading usage.Decrypt all accounts first, then pipeline hGetAll for usage with multi()/exec() and map replies back to accounts to cut latency.
56-58: Optional: avoid using raw TELEGRAM_API_ID in accountId.For parity with Twitter and to avoid exposing identifiers in keys/logs, derive
accountIdfrom a hash of the API ID (e.g., sha256(...).slice(0,12)).
19-20: usageKeyPrefix is unused here; either use it or remove it for clarity.src/services/twitterAccountManager.ts (3)
36-41: Validate parsed structure from Redis.- try { - encryptedAccounts = JSON.parse(raw); - } catch (e) { - throw new Error('Failed to parse Twitter accounts from Redis'); - } + try { + encryptedAccounts = JSON.parse(raw); + if (!Array.isArray(encryptedAccounts)) { + throw new Error('twitter-accounts must be a JSON array'); + } + } catch (e) { + throw new Error('Failed to parse Twitter accounts from Redis'); + }
45-73: Pipeline usage reads to avoid per-account awaits.After decrypting and computing accountIds, batch hGetAll calls via multi()/exec() and merge results to reduce latency under many accounts.
19-20: usageKeyPrefix declared but not used.Use it in getApiKeyUsageLocal/trackApiKeyUsageLocal key construction or drop it.
src/services/BaseAccountManager.ts (4)
38-43: Guard against invalid lastUsed timestamps in sort.- accounts.sort((a, b) => { + accounts.sort((a, b) => { if (!a.lastUsed && !b.lastUsed) return 0; if (!a.lastUsed) return -1; if (!b.lastUsed) return 1; - return new Date(a.lastUsed).getTime() - new Date(b.lastUsed).getTime(); + const ta = Date.parse(a.lastUsed); + const tb = Date.parse(b.lastUsed); + if (Number.isNaN(ta) && Number.isNaN(tb)) return 0; + if (Number.isNaN(ta)) return 1; + if (Number.isNaN(tb)) return -1; + return ta - tb; });
15-16: Abstract usageKeyPrefix is never used.Either remove it from the base and subclasses, or refactor local usage helpers to build keys from it for consistency.
63-69: Minor: swallow-only catch hides unlock failures.Log at debug level to aid troubleshooting (without spamming):
- if (this.isConnected) { + if (this.isConnected) { await this.redisClient.quit(); this.isConnected = false; }And prefer:
try { /* unlock */ } catch (e) { console.debug('unlock failed', e); }
45-47: Operational: consider ZSET-based true LRU for scale.Model accounts in a sorted set keyed by last_used and atomically claim via LUA or ZPOPMIN/ZADD to avoid scanning/sorting full lists and reduce lock contention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/services/BaseAccountManager.ts(1 hunks)src/services/telegramAccountManager.ts(1 hunks)src/services/twitterAccountManager.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/services/twitterAccountManager.ts (2)
src/services/BaseAccountManager.ts (1)
BaseAccount(3-8)src/lib/encryption.ts (1)
decrypt(29-41)
src/services/telegramAccountManager.ts (3)
src/services/BaseAccountManager.ts (1)
BaseAccount(3-8)src/lib/encryption.ts (1)
decrypt(29-41)src/utils/redisUtils.ts (2)
getApiKeyUsage(60-88)trackApiKeyUsage(18-46)
🔇 Additional comments (2)
src/services/telegramAccountManager.ts (1)
92-95: Good: usage-only projection avoids leaking credentials.src/services/twitterAccountManager.ts (1)
56-59: Good: stable, non-reversible accountId via SHA‑256.
Description:
This PR introduces Least Recently Used (LRU) account rotation for both Telegram and Twitter ingestion. The system now fetches the account that was used earliest from Redis and uses it for API calls, helping to distribute usage and avoid rate limits. Key changes include:
TelegramAccountManagerservice to manage Telegram account rotation and usage tracking via Redis.any[]withRecord<string, string>[]where appropriate.This update ensures robust, scalable, and fair usage of multiple API accounts for both Telegram and Twitter.
Summary by CodeRabbit
New Features
Refactor
Tests