Merged
Conversation
📝 WalkthroughWalkthroughFastAPI ベースの新しいランクカード画像生成マイクロサービス（ Changes
Sequence Diagram(s)sequenceDiagram
participant User as Discord User
participant Bot as Discord Bot\n(src/bot/cogs/levels.py)
participant ImageAPI as Images Service\n(FastAPI)
participant Gen as Image Generator\n(src/images/utils/levels.py)
participant Discord as Discord API
User->>Bot: /rank コマンド実行
activate Bot
Bot->>Bot: レベル・XP計算 (LevelUtils)
Bot->>ImageAPI: GET /levels/rank?name=...&level=...¤t_xp=...&max_xp=...&avatar_url=...
activate ImageAPI
ImageAPI->>Gen: generate_rank_card_blocking(...) (asyncio.to_thread)
activate Gen
Gen->>Gen: 背景ロード (URL or static)、アバター取得、円形マスク、ステータス・進捗描画、テキスト描画
Gen-->>ImageAPI: PNG バイト列
deactivate Gen
ImageAPI-->>Bot: image/png (PNG bytes)
deactivate ImageAPI
Bot->>Discord: ファイル添付で返信 (rank_card.png)
deactivate Bot
Discord-->>User: ランクカード画像表示
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 6
🧹 Nitpick comments (3)
docker-compose.yaml (1)
43-50:imagesサービスにヘルスチェックを追加することを推奨します。
botサービスがimagesサービスにHTTPリクエストを送信する際、imagesサービスがまだ起動していない場合にエラーが発生する可能性があります。また、botサービスがimagesに依存していることを明示するため、depends_onの追加も検討してください。♻️ ヘルスチェックと依存関係の追加例
images: build: context: ./src/images dockerfile: Dockerfile networks: - sharkbot_network ports: - "8000:8000" + healthcheck: + test: ["CMD", "wget", "--spider", "-q", "http://localhost:8000/"] + interval: 5s + timeout: 3s + retries: 5
botサービスに依存関係を追加:bot: depends_on: images: condition: service_healthy🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yaml` around lines 43 - 50, Add a healthcheck to the images service and make bot depend on images' healthy state: update the images service block to include a healthcheck (test/cmd, interval, timeout, retries) so Docker can mark it healthy, and update the bot service to use depends_on with images: condition: service_healthy; reference the images service name and the bot service's depends_on key when making these edits.src/images/routers/levels.py (2)
25-37: 画像生成処理のエラーハンドリングを追加することを検討してください。
generate_rank_card_blockingは内部でエラーを静かにキャッチしてフォールバックしますが、デバッグを容易にするためにログ出力やエラーレスポンスの追加が有用です。♻️ エラーハンドリングの追加例
+import logging + +logger = logging.getLogger(__name__) + `@router.get`("/rank") async def get_rank_card(...): ... - image_data = await asyncio.to_thread( - generate_rank_card_blocking, - ... - ) + try: + image_data = await asyncio.to_thread( + generate_rank_card_blocking, + name, + level, + current_xp, + max_xp, + avatar_url, + bg, + status, + clean_color + ) + except Exception as e: + logger.exception("Failed to generate rank card") + raise HTTPException(status_code=500, detail="Image generation failed") return Response(content=image_data, media_type="image/png")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/images/routers/levels.py` around lines 25 - 37, Wrap the call to asyncio.to_thread(generate_rank_card_blocking, ...) in a try/except that catches Exception, logs the exception (use the module's logger or logging.getLogger) including the function name generate_rank_card_blocking and input identifiers (name, level, avatar_url) for context, and return an appropriate error Response (e.g., HTTP 500 JSON error or a small fallback image) instead of letting the handler fail silently; update the Response return path to use the successfully generated image_data when no exception occurs and ensure the log message includes the exception traceback to aid debugging.
10-19: 入力パラメータのバリデーション追加を推奨します。現在、
statusのみ正規表現でバリデーションされていますが、他のパラメータにも検証を追加することで、無効なリクエストを早期に検出できます。
level: 負の値を防止current_xp/max_xp: 負の値を防止、max_xpが0の場合のゼロ除算を防止avatar_url: URL形式の検証♻️ バリデーションの追加例
+from fastapi import APIRouter, Response, Query, HTTPException +from pydantic import HttpUrl + `@router.get`("/rank") async def get_rank_card( name: str = "neko", - level: int = 10, - current_xp: int = 750, - max_xp: int = 1000, + level: int = Query(10, ge=0), + current_xp: int = Query(750, ge=0), + max_xp: int = Query(1000, gt=0), avatar_url: str = "https://github.com/identicons/neko.png", bg: str = "static/sea.jpg", status: str = Query("online", pattern="^(online|idle|dnd|offline)$"), color: str = "#e63119" ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/images/routers/levels.py` around lines 10 - 19, Add explicit validation for get_rank_card parameters: ensure level is non-negative, current_xp and max_xp are non-negative and max_xp > 0 (and optionally current_xp <= max_xp), and validate avatar_url is a proper URL; implement this by either switching the function parameter types to pydantic-constrained types (e.g., NonNegativeInt/PositiveInt and HttpUrl/AnyUrl) or by adding runtime checks at the start of get_rank_card that raise fastapi.HTTPException(400) with clear messages when level < 0, current_xp < 0, max_xp <= 0 or current_xp > max_xp, and when avatar_url fails URL parsing/validation; keep the existing status regex validation as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bot/cogs/levels.py`:
- Around line 83-91: The code erroneously recalculates current_level from
data['xp'] (treating xp as cumulative) which breaks rank cards; instead use the
stored data['level'] as the user's current level and treat data['xp'] as the
progress-within-level value. Replace usage of
LevelUtils.get_level_from_total_xp(total_xp) and any logic that assumes
data['xp'] is total XP with using level = data['level'], compute
xp_at_start_of_level via LevelUtils.get_total_xp_for_level(level) only if needed
for display_max_xp, set display_current_xp = data['xp'], and set display_max_xp
= LevelUtils.get_total_xp_for_level(level + 1) -
LevelUtils.get_total_xp_for_level(level) so the rank card shows correct
progress.
- Around line 117-119: Replace the broad "except Exception as e" block that
prints "Connection Error" and sends a generic followup with targeted exception
handling: catch aiohttp.ClientError and asyncio.TimeoutError and handle them by
sending the existing user-facing message via interaction.followup.send, while
logging the exception details (use the module logger instead of print); for any
other Exception, log the full exception with traceback and re-raise it so it
doesn't get misclassified as a connection error. Ensure you import aiohttp and
asyncio (and the logger) and update the exception handlers around the code that
calls the image generation service and interaction.followup.send.
In `@src/images/Dockerfile`:
- Around line 1-9: The Dockerfile currently runs as root and does not install
the DejaVu font required by utils/levels.py; modify the Dockerfile to add a
non-root user (create a user/group, chown /app, and switch to that user with
USER) and install a system font package (e.g., ttf-dejavu or equivalent) during
build so DejaVuSans.ttf exists under a standard path, and then update the font
path in utils/levels.py to point to the installed font (e.g.,
/usr/share/fonts/.../DejaVuSans.ttf); ensure file ownership and permissions
allow the non-root user to run uv run main.py.
In `@src/images/pyproject.toml`:
- Around line 6-12: Update the pyproject.toml entries so they reference real,
released versions and a valid Python baseline: change requires-python from
">=3.14" to a supported runtime like ">=3.10", and replace the dependency
entries "pillow>=12.1.1", "requests>=2.33.1", "fastapi>=0.135.2", and
"uvicorn>=0.42.0" with actual existing versions available on PyPI (or use
permissive ranges like "pillow>=9.5.0", "requests>=2.31.0", "fastapi>=0.95.0",
"uvicorn>=0.22.0" as examples); ensure you update the strings inside the
dependencies array (the literal items containing those package names) so the
project can be installed.
In `@src/images/utils/levels.py`:
- Around line 80-85: progress = current_xp / max_xp can raise ZeroDivisionError
or produce invalid progress when max_xp is 0 or negative; change the logic
around the progress calculation (the variables progress, current_xp, max_xp used
before draw.rounded_rectangle) to guard max_xp <= 0 and set progress = 0.0 (or
clamp to a safe positive minimum) before computing the division, then continue
using that safe progress value for the filled-bar draw call so no
division-by-zero or negative-progress occurs.
- Around line 17-22: The code currently treats background_path as either a URL
or a local path and fetches avatar_url without validation, allowing SSRF and
arbitrary local file reads; fix by forcing background selection to a server-side
asset ID (replace direct Image.open(background_path) with a lookup map that
loads only known assets), and restrict avatar_url fetching to HTTPS host
allowlist and safe IP ranges (validate scheme == "https", resolve and reject
private/loopback IPs, and enforce a hostname whitelist) before calling
requests.get with a timeout and stream=False; also remove any code that opens
arbitrary filesystem paths from user input and add the same validation in the
endpoint that passes background_path/avatar_url so only validated values reach
the code paths using requests.get and Image.open.
---
Nitpick comments:
In `@docker-compose.yaml`:
- Around line 43-50: Add a healthcheck to the images service and make bot depend
on images' healthy state: update the images service block to include a
healthcheck (test/cmd, interval, timeout, retries) so Docker can mark it
healthy, and update the bot service to use depends_on with images: condition:
service_healthy; reference the images service name and the bot service's
depends_on key when making these edits.
In `@src/images/routers/levels.py`:
- Around line 25-37: Wrap the call to
asyncio.to_thread(generate_rank_card_blocking, ...) in a try/except that catches
Exception, logs the exception (use the module's logger or logging.getLogger)
including the function name generate_rank_card_blocking and input identifiers
(name, level, avatar_url) for context, and return an appropriate error Response
(e.g., HTTP 500 JSON error or a small fallback image) instead of letting the
handler fail silently; update the Response return path to use the successfully
generated image_data when no exception occurs and ensure the log message
includes the exception traceback to aid debugging.
- Around line 10-19: Add explicit validation for get_rank_card parameters:
ensure level is non-negative, current_xp and max_xp are non-negative and max_xp
> 0 (and optionally current_xp <= max_xp), and validate avatar_url is a proper
URL; implement this by either switching the function parameter types to
pydantic-constrained types (e.g., NonNegativeInt/PositiveInt and HttpUrl/AnyUrl)
or by adding runtime checks at the start of get_rank_card that raise
fastapi.HTTPException(400) with clear messages when level < 0, current_xp < 0,
max_xp <= 0 or current_xp > max_xp, and when avatar_url fails URL
parsing/validation; keep the existing status regex validation as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 154d9434-c18d-4bf0-bfc9-dedb73e72fb0
⛔ Files ignored due to path filters (2)
src/images/static/sea.jpgis excluded by!**/*.jpgsrc/images/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.gitignoredev.shdocker-compose.yamlsrc/bot/cogs/levels.pysrc/images/.python-versionsrc/images/Dockerfilesrc/images/README.mdsrc/images/main.pysrc/images/pyproject.tomlsrc/images/routers/levels.pysrc/images/utils/levels.py
Comment on lines
+1
to
+9
| FROM ghcr.io/astral-sh/uv:python3.14-alpine | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| COPY . . | ||
|
|
||
| RUN uv sync --no-dev --no-editable --no-cache --frozen | ||
|
|
||
| CMD ["uv", "run", "main.py"] No newline at end of file |
There was a problem hiding this comment.
セキュリティ: コンテナがrootユーザーで実行されています。また、フォントファイルが不足しています。
- rootユーザーの問題: コンテナは非rootユーザーで実行することがセキュリティのベストプラクティスです。
- フォントの不足:
utils/levels.pyでfont/DejaVuSans.ttfを読み込もうとしていますが、font/ディレクトリは.gitignoreで除外されており、Dockerビルド時にコピーされません。
🛡️ 非rootユーザーの追加とフォントのインストール
FROM ghcr.io/astral-sh/uv:python3.14-alpine
+# フォントのインストール
+RUN apk add --no-cache fontconfig ttf-dejavu
+
WORKDIR /app
COPY . .
RUN uv sync --no-dev --no-editable --no-cache --frozen
+# 非rootユーザーの作成
+RUN adduser -D -u 1000 appuser && chown -R appuser:appuser /app
+USER appuser
+
CMD ["uv", "run", "main.py"]フォントパスも更新が必要です（src/images/utils/levels.py）:
font_name = ImageFont.truetype("/usr/share/fonts/ttf-dejavu/DejaVuSans.ttf", 34)🧰 Tools
🪛 Trivy (0.69.3)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/images/Dockerfile` around lines 1 - 9, The Dockerfile currently runs as
root and does not install the DejaVu font required by utils/levels.py; modify
the Dockerfile to add a non-root user (create a user/group, chown /app, and
switch to that user with USER) and install a system font package (e.g.,
ttf-dejavu or equivalent) during build so DejaVuSans.ttf exists under a standard
path, and then update the font path in utils/levels.py to point to the installed
font (e.g., /usr/share/fonts/.../DejaVuSans.ttf); ensure file ownership and
permissions allow the non-root user to run uv run main.py.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/images/utils/levels.py (1)
13-20:⚠️ Potential issue | 🟠 Major現在の URL 検証では SSRF をまだ防ぎ切れていません。
ip.is_private or ip.is_loopbackだけだと169.254.169.254のような link-local を通しますし、Lines 46/66 のrequests.get()はリダイレクト先を再検証していません。公開 URL を 302 で内部向け URL に飛ばすだけで到達できるので、link-local/reserved/unspecified/multicast も拒否し、取得時はallow_redirects=Falseか各 hop の再検証が必要です。確認方法: 次のスクリプトで現在の IP 判定が許可してしまうアドレスと、
requestsのリダイレクト既定値を確認できます。#!/bin/bash python - <<'PY' import ipaddress def allowed_by_current_check(host): ip = ipaddress.ip_address(host) return not (ip.is_private or ip.is_loopback) for host in ["169.254.169.254", "0.0.0.0", "224.0.0.1"]: print(host, "allowed_by_current_check=", allowed_by_current_check(host)) PY python -m pip install -q requests >/dev/null 2>&1 python - <<'PY' import inspect import requests print(inspect.signature(requests.sessions.Session.request)) PY rg -n 'requests\.get\(' src/images/utils/levels.py -A1 -B1🔐 最低限の修正例
- if ip.is_private or ip.is_loopback: + if ( + ip.is_private + or ip.is_loopback + or ip.is_link_local + or ip.is_multicast + or ip.is_reserved + or ip.is_unspecified + ): return False- bg_resp = requests.get(background_path, timeout=5) + bg_resp = requests.get(background_path, timeout=5, allow_redirects=False) ... - response = requests.get(avatar_url, timeout=5) + response = requests.get(avatar_url, timeout=5, allow_redirects=False)Also applies to: 44-47, 65-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/images/utils/levels.py` around lines 13 - 20, The current hostname/IP validation that only denies ip.is_private or ip.is_loopback is insufficient; update the validation around parsed.hostname (the block that creates ip = ipaddress.ip_address(parsed.hostname)) to also reject ip.is_link_local, ip.is_reserved, ip.is_multicast, and ip.is_unspecified (and keep loopback/private checks) so addresses like 169.254.169.254 and 0.0.0.0 are blocked. Also change the HTTP fetches that call requests.get(...) in this module to use allow_redirects=False and either manually follow redirects while revalidating each new Location with the same IP checks or explicitly re-check the resolved IP for each redirect hop before requesting it. Ensure the same stricter IP checks are reused for redirect targets and reference the existing parsed.hostname check and the requests.get calls when making these changes.src/images/Dockerfile (1)
1-11:⚠️ Potential issue | 🟠 Majorコンテナがまだ root で起動しています。
USERが無いので、このサービスや依存ライブラリ側で脆弱性が出たときに、そのまま root 権限を持ちます。専用ユーザーを作成して/appを所有させたうえで、CMDの前に切り替えてください。🔒 修正例
RUN uv sync --no-dev --no-editable --no-cache --frozen + +RUN addgroup -S app && adduser -S -G app app \ + && chown -R app:app /app +USER app CMD ["uv", "run", "main.py"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/images/Dockerfile` around lines 1 - 11, Create a non-root runtime user and make /app owned by that user before the final CMD so the container does not run as root: add a step that creates a dedicated user/group (e.g., app user), use a RUN that chowns /app to that user, and add a USER instruction just before CMD to switch to the unprivileged account; update any relevant build steps that assume root by ensuring file ownership (reference Dockerfile, the /app WORKDIR, the RUN chown step, the USER instruction, and the final CMD "uv run main.py").
🧹 Nitpick comments (2)
src/api/src/internal/router/premium.go (1)
48-56: 永続モデルへ直接 Bind せず、入力 DTO を分離した方が安全です
model.Premiumへ直接ShouldBindJSONしているため、API入力とDBモデルが強結合です。入力制約（plan_typeの許可値など）を DTO で明示し、変換して保存する形に分離することを推奨します。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/router/premium.go` around lines 48 - 56, You are binding request JSON directly into the persistence model (model.Premium) via ShouldBindJSON and then using db.Where(...).Assign(input).FirstOrCreate(&input); separate the DTO from the DB model: create a request struct (e.g., PremiumCreateDTO) for binding and validation (allowed plan_type values, required fields), use c.ShouldBindJSON(&PremiumCreateDTO) to parse/validate input, map/convert the DTO to a new model.Premium instance (set UserID and any sanitized fields) and then call db.Where("user_id = ?", userID).Assign(premiumModel).FirstOrCreate(&premiumModel) to persist; update usages of ShouldBindJSON, model.Premium, and the db.Where(...).Assign(...).FirstOrCreate call accordingly.src/api/src/internal/model/Premium.go (1)
21-21:PlanTypeの文字列リテラルを定数化してください
"free"のベタ書きが複数箇所に散ると、将来の分岐漏れや typo の温床になります。const化して比較を統一するのを推奨します。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/src/internal/model/Premium.go` at line 21, PlanType フィールドで使われている文字列リテラル "free" を定数化して比較を統一してください: 新たに例えば const PlanTypeFree = "free" を定義し、Premium構造体の PlanType フィールドに関連する全ての比較・switch・テスト・ビジネスロジックで直接 "free" を使わずこの定数を参照するよう置換してください（DBタグのデフォルト値はマイグレーションやORM設定と整合するよう必要に応じて見直してください）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/src/internal/model/Premium.go`:
- Around line 8-14: IsPremium currently treats a zero-value ExpiresAt as expired
(returns false) which mismatches the router logic; update the IsPremium(p
Premium) function to mirror router/premium.go by skipping the expiry check when
p.ExpiresAt.IsZero() — i.e., after the PlanType=="free" check, return true if
ExpiresAt is the zero value, otherwise perform the
time.Now().Before(p.ExpiresAt) check so both router and IsPremium use the same
semantics.
In `@src/api/src/internal/router/premium.go`:
- Around line 34-38: The current expiry-handling block updates premium.PlanType
to "free" via db.Model(&premium).Update("plan_type", "free") but ignores the
returned error; change this to capture the result (e.g., res :=
db.Model(&premium).Update(...)) and check res.Error (and res.RowsAffected if
needed), and if an update error occurs return a 500 error response instead of
proceeding; ensure you still set premium.PlanType = "free" only on successful
update and propagate the error out of the handler so callers receive the proper
500 status.
In `@src/bot/cogs/levels.py`:
- Around line 59-62: The truncation in process_string incorrectly treats limit
as the slice length and then appends suffix, producing strings longer than
limit; update process_string to compute effective_slice = max(0, limit -
len(suffix)) and slice text to that length before appending suffix (keep
returning original text when len(text) <= limit), ensuring it handles cases
where limit is smaller than suffix length by returning only the suffix or the
appropriate empty+suffix result.
In `@src/images/utils/levels.py`:
- Around line 8-20: The is_safe_url function currently treats URLs without a
hostname as safe due to parsing behavior; update is_safe_url to explicitly
reject missing hostnames by checking parsed.hostname is truthy before calling
ipaddress.ip_address or doing any hostname checks, returning False if
parsed.hostname is None or empty; keep the existing scheme check (parsed.scheme
in ('http','https')), then validate parsed.hostname, then proceed with
ipaddress.ip_address(parsed.hostname) and the private/loopback checks and the
fallback hostname comparisons in the same function.
---
Duplicate comments:
In `@src/images/Dockerfile`:
- Around line 1-11: Create a non-root runtime user and make /app owned by that
user before the final CMD so the container does not run as root: add a step that
creates a dedicated user/group (e.g., app user), use a RUN that chowns /app to
that user, and add a USER instruction just before CMD to switch to the
unprivileged account; update any relevant build steps that assume root by
ensuring file ownership (reference Dockerfile, the /app WORKDIR, the RUN chown
step, the USER instruction, and the final CMD "uv run main.py").
In `@src/images/utils/levels.py`:
- Around line 13-20: The current hostname/IP validation that only denies
ip.is_private or ip.is_loopback is insufficient; update the validation around
parsed.hostname (the block that creates ip =
ipaddress.ip_address(parsed.hostname)) to also reject ip.is_link_local,
ip.is_reserved, ip.is_multicast, and ip.is_unspecified (and keep
loopback/private checks) so addresses like 169.254.169.254 and 0.0.0.0 are
blocked. Also change the HTTP fetches that call requests.get(...) in this module
to use allow_redirects=False and either manually follow redirects while
revalidating each new Location with the same IP checks or explicitly re-check
the resolved IP for each redirect hop before requesting it. Ensure the same
stricter IP checks are reused for redirect targets and reference the existing
parsed.hostname check and the requests.get calls when making these changes.
---
Nitpick comments:
In `@src/api/src/internal/model/Premium.go`:
- Line 21: PlanType フィールドで使われている文字列リテラル "free" を定数化して比較を統一してください: 新たに例えば const
PlanTypeFree = "free" を定義し、Premium構造体の PlanType
フィールドに関連する全ての比較・switch・テスト・ビジネスロジックで直接 "free"
を使わずこの定数を参照するよう置換してください（DBタグのデフォルト値はマイグレーションやORM設定と整合するよう必要に応じて見直してください）。
In `@src/api/src/internal/router/premium.go`:
- Around line 48-56: You are binding request JSON directly into the persistence
model (model.Premium) via ShouldBindJSON and then using
db.Where(...).Assign(input).FirstOrCreate(&input); separate the DTO from the DB
model: create a request struct (e.g., PremiumCreateDTO) for binding and
validation (allowed plan_type values, required fields), use
c.ShouldBindJSON(&PremiumCreateDTO) to parse/validate input, map/convert the DTO
to a new model.Premium instance (set UserID and any sanitized fields) and then
call db.Where("user_id = ?",
userID).Assign(premiumModel).FirstOrCreate(&premiumModel) to persist; update
usages of ShouldBindJSON, model.Premium, and the
db.Where(...).Assign(...).FirstOrCreate call accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac745857-dd25-4ab3-9abe-9308499eb74d
📒 Files selected for processing (6)
src/api/src/cmd/main.gosrc/api/src/internal/model/Premium.gosrc/api/src/internal/router/premium.gosrc/bot/cogs/levels.pysrc/images/Dockerfilesrc/images/utils/levels.py
Comment on lines
+34
to
+38
| if !premium.ExpiresAt.IsZero() && time.Now().After(premium.ExpiresAt) { | ||
| if premium.PlanType != "free" { | ||
| premium.PlanType = "free" | ||
| db.Model(&premium).Update("plan_type", "free") | ||
| } |
There was a problem hiding this comment.
期限切れ時の plan_type 更新エラーを無視しないでください
Line 37 の更新失敗が握りつぶされており、DB更新失敗時でも成功レスポンスを返してしまいます。更新エラーを検知して 500 を返すべきです。
修正案
if !premium.ExpiresAt.IsZero() && time.Now().After(premium.ExpiresAt) {
if premium.PlanType != "free" {
premium.PlanType = "free"
- db.Model(&premium).Update("plan_type", "free")
+ if err := db.Model(&premium).Update("plan_type", "free").Error; err != nil {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "設定の更新に失敗しました"})
+ return
+ }
}
}📝 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.
Suggested change
| if !premium.ExpiresAt.IsZero() && time.Now().After(premium.ExpiresAt) { | |
| if premium.PlanType != "free" { | |
| premium.PlanType = "free" | |
| db.Model(&premium).Update("plan_type", "free") | |
| } | |
| if !premium.ExpiresAt.IsZero() && time.Now().After(premium.ExpiresAt) { | |
| if premium.PlanType != "free" { | |
| premium.PlanType = "free" | |
| if err := db.Model(&premium).Update("plan_type", "free").Error; err != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": "設定の更新に失敗しました"}) | |
| return | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/src/internal/router/premium.go` around lines 34 - 38, The current
expiry-handling block updates premium.PlanType to "free" via
db.Model(&premium).Update("plan_type", "free") but ignores the returned error;
change this to capture the result (e.g., res := db.Model(&premium).Update(...))
and check res.Error (and res.RowsAffected if needed), and if an update error
occurs return a 500 error response instead of proceeding; ensure you still set
premium.PlanType = "free" only on successful update and propagate the error out
of the handler so callers receive the proper 500 status.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/bot/cogs/levels.py (1)
104-110:BytesIOをwith文で使用することを検討してください。
interaction.followup.sendで例外が発生した場合、byte.close()が呼ばれません。BytesIOはシステムリソースを保持しないため重大ではありませんが、明示的なリソース管理のためにwith文を使用するのがベストプラクティスです。♻️ 修正例
if response.status == 200: content = await response.read() - byte = io.BytesIO(content) - image_file = discord.File(byte, filename="rank_card.png") - await interaction.followup.send(file=image_file) - - byte.close() + with io.BytesIO(content) as byte: + image_file = discord.File(byte, filename="rank_card.png") + await interaction.followup.send(file=image_file)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bot/cogs/levels.py` around lines 104 - 110, The BytesIO object "byte" created from response content should be managed with a context manager to ensure it is closed even if interaction.followup.send raises; wrap the creation and usage of io.BytesIO(...) and discord.File(...) in a with block (e.g., with io.BytesIO(content) as byte:) and send the resulting discord.File(byte, filename="rank_card.png") inside that block so the BytesIO is always closed after use in the code path inside the function handling the HTTP response.src/images/utils/levels.py (2)
91-96:except:を具体的な例外に置き換えてください。フォント読み込みでも裸の
except:が使われています。OSErrorやIOErrorなど具体的な例外を指定するか、最低限except Exception:にしてください。♻️ 修正例
try: font_name = ImageFont.truetype("/usr/share/fonts/ttf-dejavu/DejaVuSans.ttf", 34) font_sub = ImageFont.truetype("/usr/share/fonts/ttf-dejavu/DejaVuSans.ttf", 18) - except: + except OSError: font_name = ImageFont.load_default() font_sub = ImageFont.load_default()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/images/utils/levels.py` around lines 91 - 96, The try/except around ImageFont.truetype is using a bare except; change it to catch specific errors (e.g., OSError or IOError) or at minimum except Exception as capture the failure when loading fonts so you don't swallow system-exiting exceptions; update the block that assigns font_name and font_sub (calls to ImageFont.truetype) to catch the specific exception(s) and optionally bind the exception to a variable for logging before falling back to ImageFont.load_default().
68-72:except:を具体的な例外に置き換えてください。裸の
except:はKeyboardInterruptやSystemExitも捕捉してしまい、予期しない動作を引き起こす可能性があります。♻️ 修正例
try: response = requests.get(avatar_url, timeout=5) avatar_raw = Image.open(io.BytesIO(response.content)).convert("RGBA") - except: + except Exception: avatar_raw = Image.new("RGBA", (100, 100), (100, 100, 100))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/images/utils/levels.py` around lines 68 - 72, Replace the bare except in the avatar download/load block with specific exceptions: wrap the requests.get call with response.raise_for_status() and catch requests.RequestException for network issues and PIL.Image.UnidentifiedImageError (and/or OSError) for image decoding errors when creating avatar_raw from Image.open; include the exception as a variable (e) to allow logging or debugging, and fall back to the placeholder Image.new("RGBA", (100, 100), (100, 100, 100)) only in those specific error cases so KeyboardInterrupt/SystemExit are not swallowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/bot/cogs/levels.py`:
- Around line 104-110: The BytesIO object "byte" created from response content
should be managed with a context manager to ensure it is closed even if
interaction.followup.send raises; wrap the creation and usage of io.BytesIO(...)
and discord.File(...) in a with block (e.g., with io.BytesIO(content) as byte:)
and send the resulting discord.File(byte, filename="rank_card.png") inside that
block so the BytesIO is always closed after use in the code path inside the
function handling the HTTP response.
In `@src/images/utils/levels.py`:
- Around line 91-96: The try/except around ImageFont.truetype is using a bare
except; change it to catch specific errors (e.g., OSError or IOError) or at
minimum except Exception as capture the failure when loading fonts so you don't
swallow system-exiting exceptions; update the block that assigns font_name and
font_sub (calls to ImageFont.truetype) to catch the specific exception(s) and
optionally bind the exception to a variable for logging before falling back to
ImageFont.load_default().
- Around line 68-72: Replace the bare except in the avatar download/load block
with specific exceptions: wrap the requests.get call with
response.raise_for_status() and catch requests.RequestException for network
issues and PIL.Image.UnidentifiedImageError (and/or OSError) for image decoding
errors when creating avatar_raw from Image.open; include the exception as a
variable (e) to allow logging or debugging, and fall back to the placeholder
Image.new("RGBA", (100, 100), (100, 100, 100)) only in those specific error
cases so KeyboardInterrupt/SystemExit are not swallowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0dcfcc31-ac46-4205-9301-ceac077ba8bb
📒 Files selected for processing (3)
src/api/src/internal/model/Premium.gosrc/bot/cogs/levels.pysrc/images/utils/levels.py
✅ Files skipped from review due to trivial changes (1)
- src/api/src/internal/model/Premium.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores