Optimize random image attachment to invoices. - #695
Conversation
- We now store the image path, which is referenced when creating invoices.
WalkthroughImage caching was changed to store filenames and convert to base64 on demand. New ImageProcessingError was added. QR generation and multiple bot message flows now lazily convert images to base64 and handle image-processing failures, and new localized Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
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.
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. ✨ Finishing Touches
🧪 Generate unit tests
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 |
ndungudedan
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bot/messages.ts (1)
145-153: Guard sendMediaGroup when imageBase64 is empty to avoid Telegram “file must be non-empty” errorsIf conversion fails or no image is set, skip the photo and still send the caption.
- const imageBase64 = await imageCache.convertImageToBase64(order.random_image); - - await ctx.telegram.sendMediaGroup(user.tg_id, [{ - type: 'photo', - media: { source: Buffer.from(imageBase64, 'base64') }, - caption: pendingSellCaption, - parse_mode: 'Markdown', - }] - ); + const imageBase64 = await imageCache.convertImageToBase64(order.random_image); + if (imageBase64) { + await ctx.telegram.sendMediaGroup(user.tg_id, [{ + type: 'photo', + media: { source: Buffer.from(imageBase64, 'base64') }, + caption: pendingSellCaption, + parse_mode: 'Markdown', + }]); + } else { + logger.warning(`No image available for order ${order._id}, sending text-only pending sell message`); + await ctx.telegram.sendMessage(user.tg_id, pendingSellCaption, { parse_mode: 'Markdown' }); + }util/index.ts (1)
558-576: Make QR overlay optional: draw center image only if base64 is present and validPrevents failures when no image is selected or conversion fails. Also logs a concise reason.
- const imageBase64 = await imageCache.convertImageToBase64(randomImage); - const ctx = canvas.getContext('2d'); - const centerImage = new Image(); - centerImage.src = `data:image/png;base64,${imageBase64}`; + const imageBase64 = randomImage ? await imageCache.convertImageToBase64(randomImage) : ''; + const ctx = canvas.getContext('2d'); + if (!imageBase64) { + logger.warning('generateQRWithImage: no center image provided or conversion failed; returning QR without overlay'); + return canvas.toBuffer(); + } + const centerImage = new Image(); + centerImage.src = `data:image/png;base64,${imageBase64}`; @@ - const imageSize = canvas.width * imageToQrRatio; - const imagePos = (canvas.width - imageSize) / 2; - - ctx.drawImage(centerImage, imagePos, imagePos, imageSize, imageSize); + const imageSize = canvas.width * imageToQrRatio; + const imagePos = (canvas.width - imageSize) / 2; + ctx.drawImage(centerImage, imagePos, imagePos, imageSize, imageSize);
🧹 Nitpick comments (1)
util/imageCache.ts (1)
39-44: Exclude Honeybadger by name case-insensitively to avoid accidental inclusionIf the file on disk is named with different case (e.g., Honeybadger.PNG), it can slip into regularImages. Compare using lowercase basenames.
- const imageFiles = files.filter((file: string) => - ['.png'].includes(path.extname(file).toLowerCase()) && - file !== honeybadgerFilename - ); + const imageFiles = files.filter((file: string) => + path.extname(file).toLowerCase() === '.png' && + path.basename(file).toLowerCase() !== honeybadgerFilename.toLowerCase() + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
bot/messages.ts(3 hunks)util/imageCache.ts(3 hunks)util/index.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
bot/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
bot/**/*.ts: Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules
Integrate @grammyjs/i18n for dynamic language selection based on user preferences
Files:
bot/messages.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
bot/messages.tsutil/index.tsutil/imageCache.ts
util/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared utilities and helpers under util/
Files:
util/index.tsutil/imageCache.ts
🧬 Code graph analysis (2)
bot/messages.ts (3)
util/imageCache.ts (1)
imageCache(127-127)bot/ordersActions.ts (2)
i18n(47-172)i18n(186-267)bot/modules/orders/takeOrder.ts (1)
ctx(52-97)
util/index.ts (1)
util/imageCache.ts (1)
imageCache(127-127)
🔇 Additional comments (2)
util/index.ts (2)
549-551: Dynamic import to avoid circular dependency — good callThis avoids eager module initialization issues between util/index and util/imageCache.
445-448: Fee branch clarity — OKReturn values for golden vs. regular are explicit and unchanged in semantics.
Returning the invoice without an image after a failure makes sense, only that the message sent will communicate to the user to check for an image. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
util/imageCache.ts (2)
61-73: Fix footgun: 0/negative probability currently yields 100% golden winners.Math.max(1, value) turns 0 into 1, making every draw win. Treat <= 0 as “disabled.”
Apply:
- const goldenProbability = parseInt(process.env.GOLDEN_HONEY_BADGER_PROBABILITY || '100'); - const probability = isNaN(goldenProbability) ? 100 : Math.max(1, goldenProbability); - const luckyNumber = Math.floor(Math.random() * probability) + 1; - const winningNumber = 1; - - logger.debug(`Golden Honey Badger probability check: ${luckyNumber}/${probability} (wins if ${luckyNumber}=${winningNumber})`); - - if (luckyNumber === winningNumber) { - randomImage = this.cache.honeybadgerImage; - isGoldenHoneyBadger = true; - logger.info(`🏆 GOLDEN HONEY BADGER ASSIGNED to order with nonce: ${nonce} - FEES WILL BE ZERO`); - return { randomImage, isGoldenHoneyBadger }; - } + const raw = Number(process.env.GOLDEN_HONEY_BADGER_PROBABILITY ?? '100'); + const probability = Number.isFinite(raw) ? Math.floor(raw) : 100; + if (probability > 0) { + const luckyNumber = Math.floor(Math.random() * probability) + 1; + const winningNumber = 1; + logger.debug(`Golden Honey Badger probability check: ${luckyNumber}/${probability} (wins if ${luckyNumber}=${winningNumber})`); + if (luckyNumber === winningNumber) { + randomImage = this.cache.honeybadgerImage; + isGoldenHoneyBadger = true; + logger.info(`🏆 GOLDEN HONEY BADGER ASSIGNED to order with nonce: ${nonce} - FEES WILL BE ZERO`); + return { randomImage, isGoldenHoneyBadger }; + } + } else { + logger.debug('Golden Honey Badger probability disabled (<= 0).'); + }
91-106: Harden convertImageToBase64 against path traversal and invalid inputs.Current implementation allows '../' escapes and arbitrary file reads; logs lack context.
Apply:
- convertImageToBase64 = async (image: string) => { - let base64Image = ''; - try { - const imageData = await fs.readFile(`images/${image}`); - base64Image = imageData.toString('base64'); - } catch (error) { - logger.error(error); - } - return base64Image; - } + convertImageToBase64 = async (image: string): Promise<string> => { + try { + if (!image) { + logger.warning('convertImageToBase64 called with empty image name'); + return ''; + } + // Reject traversal and subpaths + if (image.includes('..') || image.includes('/') || image.includes('\\')) { + logger.error(`Blocked path traversal attempt for image: "${image}"`); + return ''; + } + if (path.extname(image).toLowerCase() !== '.png') { + logger.warning(`Rejected non-PNG image request: "${image}"`); + return ''; + } + const imagesDir = path.resolve(process.cwd(), 'images'); + const candidate = path.resolve(imagesDir, image); + if (!candidate.startsWith(imagesDir + path.sep)) { + logger.error(`Resolved path escapes images dir: "${candidate}"`); + return ''; + } + const imageData = await fs.readFile(candidate); + if (!imageData?.length) { + logger.error(`Empty image read for "${candidate}"`); + return ''; + } + return imageData.toString('base64'); + } catch (error) { + logger.error(`convertImageToBase64 failed for "${image}": ${error}`); + return ''; + } + }
🧹 Nitpick comments (2)
util/imageCache.ts (2)
91-106: Optional: memoize base64 reads to reduce I/O without bloating DB.Cache by filename with a tiny in-memory Map; evict on demand if needed.
Add near the class fields:
private base64Memo = new Map<string, string>();Then in convertImageToBase64 (after input validation):
const cached = this.base64Memo.get(image); if (cached) return cached; // after successful read: const b64 = imageData.toString('base64'); this.base64Memo.set(image, b64); return b64;
108-114: Nit: metric name may now be misleading.honeybadgerCached now means “filename set,” not “binary cached.” Consider renaming to honeybadgerConfigured or documenting semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
util/imageCache.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
util/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared utilities and helpers under util/
Files:
util/imageCache.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
util/imageCache.ts
🧬 Code graph analysis (1)
util/imageCache.ts (1)
util/index.ts (1)
nonce(542-546)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: ci_to_main
🔇 Additional comments (7)
util/imageCache.ts (7)
6-6: Good: centralized constant for the golden filename.This aligns with the PR goal to use filenames instead of base64.
25-26: Correct: store/propagate the golden image as a filename (not base64).This fixes the downstream API mismatch noted earlier and prevents bloating order docs.
30-35: Case-insensitive filtering and Honeybadger exclusion look right.This addresses the nit about accidental inclusion due to casing.
40-40: No actionable change in this line range.
57-57: No actionable change in this line range.
75-75: No actionable change in this line range.
83-83: No actionable change in this line range.
In a failure case we shouldn't send the QR code at all, in that case we should send to log an error and send the error message to the user like "we had an error processing the image, please wait few minutes and try again" |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
util/index.ts (1)
565-597: Make the center-image overlay optional and fail-safe; return QR-only on any image error.As-is, an empty/failed base64 (or missing randomImage) will set an invalid Image.src and can throw in node-canvas. Wrap the conversion in try/catch, guard falsy results, and early-return the QR buffer without overlay. Also add context to logs.
const generateQRWithImage = async (request: string, randomImage: string) => { // Import imageCache here to avoid circular dependency const { imageCache } = require('./imageCache'); const canvas = createCanvas(400, 400); await QRCode.toCanvas(canvas, request, { margin: 2, width: 400, }); - const imageBase64 = await imageCache.convertImageToBase64(randomImage); + // If no image provided, return QR-only + if (!randomImage) { + logger.warn('generateQRWithImage: no randomImage provided; returning QR without overlay', { + requestLen: request?.length, + }); + return canvas.toBuffer(); + } + + let imageBase64: string | undefined; + try { + imageBase64 = await imageCache.convertImageToBase64(randomImage); + } catch (err) { + logger.warn('generateQRWithImage: convertImageToBase64 failed; returning QR without overlay', { + err, + randomImage, + }); + return canvas.toBuffer(); + } + if (!imageBase64 || imageBase64.length === 0) { + logger.warn('generateQRWithImage: empty base64; returning QR without overlay', { randomImage }); + return canvas.toBuffer(); + } const ctx = canvas.getContext('2d'); const centerImage = new Image(); - centerImage.src = `data:image/png;base64,${imageBase64}`; + centerImage.src = `data:image/png;base64,${imageBase64}`;
🧹 Nitpick comments (1)
util/index.ts (1)
578-579: Minor: set Image.src from Buffer to avoid data-URI overhead.Slightly reduces string churn and parsing cost in node-canvas.
- centerImage.src = `data:image/png;base64,${imageBase64}`; + centerImage.src = Buffer.from(imageBase64, 'base64');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bot/messages.ts(4 hunks)util/imageCache.ts(3 hunks)util/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- util/imageCache.ts
- bot/messages.ts
🧰 Additional context used
📓 Path-based instructions (2)
util/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared utilities and helpers under util/
Files:
util/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
util/index.ts
🔇 Additional comments (1)
util/index.ts (1)
583-589: ```shell
#!/bin/bash
set -eLocate where
loggeris exportedrg -lP "export\s+const\s+logger" -g "*.ts"
Show the definition around that export
rg -nP "export\s+const\s+logger" -g "*.ts" -n -C5
Find any uses of
logger.warning(rg -nP "logger.warning(" -g "*.ts" -n -C2
Find any uses of
logger.warn(rg -nP "logger.warn(" -g "*.ts" -n -C2
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@ndungudedan looks like you didn't update the prettier version of your clone before running the formatting command? it has to be the same version as the one in master branch's package.json |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
util/imageCache.ts (2)
66-86: Probability “0” makes every order golden — treat <= 0 as disabledCurrent Math.max(1, x) forces probability=1 for 0/negatives, making 100% winners. Disable the lottery when value <= 0 and keep the default for NaN.
Apply:
- const goldenProbability = parseInt( - process.env.GOLDEN_HONEY_BADGER_PROBABILITY || '100', - ); - const probability = isNaN(goldenProbability) - ? 100 - : Math.max(1, goldenProbability); - const luckyNumber = Math.floor(Math.random() * probability) + 1; - const winningNumber = 1; - - logger.debug( - `Golden Honey Badger probability check: ${luckyNumber}/${probability} (wins if ${luckyNumber}=${winningNumber})`, - ); - - if (luckyNumber === winningNumber) { - randomImage = this.cache.honeybadgerImage; - isGoldenHoneyBadger = true; - logger.info( - `🏆 GOLDEN HONEY BADGER ASSIGNED to order with nonce: ${nonce} - FEES WILL BE ZERO`, - ); - return { randomImage, isGoldenHoneyBadger }; - } + const raw = Number(process.env.GOLDEN_HONEY_BADGER_PROBABILITY ?? '100'); + const probability = Number.isFinite(raw) ? Math.floor(raw) : 100; + if (probability >= 1) { + const luckyNumber = Math.floor(Math.random() * probability) + 1; + const winningNumber = 1; + logger.debug( + `Golden Honey Badger probability check: ${luckyNumber}/${probability} (wins if ${luckyNumber}=${winningNumber})`, + ); + if (luckyNumber === winningNumber) { + randomImage = this.cache.honeybadgerImage; + isGoldenHoneyBadger = true; + logger.info( + `🏆 GOLDEN HONEY BADGER ASSIGNED to order with nonce: ${nonce} - FEES WILL BE ZERO`, + ); + return { randomImage, isGoldenHoneyBadger }; + } + } else { + logger.debug('Golden Honey Badger lottery disabled (probability <= 0)'); + }
105-121: Harden convertImageToBase64: prevent path traversal, validate extension, avoid silent emptiesReading images/${image} without sanitization is a path traversal risk and can return empty buffers silently.
Apply:
- convertImageToBase64 = async (image: string) => { - let base64Image = ''; - try { - const imageData = await fs.readFile(`images/${image}`); - base64Image = imageData.toString('base64'); - } catch (error) { - logger.error(error); - } - return base64Image; - }; + convertImageToBase64 = async (image: string): Promise<string> => { + try { + if (!image) return ''; + const base = path.basename(image); + if (base !== image) { + logger.error(`Blocked path traversal attempt for image: "${image}"`); + return ''; + } + const imagesDir = path.resolve(process.cwd(), 'images'); + const candidate = path.resolve(imagesDir, base); + if (!candidate.startsWith(imagesDir + path.sep)) { + logger.error(`Blocked path escape for image: "${image}"`); + return ''; + } + if (path.extname(candidate).toLowerCase() !== '.png') { + logger.warn(`Rejected non-PNG image request: "${image}"`); + return ''; + } + const buf = await fs.readFile(candidate); + if (!buf || buf.length === 0) { + logger.error(`Empty/invalid image file: "${candidate}"`); + return ''; + } + return buf.toString('base64'); + } catch (error) { + logger.error(`convertImageToBase64 failed for "${image}": ${error}`); + return ''; + } + };bot/messages.ts (2)
162-173: Guard sendMediaGroup when imageBase64 is empty; fall back to text-onlyPrevents Telegram “file must be non-empty” errors and aligns with PR comments.
Apply:
- const imageBase64 = await imageCache.convertImageToBase64( - order.random_image, - ); - - await ctx.telegram.sendMediaGroup(user.tg_id, [ - { - type: 'photo', - media: { source: Buffer.from(imageBase64, 'base64') }, - caption: pendingSellCaption, - parse_mode: 'Markdown', - }, - ]); + const base64 = await imageCache.convertImageToBase64(order.random_image); + const buf = base64 ? Buffer.from(base64, 'base64') : Buffer.alloc(0); + if (buf.length > 0) { + await ctx.telegram.sendMediaGroup(user.tg_id, [ + { + type: 'photo', + media: { source: buf }, + caption: pendingSellCaption, + parse_mode: 'Markdown', + }, + ]); + } else { + logger.warn(`No image available for order ${order._id}; sending text-only pendingSell message`); + await ctx.telegram.sendMessage(user.tg_id, pendingSellCaption, { parse_mode: 'Markdown' }); + }
421-433: Same guard for beginTakeBuyMessage: skip photo when no image is availableAvoids failures on empty/invalid buffers; fall back to caption-only DM.
Apply:
- const caption = ctx.i18n.t('begin_take_buy', { expirationTime }); - const imageBase64 = await imageCache.convertImageToBase64( - order.random_image, - ); - - await bot.telegram.sendMediaGroup(seller.tg_id, [ - { - type: 'photo', - media: { source: Buffer.from(imageBase64, 'base64') }, - caption, - }, - ]); + const caption = ctx.i18n.t('begin_take_buy', { expirationTime }); + const base64 = await imageCache.convertImageToBase64(order.random_image); + const buf = base64 ? Buffer.from(base64, 'base64') : Buffer.alloc(0); + if (buf.length > 0) { + await bot.telegram.sendMediaGroup(seller.tg_id, [ + { + type: 'photo', + media: { source: buf }, + caption, + }, + ]); + } else { + logger.warn(`No image available for order ${order._id}; sending text-only beginTakeBuy message`); + await bot.telegram.sendMessage(seller.tg_id, caption); + }
🧹 Nitpick comments (1)
util/imageCache.ts (1)
25-25: Store filename (not base64) for golden image — verify existence at initConsider checking that images/Honeybadger.png exists and logging a clear warning if missing, so downstream consumers don’t get an empty base64 later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bot/messages.ts(3 hunks)util/imageCache.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
bot/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
bot/**/*.ts: Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules
Integrate @grammyjs/i18n for dynamic language selection based on user preferences
Files:
bot/messages.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
bot/messages.tsutil/imageCache.ts
util/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared utilities and helpers under util/
Files:
util/imageCache.ts
🧬 Code graph analysis (2)
bot/messages.ts (3)
util/imageCache.ts (1)
imageCache(136-136)bot/modules/orders/takeOrder.ts (1)
ctx(52-99)bot/ordersActions.ts (1)
i18n(55-176)
util/imageCache.ts (1)
util/index.ts (1)
nonce(559-563)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: ci_to_main
🔇 Additional comments (4)
util/imageCache.ts (3)
6-6: Hoisting a constant for the golden filename is correctAligns with “store reference, not base64” and avoids magic strings.
32-35: Case-insensitive .png filtering and exclusion of Honeybadger — LGTMPrevents accidental inclusion and matches PR objective to use filenames.
36-36: Cache holds filenames instead of payloads — LGTMCuts memory usage and DB bloat.
bot/messages.ts (1)
27-27: Import imageCache here — LGTMAvoids circular deps and matches util/index.ts usage.
Thanks @knocte. Resolved |
AFAICS this hasn't been addressed yet? |
thanks @knocte , @ndungudedan can you address this please? |
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)
util/imageCache.ts (1)
67-75: Footgun: probability=0 makes EVERY order golden. Treat <=0 as disabled.- const goldenProbability = parseInt( - process.env.GOLDEN_HONEY_BADGER_PROBABILITY || '100', - ); - const probability = isNaN(goldenProbability) - ? 100 - : Math.max(1, goldenProbability); - const luckyNumber = Math.floor(Math.random() * probability) + 1; - const winningNumber = 1; + const raw = Number(process.env.GOLDEN_HONEY_BADGER_PROBABILITY ?? '100'); + const probability = Number.isFinite(raw) && raw >= 1 ? Math.floor(raw) : null; // null => disabled + const winningNumber = 1; + let luckyNumber: number | null = null; + if (probability !== null) { + luckyNumber = Math.floor(Math.random() * probability) + 1; + }And below, guard the win check and log accordingly:
- logger.debug( - `Golden Honey Badger probability check: ${luckyNumber}/${probability} (wins if ${luckyNumber}=${winningNumber})`, - ); + logger.debug( + probability === null + ? 'Golden Honey Badger lottery disabled' + : `Golden Honey Badger probability check: ${luckyNumber}/${probability} (wins if ${luckyNumber}=${winningNumber})`, + ); - - if (luckyNumber === winningNumber) { + if (probability !== null && luckyNumber === winningNumber) {
♻️ Duplicate comments (5)
bot/messages.ts (4)
169-177: Guard empty/invalid base64 and fall back to text-only caption.Avoid Telegram “file must be non-empty” errors by checking data before sending.
- const imageBase64 = await imageCache.convertImageToBase64( - order.random_image, - ); - - await ctx.telegram.sendMediaGroup(user.tg_id, [ - { - type: 'photo', - media: { source: Buffer.from(imageBase64, 'base64') }, - caption: pendingSellCaption, - parse_mode: 'Markdown', - }, - ]); + const imageBase64 = + order.random_image + ? await imageCache.convertImageToBase64(order.random_image) + : ''; + const buf = imageBase64 ? Buffer.from(imageBase64, 'base64') : Buffer.alloc(0); + if (buf.length > 0) { + await ctx.telegram.sendMediaGroup(user.tg_id, [ + { + type: 'photo', + media: { source: buf }, + caption: pendingSellCaption, + parse_mode: 'Markdown', + }, + ]); + } else { + logger.warn(`No image available for order ${order._id}, sending text-only pending-sell message`); + await ctx.telegram.sendMessage(user.tg_id, pendingSellCaption, { parse_mode: 'Markdown' }); + }
193-199: Also handle non-ImageProcessingError send failures with text-only fallback.} catch (error) { logger.error(error); - if (error instanceof ImageProcessingError) { - await ctx.telegram.sendMessage( - user.tg_id, - i18n.t('image_processing_error'), - ); - } + if (error instanceof ImageProcessingError) { + await ctx.telegram.sendMessage(user.tg_id, i18n.t('image_processing_error')); + } else { + // Fallback: send caption-only if media group failed + await ctx.telegram.sendMessage(user.tg_id, i18n.t('pending_sell', { + channel, + orderExpirationWindow: Math.round(Number(process.env.ORDER_PUBLISHED_EXPIRATION_WINDOW) / 60 / 60), + }), { parse_mode: 'Markdown' }); + } }
435-445: Same guard for beginTakeBuyMessage: skip photo when base64/buffer is empty.- const imageBase64 = await imageCache.convertImageToBase64( - order.random_image, - ); - - await bot.telegram.sendMediaGroup(seller.tg_id, [ - { - type: 'photo', - media: { source: Buffer.from(imageBase64, 'base64') }, - caption, - }, - ]); + const imageBase64 = + order.random_image + ? await imageCache.convertImageToBase64(order.random_image) + : ''; + const buf = imageBase64 ? Buffer.from(imageBase64, 'base64') : Buffer.alloc(0); + if (buf.length > 0) { + await bot.telegram.sendMediaGroup(seller.tg_id, [ + { type: 'photo', media: { source: buf }, caption }, + ]); + } else { + logger.warn(`No image available for order ${order._id}, sending text-only begin-take-buy message`); + await bot.telegram.sendMessage(seller.tg_id, caption); + }
465-470: Also fall back to text-only caption on non-ImageProcessingError media failures.} catch (error) { logger.error(error); - if (error instanceof ImageProcessingError) { - await bot.telegram.sendMessage( - seller.tg_id, - ctx.i18n.t('image_processing_error'), - ); - } + if (error instanceof ImageProcessingError) { + await bot.telegram.sendMessage(seller.tg_id, ctx.i18n.t('image_processing_error')); + } else { + await bot.telegram.sendMessage(seller.tg_id, ctx.i18n.t('begin_take_buy', { + expirationTime: Number(process.env.HOLD_INVOICE_EXPIRATION_WINDOW) / 60, + })); + } }util/imageCache.ts (1)
114-122: Path traversal + weak validation in convertImageToBase64. Harden file access.- convertImageToBase64 = async (image: string) => { - try { - const imageData = await fs.readFile(`images/${image}`); - return imageData.toString('base64'); - } catch (error) { - logger.error(`Error in convertImageToBase64: ${error}, image: ${image}`); - throw new ImageProcessingError(`Failed to convert image ${image}`, image); - } - }; + convertImageToBase64 = async (image: string) => { + try { + if (!image) { + throw new ImageProcessingError('Empty image filename', image); + } + const imagesDir = path.resolve(process.cwd(), 'images'); + const candidate = path.resolve(imagesDir, image); + if (!candidate.startsWith(imagesDir + path.sep)) { + logger.warn(`Blocked path traversal attempt for image: ${image}`); + throw new ImageProcessingError('Invalid image path', image); + } + if (path.extname(candidate).toLowerCase() !== '.png') { + logger.warn(`Rejected non-PNG image request: ${image}`); + throw new ImageProcessingError('Unsupported image extension', image); + } + const imageData = await fs.readFile(candidate); + if (!imageData || imageData.length === 0) { + throw new ImageProcessingError('Empty image file', image); + } + return imageData.toString('base64'); + } catch (error) { + logger.error(`convertImageToBase64 failed for "${image}": ${error}`); + if (error instanceof ImageProcessingError) throw error; + throw new ImageProcessingError(`Failed to convert image ${image}`, image); + } + };
🧹 Nitpick comments (7)
locales/pt.yaml (1)
665-665: Fix lint: add trailing newline at EOF.YAMLlint flags “no new line at end of file”.
Apply:
-image_processing_error: Tivemos um erro ao processar a imagem, por favor aguarde alguns minutos e tente novamente. +image_processing_error: Tivemos um erro ao processar a imagem, por favor aguarde alguns minutos e tente novamente. +locales/en.yaml (1)
671-671: Grammar tweak + newline at EOF.Add “a” and fix YAMLlint newline.
-image_processing_error: We had an error processing the image, please wait few minutes and try again +image_processing_error: We had an error processing the image, please wait a few minutes and try again +locales/fa.yaml (1)
665-665: Fix lint: add trailing newline at EOF.-image_processing_error: ما یک خطا در پردازش تصویر داشتیم، لطفاً چند دقیقه صبر کنید و دوباره تلاش کنید. +image_processing_error: ما یک خطا در پردازش تصویر داشتیم، لطفاً چند دقیقه صبر کنید و دوباره تلاش کنید. +locales/ko.yaml (1)
663-663: Fix lint: add trailing newline at EOF.-image_processing_error: 이미지 처리에 오류가 발생했습니다. 몇 분 후에 다시 시도해 주세요. +image_processing_error: 이미지 처리에 오류가 발생했습니다. 몇 분 후에 다시 시도해 주세요. +locales/fr.yaml (1)
665-665: Fix lint: add trailing newline at EOF.-image_processing_error: Nous avons eu une erreur lors du traitement de l'image, veuillez attendre quelques minutes et réessayer. +image_processing_error: Nous avons eu une erreur lors du traitement de l'image, veuillez attendre quelques minutes et réessayer. +locales/it.yaml (1)
663-663: Fix lint: add trailing newline at EOF.-image_processing_error: Abbiamo avuto un errore nel processare l'immagine, per favore attendi qualche minuto e prova di nuovo. +image_processing_error: Abbiamo avuto un errore nel processare l'immagine, per favore attendi qualche minuto e prova di nuovo. +util/errors.ts (1)
1-11: Optional: add cause/code for richer diagnostics without breaking callers.-export class ImageProcessingError extends Error { - constructor( - message: string, - public readonly imageName?: string, - ) { - super(message); - this.name = 'ImageProcessingError'; - - Object.setPrototypeOf(this, ImageProcessingError.prototype); - } -} +export class ImageProcessingError extends Error { + constructor( + message: string, + public readonly imageName?: string, + public readonly cause?: unknown, + ) { + super(message); + this.name = 'ImageProcessingError'; + (this as any).code = 'IMAGE_PROCESSING_ERROR'; + if (cause !== undefined) (this as any).cause = cause; // TS <4.9 compatible + Object.setPrototypeOf(this, ImageProcessingError.prototype); + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
bot/messages.ts(7 hunks)locales/de.yaml(1 hunks)locales/en.yaml(1 hunks)locales/es.yaml(1 hunks)locales/fa.yaml(1 hunks)locales/fr.yaml(1 hunks)locales/it.yaml(1 hunks)locales/ko.yaml(1 hunks)locales/pt.yaml(1 hunks)locales/ru.yaml(1 hunks)locales/uk.yaml(1 hunks)util/errors.ts(1 hunks)util/imageCache.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
locales/**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Store i18n message templates as YAML files under locales/ for multi-language support
Files:
locales/fr.yamllocales/ru.yamllocales/pt.yamllocales/es.yamllocales/en.yamllocales/de.yamllocales/fa.yamllocales/it.yamllocales/ko.yamllocales/uk.yaml
util/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared utilities and helpers under util/
Files:
util/errors.tsutil/imageCache.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Winston for logging (avoid console) and include contextual details and timeout monitoring where relevant
Wrap risky operations in try-catch with meaningful error context
Files:
util/errors.tsbot/messages.tsutil/imageCache.ts
bot/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
bot/**/*.ts: Use custom Telegraf context types (MainContext, CommunityContext) in handlers and modules
Integrate @grammyjs/i18n for dynamic language selection based on user preferences
Files:
bot/messages.ts
🧬 Code graph analysis (2)
bot/messages.ts (2)
util/errors.ts (1)
ImageProcessingError(1-11)util/imageCache.ts (1)
imageCache(138-138)
util/imageCache.ts (2)
util/errors.ts (1)
ImageProcessingError(1-11)util/index.ts (1)
nonce(559-563)
🪛 YAMLlint (1.37.1)
locales/fr.yaml
[error] 665-665: no new line character at the end of file
(new-line-at-end-of-file)
locales/ru.yaml
[error] 666-666: no new line character at the end of file
(new-line-at-end-of-file)
locales/pt.yaml
[error] 665-665: no new line character at the end of file
(new-line-at-end-of-file)
locales/en.yaml
[error] 671-671: no new line character at the end of file
(new-line-at-end-of-file)
locales/de.yaml
[error] 666-666: no new line character at the end of file
(new-line-at-end-of-file)
locales/fa.yaml
[error] 665-665: no new line character at the end of file
(new-line-at-end-of-file)
locales/it.yaml
[error] 663-663: no new line character at the end of file
(new-line-at-end-of-file)
locales/ko.yaml
[error] 663-663: no new line character at the end of file
(new-line-at-end-of-file)
locales/uk.yaml
[error] 662-662: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (9)
locales/en.yaml (1)
671-671: Manual verification required: ensureimage_processing_erroris present in everylocales/*.yamlfile
Please confirm that the new keyimage_processing_erroris included in each locale file (e.g. by runninggrep -R -L '^\s*image_processing_error\s*:' locales/*.yaml).locales/es.yaml (1)
666-666: LGTM — consistent wording and punctuation.util/errors.ts (1)
1-11: Well-formed custom error (proper prototype, name).bot/messages.ts (3)
27-29: LGTM: imports align with new error/convert flow.
143-148: Good: localized fallback on image-processing failure for QR.
514-516: LGTM: localized error message for hold-invoice QR failures.util/imageCache.ts (3)
57-58: Use logger.warn (standard) instead of logger.warning (may be undefined).- logger.warning('Image cache not initialized, returning empty image'); + logger.warn('Image cache not initialized, returning empty image');If your logger intentionally exposes .warning, ignore this change.
31-38: LGTM: case-insensitive .png filter and honeybadger exclusion.
26-27: LGTM: store honeybadger filename (not base64) to match new API.
| user_taker_is_blocked_by_user_order: Du kannst diesen Auftrag nicht annehmen, da dich sein Ersteller gesperrt hat | ||
| check_solvers: Ihre Community ${communityName} hat keine Solver. Bitte fügen Sie innerhalb von ${remainingDays} Tagen mindestens einen hinzu, um zu verhindern, dass die Community deaktiviert wird. | ||
| check_solvers_last_warning: Ihre Community ${communityName} hat keine Solver. Bitte fügen Sie noch heute mindestens einen hinzu, um zu verhindern, dass die Community deaktiviert wird. | ||
| image_processing_error: Wir hatten einen Fehler beim Verarbeiten des Bildes, bitte warten Sie ein paar Minuten und versuchen Sie es erneut. No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix YAML lint error: add newline at EOF.
Add a trailing newline to satisfy YAMLlint.
-image_processing_error: Wir hatten einen Fehler beim Verarbeiten des Bildes, bitte warten Sie ein paar Minuten und versuchen Sie es erneut.
+image_processing_error: Wir hatten einen Fehler beim Verarbeiten des Bildes, bitte warten Sie ein paar Minuten und versuchen Sie es erneut.
+📝 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.
| image_processing_error: Wir hatten einen Fehler beim Verarbeiten des Bildes, bitte warten Sie ein paar Minuten und versuchen Sie es erneut. | |
| image_processing_error: Wir hatten einen Fehler beim Verarbeiten des Bildes, bitte warten Sie ein paar Minuten und versuchen Sie es erneut. | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 666-666: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In locales/de.yaml around line 666, the file lacks a trailing newline causing a
YAML lint error; fix it by adding a single newline character at the end of the
file (ensure the last line ends with a line break) and save the file so YAMLlint
passes.
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you | ||
| check_solvers: В вашем сообществе ${communityName} нет решателей. Добавьте хотя бы одно в течение ${remainingDays} дн., чтобы сообщество не было отключено. | ||
| check_solvers_last_warning: В вашем сообществе ${communityName} нет решателей. Пожалуйста, добавьте хотя бы один сегодня, чтобы предотвратить отключение сообщества. | ||
| image_processing_error: У нас возникла ошибка при обработке изображения, пожалуйста, подождите несколько минут и попробуйте снова. No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix YAML lint error: add newline at EOF.
YAMLlint flags “no new line character at the end of file”; add a trailing newline to keep CI green.
-image_processing_error: У нас возникла ошибка при обработке изображения, пожалуйста, подождите несколько минут и попробуйте снова.
+image_processing_error: У нас возникла ошибка при обработке изображения, пожалуйста, подождите несколько минут и попробуйте снова.
+📝 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.
| image_processing_error: У нас возникла ошибка при обработке изображения, пожалуйста, подождите несколько минут и попробуйте снова. | |
| image_processing_error: У нас возникла ошибка при обработке изображения, пожалуйста, подождите несколько минут и попробуйте снова. | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 666-666: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In locales/ru.yaml around line 666, the file is missing a trailing newline which
causes YAML lint to fail; add a single newline character at EOF so the file ends
with a blank line (i.e., ensure the last line is terminated with \n) and commit
the change.
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you | ||
| check_solvers: У вашій спільноті ${communityName} немає розв’язувачів. Додайте принаймні одну протягом ${remainingDays} днів, щоб запобігти вимкненню спільноти. | ||
| check_solvers_last_warning: У вашій спільноті ${communityName} немає розв’язувачів. Будь ласка, додайте принаймні одну сьогодні, щоб запобігти вимкненню спільноти. | ||
| image_processing_error: У нас виникла помилка при обробці зображення, будь ласка, почекайте кілька хвилин і спробуйте знову. No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix YAML lint error: add newline at EOF.
Add a trailing newline to satisfy YAMLlint.
-image_processing_error: У нас виникла помилка при обробці зображення, будь ласка, почекайте кілька хвилин і спробуйте знову.
+image_processing_error: У нас виникла помилка при обробці зображення, будь ласка, почекайте кілька хвилин і спробуйте знову.
+📝 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.
| image_processing_error: У нас виникла помилка при обробці зображення, будь ласка, почекайте кілька хвилин і спробуйте знову. | |
| # locales/uk.yaml | |
| … | |
| image_processing_error: У нас виникла помилка при обробці зображення, будь ласка, почекайте кілька хвилин і спробуйте знову. | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 662-662: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In locales/uk.yaml around line 662, the file is missing a trailing newline which
causes YAML lint errors; open the file and add a single newline character at the
end of the file (ensure the last line ends with a line break), save, and re-run
linting to confirm the error is resolved.
grunch
left a comment
grunch
left a comment
There was a problem hiding this comment.
tACK, great work @ndungudedan add a lightning invoice with 24 hours of expiration and amount of 300000 sats as a comment of this PR
|
Thanks @grunch lnbc3m1p5vyu9fpp5ytf3qamvn3494rtfsmqs3vuvkhj4tm86g72ur4gtgsyyq5pfdutsdqu235x2gz8wfjkzapqf3hkx6eqf9hqcqzzsxqyz5vqsp5w4a9u2desp43qv8nl6ltpn5wsmt59f23rx9vtk74nwv64neq4rfs9qxpqysgqg22wm3juj96u5s9h0vmc4zke9tysp7xch83zpe8z5xmescgnt773cu352t9vvcumahrg6wqy8f8nt63harxv7ee68fgchug20h6gljsp3x9ecx |
Thanks to you, paid! preimage: 3028aee30aa3db298528fe6f6115a213e780f7950b9195dfd4fb70c080402f27 |
4 participants
Optimize random image attachment to invoices.
We now store the image path, which is referenced when attaching random images to invoices.
Resolves #688
Summary by CodeRabbit
Refactor
Bug Fixes
Localization
Chores