Skip to content

fix: upload to storage#161

Merged
hmbanan666 merged 1 commit into
mainfrom
fix
Sep 16, 2025
Merged

fix: upload to storage#161
hmbanan666 merged 1 commit into
mainfrom
fix

Conversation

@hmbanan666
Copy link
Copy Markdown
Collaborator

@hmbanan666 hmbanan666 commented Sep 16, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability when sending photos, videos, and files via the Telegram bot, reducing failed uploads and errors.
    • Ensures uploads occur only when a valid download link is available, preventing unexpected failures.
    • Messages are now sent even if an attachment can’t be uploaded; in such cases the attachment link may be omitted.
    • Enhanced error handling for media retrieval to avoid crashes and provide smoother messaging experiences.

@hmbanan666 hmbanan666 self-assigned this Sep 16, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 16, 2025

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: upload to storage" is concise and directly relates to the primary change in the diff, which updates the file upload flow and makes uploads conditional when a download URL is present. It follows conventional commit style and highlights the affected area. Although it could be slightly more specific about the conditional behavior or the Telegram/Wasabi context, it is not vague or unrelated and therefore summarizes the main change acceptably.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
apps/web-app/server/services/telegram/wasabi-bot.ts (2)

181-187: Apply same hardening to video upload path.

-  let fileUrl
+  let fileUrl: string | undefined
   const downloadUrl = await getFileDownloadUrl({ ctx, fileId, botToken })
-  if (downloadUrl) {
-    const uploaded = await uploadToStorage(downloadUrl, fileId)
-    fileUrl = uploaded.fileUrl
-  }
+  if (!downloadUrl) {
+    logger.warn('handleVideo: no downloadUrl', { fileId })
+  } else {
+    try {
+      const uploaded = await uploadToStorage(downloadUrl, fileId)
+      fileUrl = uploaded.fileUrl
+    } catch (err) {
+      logger.error('handleVideo: uploadToStorage failed', err)
+    }
+  }

219-225: Apply same hardening to document upload path.

-  let fileUrl
+  let fileUrl: string | undefined
   const downloadUrl = await getFileDownloadUrl({ ctx, fileId, botToken })
-  if (downloadUrl) {
-    const uploaded = await uploadToStorage(downloadUrl, fileId)
-    fileUrl = uploaded.fileUrl
-  }
+  if (!downloadUrl) {
+    logger.warn('handleFile: no downloadUrl', { fileId })
+  } else {
+    try {
+      const uploaded = await uploadToStorage(downloadUrl, fileId)
+      fileUrl = uploaded.fileUrl
+    } catch (err) {
+      logger.error('handleFile: uploadToStorage failed', err)
+    }
+  }
🧹 Nitpick comments (2)
apps/web-app/server/services/telegram/wasabi-bot.ts (2)

143-149: Harden upload: type fileUrl and catch upload errors.

Avoid undefined type drift and unhandled rejections. Also log when no downloadUrl is returned.

-  let fileUrl
+  let fileUrl: string | undefined

   const downloadUrl = await getFileDownloadUrl({ ctx, fileId, botToken })
-  if (downloadUrl) {
-    const uploaded = await uploadToStorage(downloadUrl, fileId)
-    fileUrl = uploaded.fileUrl
-  }
+  if (!downloadUrl) {
+    logger.warn('handlePhoto: no downloadUrl', { fileId })
+  } else {
+    try {
+      const uploaded = await uploadToStorage(downloadUrl, fileId)
+      fileUrl = uploaded.fileUrl
+    } catch (err) {
+      logger.error('handlePhoto: uploadToStorage failed', err)
+    }
+  }

Please confirm that repository.ticket.createMessage accepts fileUrl as optional/nullable; otherwise persist null instead of undefined.


265-275: Tighten getFileDownloadUrl: destructure args, check file_path, clearer logs.

-async function getFileDownloadUrl(data: { ctx: Context, fileId: string, botToken: string }): Promise<string | null> {
+async function getFileDownloadUrl({ ctx, fileId, botToken }: { ctx: Context, fileId: string, botToken: string }): Promise<string | null> {
   try {
-    const file = await data.ctx.api.getFile(data.fileId)
-    if (!file) {
-      return null
-    }
-
-    return `https://api.telegram.org/file/bot${data.botToken}/${file.file_path}`
-  } catch (e) {
-    logger.error('getFileDownloadUrl', e)
+    const file = await ctx.api.getFile(fileId)
+    if (!file?.file_path) {
+      logger.warn('getFileDownloadUrl: missing file_path', { fileId })
+      return null
+    }
+    return `https://api.telegram.org/file/bot${botToken}/${file.file_path}`
+  } catch (e) {
+    logger.error('getFileDownloadUrl failed', e)
     return null
   }
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d963fd9 and 7d3170d.

📒 Files selected for processing (1)
  • apps/web-app/server/services/telegram/wasabi-bot.ts (5 hunks)
⏰ 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). (1)
  • GitHub Check: build

})

logger.log('file', data.user.id, ctx.message.from.id, ctx.message.text, ctx.message.caption, ctx.message.document, downloadUrl)
logger.log('file', data.user.id, ctx.message.from.id, ctx.message.caption, ctx.message.document, downloadUrl)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Do not log downloadUrl (bot token exposure).

downloadUrl embeds the bot token in the URL path; logging it leaks credentials. Remove or redact.

-  logger.log('file', data.user.id, ctx.message.from.id, ctx.message.caption, ctx.message.document, downloadUrl)
+  logger.log('file', data.user.id, ctx.message.from.id, ctx.message.caption, ctx.message.document, { hasDownloadUrl: Boolean(downloadUrl) })

Also update similar logs for photo/video (outside this hunk):

logger.log('photo', data.user.id, ctx.message.from.id, ctx.message.caption, ctx.message.photo, { hasDownloadUrl: Boolean(downloadUrl) })
logger.log('video', data.user.id, ctx.message.from.id, ctx.message.caption, ctx.message.video, { hasDownloadUrl: Boolean(downloadUrl) })
🤖 Prompt for AI Agents
In apps/web-app/server/services/telegram/wasabi-bot.ts around line 236, the
logger call currently includes downloadUrl which exposes the bot token; remove
the raw downloadUrl from logging and instead log only safe identifiers and
metadata (e.g., keep 'file', data.user.id, ctx.message.from.id,
ctx.message.caption, ctx.message.document) and add a boolean flag indicating
presence of a download URL (e.g., { hasDownloadUrl: Boolean(downloadUrl) }) so
the actual URL/token is never written to logs; also update the analogous photo
and video logging sites to stop outputting the raw downloadUrl and use the same
{ hasDownloadUrl: Boolean(downloadUrl) } pattern.

@hmbanan666 hmbanan666 merged commit 3d6f21b into main Sep 16, 2025
8 checks passed
@hmbanan666 hmbanan666 deleted the fix branch September 16, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant