refactor(database): GORMを使用してデータベースモデルを改善 #175
Conversation
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
There was a problem hiding this comment.
Pull request overview
Ent ベースのデータアクセスを GORM に移行し、Bot の各機能（ギルド/ユーザー/メンバー管理、RolePanel、メッセージ機能、レベル機能等）を GORM モデル前提にリファクタする PR です。
Changes:
- GORM モデルを追加/拡充し、AutoMigrate 対象を拡大
- 主要コマンド/ハンドラを ent → GORM クエリへ置換
- ent→gorm へのデバッグ用移行コマンドを追加、ent の schema migration 呼び出しを削除
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-compose.yml | image 設定のコメント追加 |
| database/models/wordsuffix.go | WordSuffix GORM モデル追加 |
| database/models/user.go | User モデル拡張 |
| database/models/rolepanel.go | RolePanel 系モデル追加 |
| database/models/messageremind.go | MessageRemind モデル追加 |
| database/models/messagepin.go | MessagePin モデル追加 |
| database/models/member.go | Member モデル追加 |
| database/models/guild.go | Guild モデル拡張 |
| database/database.go | AutoMigrate 対象追加 |
| bot/components/user.go | UserCreate を GORM 化 |
| bot/components/member.go | MemberCreate を GORM 化 |
| bot/components/guild.go | Guild lifecycle を GORM 化 |
| bot/components/generic/permission.go | Permission 判定の GORM 化 |
| bot/components/components.go | ent Client 依存除去 |
| bot/commands/setting/setting.go | Setting 更新を GORM 化 |
| bot/commands/role/role.go | RolePanel 操作を GORM 化 |
| bot/commands/role/panel_message.go | RolePanel メッセージ生成を GORM 化 |
| bot/commands/role/panel_autocomplete.go | Autocomplete を GORM 化 |
| bot/commands/role/panel.go | RolePanel 配置処理を GORM 化 |
| bot/commands/role/import.go | RolePanel インポートを GORM 化 |
| bot/commands/permission/permission.go | Permission コマンドを GORM 化 |
| bot/commands/message/message.go | suffix/pin/remind を GORM 化 |
| bot/commands/level/level_message.go | level 表示のモデル更新対応 |
| bot/commands/level/level_handlers.go | level ハンドラ分割・GORM 化 |
| bot/commands/level/level_components.go | level modal/event を GORM 化 |
| bot/commands/level/level.go | level 本体を分割・GORM 化 |
| bot/commands/debug/migration.go | ent→gorm 移行コマンド追加 |
| bot/commands/debug/debug.go | debug に migration サブコマンド追加 |
| bot/bot.go | ent schema migration 呼び出し削除 |
| .gemini/sandbox.Dockerfile | gemini sandbox 用 Dockerfile 追加 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughEnt ORM から GORM ベースのデータベース モデルへの大規模な移行。コンポーネント初期化から Ent 依存性を削除し、ディバッグ コマンドにマイグレーション ハンドラーを追加。複数のコマンド セット（レベル、権限、設定、ロール）を拡張し、新しいデータベース モデル構造を導入。 Changes
Sequence DiagramsequenceDiagram
participant User as User Request
participant Handler as Migration Handler
participant Ent as Ent Database
participant Tx as GORM Transaction
participant GORM as GORM Database
participant Response as Discord Response
User->>Handler: Invoke /debug/migration/ent_to_gorm
Handler->>Handler: Defer interaction response
Handler->>Tx: Begin transaction
Tx->>Ent: Query Users
Ent-->>Tx: Return user data
Tx->>GORM: Map and save User models
GORM-->>Tx: Persist complete
Tx->>Ent: Query Guilds
Ent-->>Tx: Return guild data
Tx->>GORM: Map and save Guild models
GORM-->>Tx: Persist complete
Tx->>Ent: Query WordSuffix
Ent-->>Tx: Return suffix data
Tx->>GORM: Map and save WordSuffix models
GORM-->>Tx: Persist complete
Note over Tx: Continue for RolePanel,<br/>MessagePin, Members...
Tx->>GORM: Commit transaction
GORM-->>Tx: Transaction committed
Handler->>Response: Send "Migration complete!" message
Response-->>User: Success response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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
Comment |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
bot/commands/permission/permission.go (1)
139-152: 権限設定のSave操作でエラーハンドリングを追加してください。権限の変更はセキュリティに影響するため、
c.GormDB().Save()の失敗を検出することが重要です。エラーが無視されると、ユーザーには権限が設定されたように見えますが、実際にはデータベースに保存されていない状態になる可能性があります。🔧 エラーハンドリングの追加
t.Permission.Set(perm, value) - c.GormDB().Save(t) + if err := c.GormDB().Save(t).Error; err != nil { + return errors.NewError(err) + } mention = discord.UserMention(t.UserID)同様に Line 151, 190, 201 にも適用してください。
bot/components/member.go (1)
23-55: メンバー作成時の競合で重複エラーが発生します。First→Create の二段階は並行実行で両方が未検出になり、ユニーク制約違反で Create が失敗します。
OnConflict(DoNothing: true)で無害化し、RowsAffected が 0 の場合は再取得する形にすると安全です。🛠 提案修正
import ( "context" "github.com/disgoorg/disgo/discord" "github.com/disgoorg/snowflake/v2" "github.com/sabafly/gobot/database/models" "gorm.io/gorm" + "gorm.io/gorm/clause" ) @@ member = models.Member{ GuildID: gid, UserID: u.ID, } - if err := c.GormDB().Create(&member).Error; err != nil { - return nil, err - } + result := c.GormDB().Clauses(clause.OnConflict{DoNothing: true}).Create(&member) + if result.Error != nil { + return nil, result.Error + } + if result.RowsAffected == 0 { + if err := c.GormDB().Where("guild_id = ? AND user_id = ?", gid, u.ID).First(&member).Error; err != nil { + return nil, err + } + }bot/components/generic/permission.go (1)
116-126: ロールパーミッションチェックのロジックにバグがあります。現在のコードでは、各パーミッションに対してイテレートするループ内で、
memberRolesの全ロールをチェックしていますが、okフラグが各ロールで上書きされるため、最後に処理されたロールの判定だけが最終結果に反映されます。ロールの優先度を無視しており、ロールを並列でチェックする意味がありません。同じファイルの
PermissionCheck関数（62-94行目）では異なるロジックが使用されており、こちらは適切にロール優先度を考慮しています。RolePermissionCheckも同様に修正が必要です。bot/commands/message/message.go (1)
424-441: 語尾チェックがDBエラーを握りつぶしています
Firstが失敗しても既定の「なし」メッセージで進むため、DBエラーがユーザーに隠蔽されます。同ファイル内の他の箇所ではErrRecordNotFoundを適切に処理していることからも、ここは一貫性を欠いています。ErrRecordNotFound以外のエラーは返却してください。🛠️ 修正案
err = c.GormDB().Where("guild_id = ? AND owner_id = ?", g.ID, u.ID).First(&w).Error +if err != nil && err != gorm.ErrRecordNotFound { + return errors.NewError(err) +} if err == nil { messageStr = translate.Message(event.Locale(), "components.message.suffix.check.message", translate.WithTemplate( map[string]any{ "Duration": builtin.Or(w.Expired != nil, discord.FormattedTimestampMention(builtin.NonNil(w.Expired).Unix(), discord.TimestampStyleRelative), translate.Message(event.Locale(), "components.message.suffix.duration.none"), ), "User": discord.UserMention(u.ID), "Suffix": w.Suffix, "Rule": translate.Message(event.Locale(), "components.message.suffix.set.command.options.rule."+w.Rule), }, ), ) }
🤖 Fix all issues with AI agents
In @.gemini/sandbox.Dockerfile:
- Line 1: Replace the incomplete base image reference on the FROM line by using
the full Google Artifact Registry path
(us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox) and avoid the :latest
tag—use a fixed tag or digest instead; also update the apt-get install
invocation(s) to include --no-install-recommends and, after installation, run
apt-get clean && rm -rf /var/lib/apt/lists/* to remove APT caches (apply these
changes around the apt-get install lines in the Dockerfile).
In `@bot/commands/debug/migration.go`:
- Around line 52-105: The migration panics because several places assume edges
exist (e.g., g.Edges.Owner.ID, ws.Edges.Owner.ID, rp.Edges.Guild.ID,
rpe.Edges.Guild/Parent.ID, rpp.Edges.Guild/RolePanel.ID, p.Edges.Guild.ID,
r.Edges.Guild.ID, m.Edges.Guild/User.ID); before dereferencing any .Edges.* you
must nil-check the edge (e.g., if g.Edges != nil && g.Edges.Owner != nil {
gg.OwnerID = g.Edges.Owner.ID } or set the corresponding GuildID/OwnerID pointer
only when the edge is non-nil), apply this pattern for WordSuffix (ws), role
panel vars (rp, rpe, rpp), p, r, m, etc., and optionally log a warning or
continue when an edge is missing instead of dereferencing and panicking.
In `@bot/commands/level/level_components.go`:
- Around line 45-58: The save and message-send errors are being swallowed: check
and handle the error returned by c.GormDB().Save(g) (the call that persists g
after setting g.LevelUpMessage) and return it if non-nil, and likewise propagate
the error from event.CreateMessage instead of returning nil; update the function
to capture errors from c.GormDB().Save(g) and from event.CreateMessage(...) and
return them (or wrap with context) so failures in Save or CreateMessage are not
ignored.
- Around line 96-104: The call to c.GormDB().Save(m) ignores its returned error,
so check the result of c.GormDB().Save(m).Error (or the Save return) after
updating m.LastMessageHashes and if non-nil return errors.NewError(err)
(matching the surrounding pattern used for addXp) so the DB failure
short-circuits the handler and prevents proceeding to addXp; reference the Save
call for m, the LastMessageHashes update, and the addXp(...) invocation to
locate where to insert the error check.
In `@bot/commands/level/level_handlers.go`:
- Around line 189-205: The current flow calls c.MemberCreate to load
fromUser/toUser, then sets fromUser.XP=0 and c.GormDB().Save(fromUser) before
calling addXp; if addXp fails the XP is lost and Save errors are ignored. Wrap
the sequence in a single DB transaction (use c.GormDB().Begin()), perform the
fromUser update (checking Save error) and call addXp with the transaction DB
pointer (change addXp signature to accept *gorm.DB), ensure addXp uses that tx
for its updates, and rollback the tx on any error or commit only after addXp
succeeds; also propagate/save any errors from Save and the transaction.
- Around line 410-437: The http.Get call in the loop (building url and paging)
lacks a timeout and response.Body is not always closed on error paths, causing
possible hangs and resource leaks; replace http.Get with an http.Client or
request using a context-with-timeout (e.g., create a client with Timeout or use
context.WithTimeout for the request) when calling the Mee6 endpoint, ensure that
whenever a non-nil response is received you always close response.Body (use
defer response.Body.Close() immediately after verifying response != nil) and
handle closing before returning on error branches (including before constructing
and returning errors in the status check and decode error paths) so the loop
(and functions that reference url, the paging loop, and the response handling
code) never leaks bodies or blocks indefinitely.
In `@bot/commands/message/message.go`:
- Around line 676-678:
m.BeforeIDの保存でエラーチェックが抜けており失敗が無視されていますので、component.GormDB().Save(&m)
の戻り値（エラー）を確認してエラー発生時は適切に返すように修正してください。具体的には Save の結果を受け取り（例: tx :=
component.GormDB().Save(&m)）、tx.Error
を他のDB操作と同様に処理してエラーを返すかロールバックするロジックを追加し、関数内の既存のエラーハンドリングパターン（他の Save/Delete
操作）と整合させてください。
- Around line 290-312: The current logic treats any error from
c.GormDB().Where(...).First(&w).Error as "not found" and proceeds to create a
new models.WordSuffix; change this so only gorm.ErrRecordNotFound triggers the
create flow and any other error is returned immediately. Specifically, after
calling First(&w) check if err != nil { if !errors.Is(err,
gorm.ErrRecordNotFound) { return errors.NewError(err) } /* proceed to create */
} and keep the existing update path when err == nil; reference the First call,
the models.WordSuffix variable w, c.GormDB(), Save and Create calls, and
uuidv7.New() when constructing the new record.
In `@bot/commands/role/panel_message.go`:
- Around line 86-88: The Count() call that populates placeCount
(c.GormDB().Model(&models.RolePanelPlaced{}).Where("role_panel_id = ?",
panel.ID).Count(&placeCount)) can fail and leave placeCount at 0, which will
incorrectly affect the Apply/Place button disabled logic; capture the call
result (check .Error), and on error either log the error (using the existing
logger/context) and treat the state conservatively or return the error to the
caller so the caller can decide; update the code around placeCount handling to
use the checked error path and avoid proceeding with an unverified 0 value.
In `@bot/commands/role/role.go`:
- Around line 186-191: Multiple GORM Save/Create/Delete calls (e.g.,
c.GormDB().Save(&rolePanel), c.GormDB().Create(&edit), and other
c.GormDB().Delete/Save usages across this file) ignore returned errors; update
each DB operation to check the returned error from the GORM call and return or
handle it consistently (wrap it with errors.NewError(err) where other handlers
use that pattern). Specifically, after calls like c.GormDB().Save(&rolePanel),
c.GormDB().Create(&edit) and any Delete/Create/Save invocations referenced in
the review (lines around rolePanel, models.RolePanelEdit, and the other noted
spots), capture result := c.GormDB().Save(...); if result.Error != nil { return
errors.NewError(result.Error) } (or the project’s standard error propagation),
ensuring UpdatedAt changes and downstream logic only run on success.
- Around line 246-254: When deleting a role panel, the code currently ignores
errors from message deletion and DB deletes; wrap the whole operation in a DB
transaction using c.GormDB().Transaction, check and handle errors from
event.Client().Rest.DeleteMessage (for each place where place.MessageID != nil)
and from the DB delete calls (Delete on models.RolePanelPlaced, RolePanelEdit,
and c.GormDB().Delete(&panel)), log failures with context (panel.ID,
place.ChannelID, *place.MessageID) and roll back the transaction on any error so
partial deletes cannot leave orphaned records; ensure the handler returns an
error if the transaction fails so callers know the operation did not complete.
- Around line 502-506: The code is dereferencing edit.Name and edit.Description
without nil checks, causing a panic if either is nil; update the block that sets
panel.UpdatedAt, panel.Name, panel.Description (the assignment using *edit.Name
and *edit.Description) to first check if edit.Name != nil and edit.Description
!= nil (or handle each independently) before dereferencing and assigning, only
assign panel.Name/panel.Description when the corresponding edit field is
non-nil, keep panel.UpdatedAt set as needed, and retain the existing conditional
update of panel.Roles and the c.GormDB().Save(&panel) call; also ensure that
callers of the save_change action populate edit.Name/edit.Description or handle
their absence upstream.
In `@bot/components/guild.go`:
- Around line 118-143: The Transaction closure in c.GormDB().Transaction always
returns nil even when delete operations fail, so commits occur instead of
rollbacks; update the closure used in c.GormDB().Transaction to return the first
non-nil error from each tx.Where(...).Delete(...) call (or wrap it with context
using fmt.Errorf) instead of only calling slog.Error, ensuring that any error
from tx.Delete(&models.Guild{ID: event.Guild.ID}) or the other models (Member,
MessagePin, MessageRemind, RolePanelPlaced, RolePanelEdit, RolePanel,
WordSuffix) is returned so GORM will roll back the transaction.
In `@bot/components/user.go`:
- Around line 32-50: Move the new-user logging to after the DB operation and
decide creation from the GORM result: call c.GormDB().FirstOrCreate(&user,
models.User{ID: u.ID}) first (capture the returned *gorm.DB as, e.g., res), then
log the "新規ユーザー作成" only when res.Error == nil && res.RowsAffected == 1 &&
!user.CreatedAt.IsZero() (this ensures the log runs only for an actually created
record in UserCreate).
In `@database/models/guild.go`:
- Around line 37-38: The OwnerID field on the Guild model is non-nullable
(snowflake.ID) but the Owner relation uses constraint:OnDelete:SET NULL which
can cause DB errors; update the model so the foreign key and constraint agree:
either make OwnerID nullable by changing its type to *snowflake.ID (and keep
gorm tag for column/index) so SET NULL can succeed, or keep OwnerID non-nullable
and change the Owner relation constraint to OnDelete:RESTRICT or
OnDelete:CASCADE; update the Guild struct fields OwnerID and/or Owner
accordingly to reflect the chosen behavior.
In `@database/models/messagepin.go`:
- Around line 45-49: The BeforeCreate hook on the MessagePin model uses the
wrong signature so GORM won't call it: change the method receiver signature from
func (m *MessagePin) BeforeCreate(tx gorm.DB) error to accept a pointer to
gorm.DB (i.e., *gorm.DB) so GORM recognizes the hook; update the function
declaration for MessagePin.BeforeCreate accordingly and keep the existing UUID
generation logic inside the function.
🧹 Nitpick comments (11)
.gemini/sandbox.Dockerfile (1)
3-4: APTの後片付けと--no-install-recommendsでイメージ肥大化を抑止。現状だと不要パッケージとAPTキャッシュが残り、サイズ増加と脆弱面の拡大につながります。
♻️ 例: APTクリーンアップ
-RUN apt-get update && \ - apt-get install -y build-essential golang-go git +RUN apt-get update && \ + apt-get install -y --no-install-recommends build-essential golang-go git && \ + rm -rf /var/lib/apt/lists/*bot/commands/role/role.go (1)
828-838: ループ内のSave操作でエラーを確認してください。
updateRolePanel関数内で複数のplaceを更新する際、Saveのエラーが無視されています。一部の更新が失敗した場合でも処理が続行されます。♻️ エラーログの追加
for _, place := range places { place.Name, place.Description, place.Roles, place.UpdatedAt = panel.Name, panel.Description, panel.Roles, time.Now() - c.GormDB().Save(&place) + if err := c.GormDB().Save(&place).Error; err != nil { + slog.Error("プレースの更新に失敗", "place_id", place.ID, "err", err) + continue + } if err := rolePanelPlace(ctx, &place, locale, client, react, c); err != nil { slog.Error("アップデートに失敗", "err", err) } }bot/commands/setting/setting.go (1)
180-182: Save操作のエラーチェックを追加することを推奨します。設定変更時の
c.GormDB().Save(g)でエラーハンドリングがありません。データベースへの書き込みが失敗した場合、ユーザーに成功メッセージが表示されますが、実際には変更が保存されていない状態になります。♻️ エラーハンドリングの追加例
g.BumpEnabled = !g.BumpEnabled - c.GormDB().Save(g) + if err := c.GormDB().Save(g).Error; err != nil { + return errors.NewError(err) + }bot/commands/role/panel_autocomplete.go (1)
41-44: LIKEクエリのワイルドカード文字のエスケープを検討してください。ユーザー入力に
%や_が含まれている場合、LIKE検索で意図しない結果が返される可能性があります。これはセキュリティ問題ではありませんが、ユーザー体験に影響する可能性があります。♻️ エスケープ処理の追加例
+ import "strings" + + func escapeLike(s string) string { + s = strings.ReplaceAll(s, "\\", "\\\\") + s = strings.ReplaceAll(s, "%", "\\%") + s = strings.ReplaceAll(s, "_", "\\_") + return s + } + var panels []models.RolePanel - if err := c.GormDB().Where("guild_id = ? AND name LIKE ?", g.ID, "%"+event.Data.String("panel")+"%").Find(&panels).Error; err != nil { + if err := c.GormDB().Where("guild_id = ? AND name LIKE ?", g.ID, "%"+escapeLike(event.Data.String("panel"))+"%").Find(&panels).Error; err != nil { return errors.NewError(err) }database/models/messageremind.go (1)
11-20: Timeフィールドにインデックスの追加を検討してください。リマインダーは時間でクエリされることが多いと思われます。
Timeフィールドにインデックスを追加すると、スケジューラーでの検索パフォーマンスが向上します。♻️ インデックスの追加
- Time time.Time + Time time.Time `gorm:"index"`database/models/wordsuffix.go (1)
17-19: ギルド削除時のFK動作を明示しておくと安全です。GuildID が NULL 許容なのに削除時の挙動が未指定だと、ギルド削除時に制約エラーになる可能性があります。期待する挙動（SET NULL / CASCADE）を明示することを検討してください。
🛠 提案修正
- Guild *Guild `gorm:"foreignKey:GuildID"` + Guild *Guild `gorm:"foreignKey:GuildID;constraint:OnDelete:SET NULL;"`bot/components/guild.go (3)
60-71: GORMクエリのエラーチェックが欠落しています。
Findの結果に対するエラーチェックが行われていません。データベースエラーが発生した場合、空のスライスで処理が続行されます。♻️ エラーハンドリングの追加
var ownedGuilds []models.Guild - c.GormDB().Where("owner_id = ?", u.ID).Find(&ownedGuilds) + if err := c.GormDB().Where("owner_id = ?", u.ID).Find(&ownedGuilds).Error; err != nil { + slog.Error("所有ギルドの取得に失敗", "err", err) + } // For joined guilds, need a join query via Members var joinedMembers []models.Member - c.GormDB().Preload("Guild").Where("user_id = ?", u.ID).Find(&joinedMembers) + if err := c.GormDB().Preload("Guild").Where("user_id = ?", u.ID).Find(&joinedMembers).Error; err != nil { + slog.Error("参加ギルドの取得に失敗", "err", err) + }
99-109: 重複コードパターン：OnGuildReadyと同じエラーチェック欠落。OnGuildReadyと同じパターンが重複しており、同様にエラーチェックが欠落しています。共通ヘルパー関数への抽出を検討してください。
173-179: 関数名が実際の動作と一致していません。
GuildCreateIDという名前ですが、実際にはギルドを作成せず、既存のギルドを取得するだけです。GuildGetByIDやGuildFindByIDのような名前が適切です。bot/commands/role/panel.go (2)
38-42: 定数をmodelsパッケージに移動することを検討してください。これらの定数は
models.RolePanelPlaced.Typeフィールドで使用されるため、database/modelsパッケージで定義した方が、他のファイルからも一貫して参照できます。
83-84: 不完全なプレースメントの削除でエラーチェックが欠落しています。クリーンアップ処理の
Deleteでエラーが発生した場合、後続の処理に影響する可能性があります。♻️ エラーハンドリングの追加
// Clean up incomplete placements - c.GormDB().Where("(message_id IS NULL OR type = '') AND guild_id = ?", g.ID).Delete(&models.RolePanelPlaced{}) + if err := c.GormDB().Where("(message_id IS NULL OR type = '') AND guild_id = ?", g.ID).Delete(&models.RolePanelPlaced{}).Error; err != nil { + // ログ出力のみ、処理は継続 + slog.Warn("不完全なプレースメントの削除に失敗", "err", err, "guild_id", g.ID) + }
Comment on lines
52
to
105
| for _, g := range guilds { | ||
| gg := gormModels.Guild{ | ||
| ID: g.ID, | ||
| Name: g.Name, | ||
| Locale: g.Locale, | ||
| LevelUpMessage: g.LevelUpMessage, | ||
| LevelUpChannel: g.LevelUpChannel, | ||
| LevelUpExcludeChannel: g.LevelUpExcludeChannel, | ||
| LevelMee6Imported: g.LevelMee6Imported, | ||
| LevelRole: g.LevelRole, | ||
| Permissions: g.Permissions, | ||
| RemindCount: g.RemindCount, | ||
| RolePanelEditTimes: g.RolePanelEditTimes, | ||
| BumpEnabled: g.BumpEnabled, | ||
| BumpMessageTitle: g.BumpMessageTitle, | ||
| BumpMessage: g.BumpMessage, | ||
| BumpRemindMessageTitle: g.BumpRemindMessageTitle, | ||
| BumpRemindMessage: g.BumpRemindMessage, | ||
| UpEnabled: g.UpEnabled, | ||
| UpMessageTitle: g.UpMessageTitle, | ||
| UpMessage: g.UpMessage, | ||
| UpRemindMessageTitle: g.UpRemindMessageTitle, | ||
| UpRemindMessage: g.UpRemindMessage, | ||
| BumpMention: g.BumpMention, | ||
| UpMention: g.UpMention, | ||
| LevelingDisabled: g.LevelingDisabled, | ||
| OwnerID: g.Edges.Owner.ID, | ||
| } | ||
| if err := tx.Save(&gg).Error; err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| // 3. WordSuffix | ||
| slog.Info("Migrating WordSuffix...") | ||
| wordSuffixes, err := entClient.WordSuffix.Query().WithGuild().WithOwner().All(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| for _, ws := range wordSuffixes { | ||
| gws := gormModels.WordSuffix{ | ||
| ID: ws.ID, | ||
| Suffix: ws.Suffix, | ||
| Expired: ws.Expired, | ||
| OwnerID: ws.Edges.Owner.ID, | ||
| Rule: string(ws.Rule), | ||
| } | ||
| if ws.Edges.Guild != nil { | ||
| gws.GuildID = &ws.Edges.Guild.ID | ||
| } | ||
| if err := tx.Save(&gws).Error; err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the migration.go file
find . -name "migration.go" -type f 2>/dev/null | head -20Repository: sabafly/gobot
Length of output: 90
🏁 Script executed:
# Get the file size to determine how to read it
wc -l bot/commands/debug/migration.go 2>/dev/null || echo "File not found in expected location"Repository: sabafly/gobot
Length of output: 92
🏁 Script executed:
# Search for migration.go in the codebase
fd -t f "migration\.go" 2>/dev/nullRepository: sabafly/gobot
Length of output: 88
🏁 Script executed:
cat -n bot/commands/debug/migration.goRepository: sabafly/gobot
Length of output: 9584
複数のエッジで nil 参照による panic のリスク
直接的なエッジ参照が複数箇所で nil チェックなしに実行されており、過去データにエッジが欠けている場合に移行全体が失敗します。
- 行 78:
g.Edges.Owner.ID- nil チェックなし - 行 96:
ws.Edges.Owner.ID- nil チェックなし - 行 120:
rp.Edges.Guild.ID- nil チェックなし - 行 146-147:
rpe.Edges.Guild/Parent.ID- nil チェックなし - 行 179-180:
rpp.Edges.Guild/RolePanel.ID- nil チェックなし - 行 200:
p.Edges.Guild.ID- nil チェックなし - 行 226:
r.Edges.Guild.ID- nil チェックなし - 行 246-247:
m.Edges.Guild/User.ID- nil チェックなし
行 99-101 で WordSuffix の Guild エッジはチェックされているのに対し、他のエッジや他の移行では防御が不足しています。すべてのエッジ参照の前に nil チェックを追加してください。
🤖 Prompt for AI Agents
In `@bot/commands/debug/migration.go` around lines 52 - 105, The migration panics
because several places assume edges exist (e.g., g.Edges.Owner.ID,
ws.Edges.Owner.ID, rp.Edges.Guild.ID, rpe.Edges.Guild/Parent.ID,
rpp.Edges.Guild/RolePanel.ID, p.Edges.Guild.ID, r.Edges.Guild.ID,
m.Edges.Guild/User.ID); before dereferencing any .Edges.* you must nil-check the
edge (e.g., if g.Edges != nil && g.Edges.Owner != nil { gg.OwnerID =
g.Edges.Owner.ID } or set the corresponding GuildID/OwnerID pointer only when
the edge is non-nil), apply this pattern for WordSuffix (ws), role panel vars
(rp, rpe, rpp), p, r, m, etc., and optionally log a warning or continue when an
edge is missing instead of dereferencing and panicking.
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
bot/commands/message/message.go (2)
740-754: スケジューラ内のDB操作エラーが無視されています。Find()とDelete()の両操作でエラーチェックが実施されていません。GORMの公式ドキュメントでは、ファイナライザメソッド後に必ず
.Errorをチェックすることが推奨されています。現在のコードではDB操作が失敗してもnilを返すため、リマインダーの重複送信や喪失につながる可能性があります。取得失敗時や削除失敗時は即座にエラーを返すべきです。
- c.GormDB().Where("time < ?", time.Now()).Find(&reminds) + if err := c.GormDB().Where("time < ?", time.Now()).Find(&reminds).Error; err != nil { + return err + }- c.GormDB().Where("time < ?", time.Now()).Delete(&models.MessageRemind{}) + if err := c.GormDB().Where("time < ?", time.Now()).Delete(&models.MessageRemind{}).Error; err != nil { + return err + }
428-441: DB取得エラーが黙殺されます。
「未登録」と「DB障害」を区別していないため、DB障害が隠れて本番環境で検出されません。First()で実際のエラーが発生した場合でも、デフォルトメッセージが送信されるため、問題が気づかれません。修正案
var w models.WordSuffix err = c.GormDB().Where("guild_id = ? AND owner_id = ?", g.ID, u.ID).First(&w).Error if err == nil { messageStr = translate.Message(event.Locale(), "components.message.suffix.check.message", translate.WithTemplate( map[string]any{ "Duration": builtin.Or(w.Expired != nil, discord.FormattedTimestampMention(builtin.NonNil(w.Expired).Unix(), discord.TimestampStyleRelative), translate.Message(event.Locale(), "components.message.suffix.duration.none"), ), "User": discord.UserMention(u.ID), "Suffix": w.Suffix, "Rule": translate.Message(event.Locale(), "components.message.suffix.set.command.options.rule."+w.Rule), }, ), ) +} else if !errors.Is(err, gorm.ErrRecordNotFound) { + return errors.NewError(err) }GORM v2ではエラーがラップされることがあるため、
errors.Is()を使用することが推奨されます。bot/commands/permission/permission.go (1)
139-154: ロール分岐と unset のいずれかの Save エラー処理が未対応です。member 分岐の Set 処理はエラーハンドリング済みですが、以下の3つの Save() 呼び出しが失敗時のエラーチェックを欠いており、DB 保存に失敗してもレスポンスは成功になります：
- Set コマンドのロール分岐（~154行目）
- Unset コマンドのメンバー分岐（~194行目）
- Unset コマンドのロール分岐（~204行目）
各 Save() に
.Errorチェックと早期 return を追加してください。修正パターン
- c.GormDB().Save(g) + if err := c.GormDB().Save(g).Error; err != nil { + return errors.NewError(err) + }- c.GormDB().Save(t) + if err := c.GormDB().Save(t).Error; err != nil { + return errors.NewError(err) + }
🤖 Fix all issues with AI agents
In `@bot/commands/level/level_handlers.go`:
- Around line 265-276: All database Save calls currently ignore errors; update
each handler (upMessageChannelHandler, excludeChannelAddHandler,
excludeChannelRemoveHandler, excludeChannelClearHandler, resetHandler,
roleSetHandler, roleRemoveHandler) to capture the GORM result (e.g., res :=
c.GormDB().Save(...)) and check res.Error, returning errors.NewError(res.Error)
on failure so the function does not silently succeed; apply this pattern for
every c.GormDB().Save invocation in those functions.
- Around line 456-469: Handle and propagate errors for all operations in the
Mee6 import loop: check the error returned by snowflake.Parse(player.ID) and
return it on failure; check and return the error from c.MemberCreate(event, ...)
before using m; check and return errors from c.GormDB().Save(m) after saving the
member; and finally check and return the error from c.GormDB().Save(g) after
setting g.LevelMee6Imported = true. Make sure each failing operation aborts the
function and returns the encountered error so partial updates are not silently
ignored.
- Around line 114-167: The DB Count/Find calls in leaderboardHandler currently
ignore errors, causing silent success on DB failures; capture the GORM result
for the Model(...).Count(&count) and the
Where(...).Order(...).Offset(...).Limit(...).Find(&members) calls (use a
variable like res := c.GormDB()... ) and if res.Error != nil return
errors.NewError(res.Error) so the handler fails fast on DB errors; keep using
models.Member, c.GormDB(), g.ID and members variables to locate the calls.
In `@bot/commands/message/message.go`:
- Around line 924-931: The Delete call for models.MessagePin ignores errors;
change the c.GormDB().Delete(&m) invocation to check its returned .Error, and if
non-nil log it (e.g. with slog.Error including context "ピン留め削除失敗", channel id
and message id) and return the error to the caller (following the same pattern
used by other Delete operations in this file). Use the existing
models.MessagePin variable (m) and the same c.GormDB() call so the change is
minimal and consistent with surrounding code.
- Around line 366-386: The err comparison for a missing record should use
errors.Is instead of ==; in the block that queries models.WordSuffix (using
c.GormDB().Where(...).First(&w).Error) replace the direct equality check "if err
== gorm.ErrRecordNotFound" with "if errors.Is(err, gorm.ErrRecordNotFound)" and
ensure the standard "errors" package is imported; keep the surrounding logic
(event.CreateMessage, discord message building, and Delete(&w) handling)
unchanged.
- Around line 790-803: The guild-specific DB query
(c.GormDB().Where(...).First(&w)) currently swallows non-NotFound errors and
falls back to the global query; change it to check errors using errors.Is(err,
gorm.ErrRecordNotFound) so only NotFound falls through to the global lookup, and
for any other err log it (slog.Error with the err) and stop processing (e.g.,
goto messagePin or return as appropriate). Update the branch around the
c.GormDB() call that uses u.ID and e.GuildID to handle non-NotFound errors
immediately instead of silently continuing to the global query, keeping the
subsequent global query logic unchanged.
- Around line 616-618: The DB call that loads reminds ignores GORM errors;
capture the result of c.GormDB().Where(...).Limit(25).Find(&reminds) (related
symbols: c.GormDB(), models.MessageRemind, reminds, event.GuildID(),
event.Data.String("remind")) and check result.Error (or use .Error on the
chained call). If an error is non-nil, return or handle it appropriately (e.g.,
return the error from the enclosing function or send an error response) instead
of proceeding as if the query succeeded.
- Around line 642-657: The deletion of the existing pin ignores errors—check and
handle errors from both event.Client().Rest.DeleteMessage(...) and
component.GormDB().Delete(&oldPin) so failures don't leave stale DB state; after
calling DeleteMessage, capture its error and return errors.NewError(err) (or log
and return) on failure, and likewise check the Error returned by
component.GormDB().Delete(&oldPin) and propagate it via errors.NewError(err)
before proceeding to create the new models.MessagePin record.
- Around line 706-719: The code increments g.RemindCount and calls
c.GormDB().Save(g) but ignores Save's error; update the block handling
models.MessageRemind so after g.RemindCount++ you capture and check the result
of c.GormDB().Save(g). If Save returns an error, return errors.NewError(err)
(same pattern as used for Create) to avoid count inconsistency; ensure you
reference the same g variable and use c.GormDB().Save(g) for the save operation.
- Around line 945-947: w.Expired チェック後の c.GormDB().Delete(w) 呼び出しで GORM
の返却結果を無視しています；Delete の戻り値を受け取り result.Error
を確認し、エラーがあれば適切に返すかログ出力して処理を中断するよう修正してください（参考箇所: w.Expired と c.GormDB().Delete(w)
を使っているブロック）。必要なら result.RowsAffected も確認して期待した削除が行われたかを検証してください。
In `@bot/commands/role/role.go`:
- Around line 391-392: GuildCreateID's returned error is currently ignored
causing g to be potentially nil and used later (e.g., where g, _ :=
c.GuildCreateID(event, *event.GuildID()) is called); change each call to capture
and check the error (e.g., g, err := c.GuildCreateID(...); if err != nil {
return err / abort processing } ) before using g so you don't dereference nil;
apply the same fix to the other occurrences noted (the calls around the
RolePanelEdit usage and the similar sites at the other occurrences).
- Around line 348-350: The code calls c.GormDB().Where("id = ?",
edit.ParentID).First(&panel) and then initialize(&edit, &panel) without checking
the DB result; ensure you check the returned error from First() (and handle
gorm.ErrRecordNotFound) before using panel: if the query fails or no record is
found for edit.ParentID (or later for place.RolePanelID usage), return early
with an appropriate error/response instead of proceeding with a zero-value
panel; update the code paths around the panel variable and initialize(&edit,
&panel) to perform this error check and short-circuit on failure.
🧹 Nitpick comments (2)
bot/components/guild.go (1)
159-205: GuildCreate 関数が既存ギルドの Name/OwnerID を更新していません。現在の実装は既存ギルドに対して早期リターンしているため、Discord 上でのオーナー移譲やリネームが DB に反映されません。GORM の
FirstOrCreateとAssignを組み合わせることで、既存レコード時にも常に最新の情報で更新できます。♻️ 変更例
- err := c.GormDB().Where("id = ?", g.ID).First(&guild).Error - if err == nil { - return &guild, nil - } - if err != gorm.ErrRecordNotFound { - return nil, err - } + err := c.GormDB(). + Where("id = ?", g.ID). + Assign(models.Guild{Name: g.Name, OwnerID: &ownerID}). + FirstOrCreate(&guild).Error + if err != nil { + return nil, err + } + return &guild, nilbot/commands/setting/setting.go (1)
180-183: Save() は全カラムを上書きするため、並行更新時にデータロスのリスクがあります。並行して別の設定が更新された場合、直前に読み込んだ値で他の列を上書きする可能性があります。変更対象の列だけを
Update()で更新する方式に変更してください。♻️ 変更例（bump toggle）
- g.BumpEnabled = !g.BumpEnabled - if err := c.GormDB().Save(g).Error; err != nil { + g.BumpEnabled = !g.BumpEnabled + if err := c.GormDB(). + Model(&models.Guild{ID: g.ID}). + Update("bump_enabled", g.BumpEnabled).Error; err != nil { return errors.NewError(err) }Lines 180-183, 205-208, 231-237, 267-273, 438-441, 460-466, 478-484 すべてで同じ対応が必要です。
Comment on lines
+114
to
+167
| func leaderboardHandler(c *components.Components, event *events.ApplicationCommandInteractionCreate) errors.Error { | ||
| const pageCount = 25 | ||
| g, err := c.GuildCreateID(event, *event.GuildID()) | ||
| if err != nil { | ||
| return errors.NewError(err) | ||
| } | ||
| gl, err := c.GuildRequest(event.Client(), g.ID) | ||
| if err != nil { | ||
| return errors.NewError(err) | ||
| } | ||
| page := event.SlashCommandInteractionData().Int("page") | ||
| if page < 1 { | ||
| page = 1 | ||
| } | ||
|
|
||
| var count int64 | ||
| c.GormDB().Model(&models.Member{}).Where("guild_id = ?", g.ID).Count(&count) | ||
|
|
||
| if int64(page) > (count+pageCount-1)/pageCount { | ||
| return errors.NewError(errors.ErrorMessage("errors.invalid.page", event)) | ||
| } | ||
|
|
||
| var members []models.Member | ||
| c.GormDB().Where("guild_id = ?", g.ID).Order("xp desc").Offset((page - 1) * pageCount).Limit(pageCount).Find(&members) | ||
|
|
||
| var leaderboard string | ||
| for i, m := range members { | ||
| leaderboard += fmt.Sprintf("**#%d | %s XP: `%d` Level: `%d`**\n", | ||
| i+1+((page-1)*pageCount), | ||
| discord.UserMention(m.UserID), | ||
| m.XP, m.XP.Level(), | ||
| ) | ||
| } | ||
|
|
||
| embed := discord.NewEmbedBuilder(). | ||
| SetEmbedAuthor( | ||
| &discord.EmbedAuthor{ | ||
| Name: g.Name, | ||
| IconURL: builtin.NonNil(gl.IconURL()), | ||
| }, | ||
| ). | ||
| SetTitlef("🏆%s(%d/%d)", | ||
| translate.Message(event.Locale(), "components.level.leaderboard.title"), | ||
| page, | ||
| (count+pageCount-1)/pageCount, | ||
| ). | ||
| SetDescription(leaderboard). | ||
| Build() | ||
|
|
||
| if err := event.CreateMessage(discord.NewMessageBuilder().SetEmbeds(embeds.SetEmbedProperties(embed)).BuildCreate()); err != nil { | ||
| return errors.NewError(err) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and the specific file mentioned
find . -type f -name "level_handlers.go" | head -5Repository: sabafly/gobot
Length of output: 95
🏁 Script executed:
# Check GORM usage patterns in the codebase
rg "\.Error\s*;" --type go -A 1 | head -30Repository: sabafly/gobot
Length of output: 1572
🏁 Script executed:
# Look for how Count() and Find() are used elsewhere
rg "Count\(&" --type go -B 2 -A 2 | head -40Repository: sabafly/gobot
Length of output: 2430
🏁 Script executed:
# Check imports and GORM version
rg "gorm|database/sql" bot/commands/level/level_handlers.go -B 2 -A 2Repository: sabafly/gobot
Length of output: 329
🏁 Script executed:
sed -n '114,167p' ./bot/commands/level/level_handlers.goRepository: sabafly/gobot
Length of output: 1600
DBクエリのエラーが握りつぶされています。
Count/Find のエラー未確認は空データで成功扱いになるため、明示的に返してください。
🐛 修正案
var count int64
- c.GormDB().Model(&models.Member{}).Where("guild_id = ?", g.ID).Count(&count)
+ if err := c.GormDB().Model(&models.Member{}).Where("guild_id = ?", g.ID).Count(&count).Error; err != nil {
+ return errors.NewError(err)
+ }
var members []models.Member
- c.GormDB().Where("guild_id = ?", g.ID).Order("xp desc").Offset((page - 1) * pageCount).Limit(pageCount).Find(&members)
+ if err := c.GormDB().Where("guild_id = ?", g.ID).Order("xp desc").Offset((page - 1) * pageCount).Limit(pageCount).Find(&members).Error; err != nil {
+ return errors.NewError(err)
+ }🤖 Prompt for AI Agents
In `@bot/commands/level/level_handlers.go` around lines 114 - 167, The DB
Count/Find calls in leaderboardHandler currently ignore errors, causing silent
success on DB failures; capture the GORM result for the Model(...).Count(&count)
and the Where(...).Order(...).Offset(...).Limit(...).Find(&members) calls (use a
variable like res := c.GormDB()... ) and if res.Error != nil return
errors.NewError(res.Error) so the handler fails fast on DB errors; keep using
models.Member, c.GormDB(), g.ID and members variables to locate the calls.
Comment on lines
+265
to
+276
| func upMessageChannelHandler(c *components.Components, event *events.ApplicationCommandInteractionCreate) errors.Error { | ||
| g, err := c.GuildCreateID(event, *event.GuildID()) | ||
| if err != nil { | ||
| return errors.NewError(err) | ||
| } | ||
| if channel, ok := event.SlashCommandInteractionData().OptChannel("channel"); ok { | ||
| g.LevelUpChannel = &channel.ID | ||
| } else { | ||
| g.LevelUpChannel = nil | ||
| } | ||
| c.GormDB().Save(g) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the file exists and get its total line count
wc -l bot/commands/level/level_handlers.goRepository: sabafly/gobot
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Now read the relevant sections of the file
echo "=== Lines 265-276 ==="
sed -n '265,276p' bot/commands/level/level_handlers.go
echo -e "\n=== Lines 300-311 ==="
sed -n '300,311p' bot/commands/level/level_handlers.go
echo -e "\n=== Lines 326-338 ==="
sed -n '326,338p' bot/commands/level/level_handlers.go
echo -e "\n=== Lines 353-360 ==="
sed -n '353,360p' bot/commands/level/level_handlers.go
echo -e "\n=== Lines 477-485 ==="
sed -n '477,485p' bot/commands/level/level_handlers.go
echo -e "\n=== Lines 500-528 ==="
sed -n '500,528p' bot/commands/level/level_handlers.go
echo -e "\n=== Lines 547-561 ==="
sed -n '547,561p' bot/commands/level/level_handlers.goRepository: sabafly/gobot
Length of output: 3915
GORM の Save() のエラーをすべて確認してください。
データベースへの保存に失敗しても、関数は成功として返される可能性があります。7 箇所すべてで Save(...).Error を確認し、エラーの場合は返却してください：
- 265-276 行:
upMessageChannelHandler - 300-311 行:
excludeChannelAddHandler - 326-338 行:
excludeChannelRemoveHandler - 353-360 行:
excludeChannelClearHandler - 477-485 行:
resetHandler - 500-528 行:
roleSetHandler - 547-561 行:
roleRemoveHandler
修正例
- c.GormDB().Save(g)
+ if err := c.GormDB().Save(g).Error; err != nil {
+ return errors.NewError(err)
+ }📝 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
| func upMessageChannelHandler(c *components.Components, event *events.ApplicationCommandInteractionCreate) errors.Error { | |
| g, err := c.GuildCreateID(event, *event.GuildID()) | |
| if err != nil { | |
| return errors.NewError(err) | |
| } | |
| if channel, ok := event.SlashCommandInteractionData().OptChannel("channel"); ok { | |
| g.LevelUpChannel = &channel.ID | |
| } else { | |
| g.LevelUpChannel = nil | |
| } | |
| c.GormDB().Save(g) | |
| func upMessageChannelHandler(c *components.Components, event *events.ApplicationCommandInteractionCreate) errors.Error { | |
| g, err := c.GuildCreateID(event, *event.GuildID()) | |
| if err != nil { | |
| return errors.NewError(err) | |
| } | |
| if channel, ok := event.SlashCommandInteractionData().OptChannel("channel"); ok { | |
| g.LevelUpChannel = &channel.ID | |
| } else { | |
| g.LevelUpChannel = nil | |
| } | |
| if err := c.GormDB().Save(g).Error; err != nil { | |
| return errors.NewError(err) | |
| } |
🤖 Prompt for AI Agents
In `@bot/commands/level/level_handlers.go` around lines 265 - 276, All database
Save calls currently ignore errors; update each handler
(upMessageChannelHandler, excludeChannelAddHandler, excludeChannelRemoveHandler,
excludeChannelClearHandler, resetHandler, roleSetHandler, roleRemoveHandler) to
capture the GORM result (e.g., res := c.GormDB().Save(...)) and check res.Error,
returning errors.NewError(res.Error) on failure so the function does not
silently succeed; apply this pattern for every c.GormDB().Save invocation in
those functions.
Comment on lines
+456
to
+469
| for _, player := range leaderboard.Players { | ||
| pID, _ := snowflake.Parse(player.ID) | ||
| idx := slices.IndexFunc(discordMembers, func(m discord.Member) bool { return m.User.ID == pID }) | ||
| if idx != -1 { | ||
| m, _ := c.MemberCreate(event, discordMembers[idx].User, *event.GuildID()) | ||
| m.XP = xppoint.XP(player.Xp) | ||
| c.GormDB().Save(m) | ||
| importedCount++ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| g.LevelMee6Imported = true | ||
| c.GormDB().Save(g) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "level_handlers.go" -type fRepository: sabafly/gobot
Length of output: 95
🏁 Script executed:
cat -n ./bot/commands/level/level_handlers.go | sed -n '440,480p'Repository: sabafly/gobot
Length of output: 1646
🏁 Script executed:
cat -n ./bot/commands/level/level_handlers.go | sed -n '400,430p'Repository: sabafly/gobot
Length of output: 1256
🏁 Script executed:
cat -n ./bot/commands/level/level_handlers.go | sed -n '385,405p'Repository: sabafly/gobot
Length of output: 956
🏁 Script executed:
rg "\.GormDB\(\)\.Save" -A 2 ./bot/commands/level/level_handlers.go | head -40Repository: sabafly/gobot
Length of output: 926
🏁 Script executed:
rg "GormDB\(\)\.Save.*Error" -B 1 -A 1 . | head -40Repository: sabafly/gobot
Length of output: 2390
🏁 Script executed:
rg "snowflake\.Parse" -B 1 -A 1 ./bot/commands/level/level_handlers.go | head -30Repository: sabafly/gobot
Length of output: 243
Mee6インポート時のMemberCreate/Saveエラーが無視されています。
部分的な更新が発生する可能性があるため、エラーハンドリングが必須です。以下の4つの操作でエラーをチェックしてください：
- Line 457:
snowflake.Parse(player.ID)のエラーをハンドルする - Line 460:
c.MemberCreate()のエラーをハンドルする - Line 462:
c.GormDB().Save(m)のエラーをハンドルする - Line 469:
c.GormDB().Save(g)のエラーをハンドルする
エラーが発生した場合は、関数を中断して呼び出し元に返却してください。
修正案
for _, player := range leaderboard.Players {
- pID, _ := snowflake.Parse(player.ID)
+ pID, err := snowflake.Parse(player.ID)
+ if err != nil {
+ return errors.NewError(err)
+ }
idx := slices.IndexFunc(discordMembers, func(m discord.Member) bool { return m.User.ID == pID })
if idx != -1 {
- m, _ := c.MemberCreate(event, discordMembers[idx].User, *event.GuildID())
+ m, err := c.MemberCreate(event, discordMembers[idx].User, *event.GuildID())
+ if err != nil {
+ return errors.NewError(err)
+ }
m.XP = xppoint.XP(player.Xp)
- c.GormDB().Save(m)
+ if err := c.GormDB().Save(m).Error; err != nil {
+ return errors.NewError(err)
+ }
importedCount++
}
}
g.LevelMee6Imported = true
-c.GormDB().Save(g)
+if err := c.GormDB().Save(g).Error; err != nil {
+ return errors.NewError(err)
+}🤖 Prompt for AI Agents
In `@bot/commands/level/level_handlers.go` around lines 456 - 469, Handle and
propagate errors for all operations in the Mee6 import loop: check the error
returned by snowflake.Parse(player.ID) and return it on failure; check and
return the error from c.MemberCreate(event, ...) before using m; check and
return errors from c.GormDB().Save(m) after saving the member; and finally check
and return the error from c.GormDB().Save(g) after setting g.LevelMee6Imported =
true. Make sure each failing operation aborts the function and returns the
encountered error so partial updates are not silently ignored.
Comment on lines
+366
to
386
| var w models.WordSuffix | ||
| err = c.GormDB().Where("guild_id = ? AND owner_id = ?", g.ID, u.ID).First(&w).Error | ||
| if err != nil { | ||
| if err == gorm.ErrRecordNotFound { | ||
| if err := event.CreateMessage( | ||
| discord.NewMessageBuilder(). | ||
| SetContent(translate.Message(event.Locale(), "components.message.suffix.remove.message.no_suffix", translate.WithTemplate(map[string]any{"User": discord.UserMention(u.ID)}))). | ||
| SetAllowedMentions(&discord.AllowedMentions{}). | ||
| SetFlags(discord.MessageFlagEphemeral). | ||
| BuildCreate(), | ||
| ); err != nil { | ||
| return errors.NewError(err) | ||
| } | ||
| return nil | ||
| } | ||
| return nil | ||
| return errors.NewError(err) | ||
| } | ||
|
|
||
| if err := c.GormDB().Delete(&w).Error; err != nil { | ||
| return errors.NewError(err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GORM ErrRecordNotFound errors.Is comparison Go
💡 Result:
In GORM v2, the idiomatic way to detect “not found” is to use Go’s standard wrapping-aware check:
err := db.First(&user, "id = ?", id).Error
if errors.Is(err, gorm.ErrRecordNotFound) {
// not found
}GORM v2 specifically documents using errors.Is(err, gorm.ErrRecordNotFound) (and notes RecordNotFound() was removed). [1]
Important gotcha: GORM v2 only returns ErrRecordNotFound for methods that are expected to return a single row like First, Last, Take. [1][2] For other patterns, you typically won’t get that error:
Find(...): usuallyError == niland you should checkRowsAffected == 0.Raw(...).Scan(...): can also returnnileven when no rows are returned, soerrors.Is(err, gorm.ErrRecordNotFound)won’t trigger; checkRowsAffected/your destination instead. [3]
Example for non-First/Take cases:
res := db.Where("email = ?", email).Find(&users)
if res.Error != nil { /* handle */ }
if res.RowsAffected == 0 { /* not found */ }Sources:
[1] GORM v2 release notes (“ErrRecordNotFound… please use errors.Is…”) (gorm.io)
[2] GORM docs: Error handling / ErrRecordNotFound section (gorm.io)
[3] go-gorm/gorm issue discussing Raw().Scan() returning nil instead of ErrRecordNotFound (github.com)
Citations:
- 1: https://gorm.io/docs/v2_release_note.html?utm_source=openai
- 2: https://gorm.io/id_ID/docs/error_handling.html?utm_source=openai
- 3:
nilerror returned instead of record not found withRaw()go-gorm/gorm#4416
🏁 Script executed:
# Search for existing GORM error handling patterns in the codebase
# Look for both direct equality and errors.Is usage
rg "gorm\.ErrRecordNotFound" -A 2 -B 2Repository: sabafly/gobot
Length of output: 4155
🏁 Script executed:
# Also check for any existing use of errors.Is with GORM errors
rg "errors\.Is.*gorm\." -A 2 -B 2Repository: sabafly/gobot
Length of output: 1806
🏁 Script executed:
# Check the specific file to understand the error handling context
head -n 400 bot/commands/message/message.go | tail -n 50Repository: sabafly/gobot
Length of output: 1965
ErrRecordNotFound の比較は errors.Is に統一してください。
GORM v2 の公式ドキュメントでは、エラーの比較に errors.Is(err, gorm.ErrRecordNotFound) を使用することが推奨されています。直接等値比較（==）では、エラーがラップされている場合に比較が失敗する可能性があります。同じファイル内の他の箇所で既に errors.Is パターンが使用されているため、統一してください。
✅ 修正案
- if err != nil {
- if err == gorm.ErrRecordNotFound {
+ if err != nil {
+ if errors.Is(err, gorm.ErrRecordNotFound) {
if err := event.CreateMessage(
discord.NewMessageBuilder().
SetContent(translate.Message(event.Locale(), "components.message.suffix.remove.message.no_suffix", translate.WithTemplate(map[string]any{"User": discord.UserMention(u.ID)}))).
SetAllowedMentions(&discord.AllowedMentions{}).
SetFlags(discord.MessageFlagEphemeral).
BuildCreate(),
); err != nil {
return errors.NewError(err)
}
return nil
}
return errors.NewError(err)
}📝 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
| var w models.WordSuffix | |
| err = c.GormDB().Where("guild_id = ? AND owner_id = ?", g.ID, u.ID).First(&w).Error | |
| if err != nil { | |
| if err == gorm.ErrRecordNotFound { | |
| if err := event.CreateMessage( | |
| discord.NewMessageBuilder(). | |
| SetContent(translate.Message(event.Locale(), "components.message.suffix.remove.message.no_suffix", translate.WithTemplate(map[string]any{"User": discord.UserMention(u.ID)}))). | |
| SetAllowedMentions(&discord.AllowedMentions{}). | |
| SetFlags(discord.MessageFlagEphemeral). | |
| BuildCreate(), | |
| ); err != nil { | |
| return errors.NewError(err) | |
| } | |
| return nil | |
| } | |
| return nil | |
| return errors.NewError(err) | |
| } | |
| if err := c.GormDB().Delete(&w).Error; err != nil { | |
| return errors.NewError(err) | |
| } | |
| var w models.WordSuffix | |
| err = c.GormDB().Where("guild_id = ? AND owner_id = ?", g.ID, u.ID).First(&w).Error | |
| if err != nil { | |
| if errors.Is(err, gorm.ErrRecordNotFound) { | |
| if err := event.CreateMessage( | |
| discord.NewMessageBuilder(). | |
| SetContent(translate.Message(event.Locale(), "components.message.suffix.remove.message.no_suffix", translate.WithTemplate(map[string]any{"User": discord.UserMention(u.ID)}))). | |
| SetAllowedMentions(&discord.AllowedMentions{}). | |
| SetFlags(discord.MessageFlagEphemeral). | |
| BuildCreate(), | |
| ); err != nil { | |
| return errors.NewError(err) | |
| } | |
| return nil | |
| } | |
| return errors.NewError(err) | |
| } | |
| if err := c.GormDB().Delete(&w).Error; err != nil { | |
| return errors.NewError(err) | |
| } |
🤖 Prompt for AI Agents
In `@bot/commands/message/message.go` around lines 366 - 386, The err comparison
for a missing record should use errors.Is instead of ==; in the block that
queries models.WordSuffix (using c.GormDB().Where(...).First(&w).Error) replace
the direct equality check "if err == gorm.ErrRecordNotFound" with "if
errors.Is(err, gorm.ErrRecordNotFound)" and ensure the standard "errors" package
is imported; keep the surrounding logic (event.CreateMessage, discord message
building, and Delete(&w) handling) unchanged.
Comment on lines
+616
to
618
| var reminds []models.MessageRemind | ||
| c.GormDB().Where("guild_id = ? AND name LIKE ?", *event.GuildID(), "%"+event.Data.String("remind")+"%").Limit(25).Find(&reminds) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd bot/commands/message && cat -n message.go | sed -n '610,625p'Repository: sabafly/gobot
Length of output: 962
🌐 Web query:
GORM Find() error handling best practices
💡 Result:
GORM Find() error-handling best practices (v2)
-
Always check
result.Errorafter the finisher callFindis a finisher method; errors show up on the returned*gorm.DB(traditional API). (gorm.io)
var users []User tx := db.Where("active = ?", true).Find(&users) if tx.Error != nil { return tx.Error }
-
Don’t expect
Findto returnErrRecordNotFound- In GORM v2,
ErrRecordNotFoundis returned byFirst/Take/Last, not byFind.Findreturning “no rows” is typically not an error. (gorm.io)
- In GORM v2,
-
Use
RowsAffectedto detect “not found” withFind- If you need to treat “no rows” as a special case, check
RowsAffected == 0. (gorm.io)
var users []User tx := db.Where("email LIKE ?", "%@acme.com").Find(&users) if tx.Error != nil { return tx.Error } if tx.RowsAffected == 0 { /* handle empty result */ }
- If you need to treat “no rows” as a special case, check
-
For “get one row”, prefer
First/Take/Lastand handleErrRecordNotFoundexplicitly- This is the idiomatic way to distinguish “missing row” from real DB errors. (gorm.io)
var u User err := db.Where("id = ?", id).First(&u).Error if errors.Is(err, gorm.ErrRecordNotFound) { /* 404 / not found */ } if err != nil { return err }
-
If you use
Findto avoidErrRecordNotFound, do it safely- Docs explicitly note you can avoid
ErrRecordNotFoundwithLimit(1).Find(&dest). Then treat “not found” viaRowsAffected. (gorm.io)
var u User tx := db.Where("id = ?", id).Limit(1).Find(&u) if tx.Error != nil { return tx.Error } if tx.RowsAffected == 0 { /* not found */ }
- Docs explicitly note you can avoid
-
Avoid
db.Find(&struct)without aLimit- GORM warns this can scan the whole table and return a non-deterministic “first” row; it’s also wasteful. (gorm.io)
-
(Optional) Normalize constraint errors with
TranslateError- If you want portable checks like
ErrDuplicatedKey, enableTranslateErrorand thenerrors.Is(...). (gorm.io)
- If you want portable checks like
Citations:
- 1: https://gorm.io/id_ID/docs/error_handling.html?utm_source=openai
- 2: https://gorm.io/id_ID/docs/error_handling.html?utm_source=openai
- 3: https://gorm.io/id_ID/docs/query.html?utm_source=openai
- 4: https://gorm.io/id_ID/docs/error_handling.html?utm_source=openai
- 5: https://gorm.io/id_ID/docs/query.html?utm_source=openai
- 6: https://gorm.io/id_ID/docs/query.html?utm_source=openai
- 7: https://gorm.io/id_ID/docs/error_handling.html?utm_source=openai
DB エラーを正しく処理してください。
このコードはGORMのFind()呼び出しの結果を無視しており、DB関連のエラーが発生してもチェックされていません。GORMの公式ドキュメントでは、Find()などのfinisherメソッドは常に.Errorフィールドをチェックすることを推奨しています。取得に失敗した場合は適切にエラーを返すべきです。
修正案
- c.GormDB().Where("guild_id = ? AND name LIKE ?", *event.GuildID(), "%"+event.Data.String("remind")+"%").Limit(25).Find(&reminds)
+ if err := c.GormDB().Where("guild_id = ? AND name LIKE ?", *event.GuildID(), "%"+event.Data.String("remind")+"%").Limit(25).Find(&reminds).Error; err != nil {
+ return errors.NewError(err)
+ }📝 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
| var reminds []models.MessageRemind | |
| c.GormDB().Where("guild_id = ? AND name LIKE ?", *event.GuildID(), "%"+event.Data.String("remind")+"%").Limit(25).Find(&reminds) | |
| var reminds []models.MessageRemind | |
| if err := c.GormDB().Where("guild_id = ? AND name LIKE ?", *event.GuildID(), "%"+event.Data.String("remind")+"%").Limit(25).Find(&reminds).Error; err != nil { | |
| return errors.NewError(err) | |
| } |
🤖 Prompt for AI Agents
In `@bot/commands/message/message.go` around lines 616 - 618, The DB call that
loads reminds ignores GORM errors; capture the result of
c.GormDB().Where(...).Limit(25).Find(&reminds) (related symbols: c.GormDB(),
models.MessageRemind, reminds, event.GuildID(), event.Data.String("remind")) and
check result.Error (or use .Error on the chained call). If an error is non-nil,
return or handle it appropriately (e.g., return the error from the enclosing
function or send an error response) instead of proceeding as if the query
succeeded.
Comment on lines
+790
to
803
| // Guild | ||
| if err := c.GormDB().Where("owner_id = ? AND guild_id = ?", u.ID, e.GuildID).First(&w).Error; err == nil { | ||
| // Found | ||
| } else { | ||
| // Global | ||
| if !u.QueryWordSuffix().Where(wordsuffix.GuildIDIsNil()).ExistX(e) { | ||
| if err := c.GormDB().Where("owner_id = ? AND guild_id IS NULL", u.ID).First(&w).Error; err != nil { | ||
| if err != gorm.ErrRecordNotFound { | ||
| slog.Error("語尾取得エラー", "err", err) | ||
| } | ||
| // Not found | ||
| slog.Debug("語尾が存在しません") | ||
| goto messagePin | ||
| } | ||
| w = u.QueryWordSuffix().Where(wordsuffix.GuildIDIsNil()).FirstX(e) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "message.go" -path "*/bot/commands/message/*" | head -5Repository: sabafly/gobot
Length of output: 90
🏁 Script executed:
cat -n bot/commands/message/message.go | sed -n '785,810p'Repository: sabafly/gobot
Length of output: 1023
🌐 Web query:
GORM First ErrRecordNotFound errors.Is best practice error handling
💡 Result:
In GORM v2, First/Take/Last are “must return one row” queries: if nothing matches, they return gorm.ErrRecordNotFound, and the recommended way to test it is errors.Is. (gorm.io)
Best-practice pattern (idiomatic Go)
import (
"errors"
"gorm.io/gorm"
)
func GetUser(db *gorm.DB, id uint) (*User, error) {
var u User
err := db.First(&u, id).Error
if err == nil {
return &u, nil
}
if errors.Is(err, gorm.ErrRecordNotFound) {
// choose your API contract:
return nil, nil // “not found” is not exceptional for this function
// or: return nil, ErrUserNotFound
}
return nil, err // real DB/SQL/driver error
}GORM’s docs also encourage checking .Error after “finisher” methods (like First). (gorm.io)
If “not found” should not be an error
Use Find with a limit; Find does not raise ErrRecordNotFound (and you can check RowsAffected). (gorm.io)
var u User
tx := db.Limit(1).Find(&u, "id = ?", id)
if tx.Error != nil { return nil, tx.Error }
if tx.RowsAffected == 0 { return nil, nil }
return &u, nilCitations:
- 1: https://gorm.io/docs/v2_release_note.html?utm_source=openai
- 2: https://gorm.io/es_ES/docs/error_handling.html?utm_source=openai
- 3: https://gorm.io/id_ID/docs/query.html?utm_source=openai
ギルド専用取得のDBエラーが黙殺されています。
ライン791のクエリでエラーが発生した場合、ErrRecordNotFound 以外のエラーは全てグローバル検索にフォールバックされ、ログに記録されません。接続エラーや権限エラーなどの実際のデータベース問題が隠れる可能性があります。
GORM v2では errors.Is() を使用して ErrRecordNotFound を判定することが推奨されています。ライン791でも他の箇所と一貫性を保つため、即座にエラーを返すべきです。
修正案
// Guild
- if err := c.GormDB().Where("owner_id = ? AND guild_id = ?", u.ID, e.GuildID).First(&w).Error; err == nil {
+ if err := c.GormDB().Where("owner_id = ? AND guild_id = ?", u.ID, e.GuildID).First(&w).Error; err == nil {
// Found
- } else {
+ } else if !errors.Is(err, gorm.ErrRecordNotFound) {
+ slog.Error("語尾取得エラー", "err", err)
+ return errors.NewError(err)
+ } else {
// Global
if err := c.GormDB().Where("owner_id = ? AND guild_id IS NULL", u.ID).First(&w).Error; err != nil {
if err != gorm.ErrRecordNotFound {
slog.Error("語尾取得エラー", "err", err)
}
// Not found
slog.Debug("語尾が存在しません")
goto messagePin
}
}🤖 Prompt for AI Agents
In `@bot/commands/message/message.go` around lines 790 - 803, The guild-specific
DB query (c.GormDB().Where(...).First(&w)) currently swallows non-NotFound
errors and falls back to the global query; change it to check errors using
errors.Is(err, gorm.ErrRecordNotFound) so only NotFound falls through to the
global lookup, and for any other err log it (slog.Error with the err) and stop
processing (e.g., goto messagePin or return as appropriate). Update the branch
around the c.GormDB() call that uses u.ID and e.GuildID to handle non-NotFound
errors immediately instead of silently continuing to the global query, keeping
the subsequent global query logic unchanged.
Comment on lines
+924
to
+931
| var m models.MessagePin | ||
| if err := c.GormDB().Where("guild_id = ? AND channel_id = ?", g.ID, e.ChannelID).First(&m).Error; err != nil { | ||
| return nil | ||
| } | ||
| m := g.QueryMessagePins().Where(messagepin.ChannelID(e.ChannelID)).FirstX(e) | ||
|
|
||
| if m.BeforeID != nil && *m.BeforeID == e.MessageID { | ||
| slog.Info("ピン留め削除", "cid", e.ChannelID, "mid", e.MessageID) | ||
| c.DB().MessagePin.DeleteOneID(m.ID).ExecX(e) | ||
| c.GormDB().Delete(&m) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its existence
find . -name "message.go" -path "*/commands/message/*" | head -20Repository: sabafly/gobot
Length of output: 90
🏁 Script executed:
# Read the specific lines mentioned in the review
sed -n '900,950p' bot/commands/message/message.go | cat -nRepository: sabafly/gobot
Length of output: 1745
🏁 Script executed:
# Search for other Delete operations and how they handle errors
rg "\.Delete\(&" bot/commands/message/message.go -A 2 -B 2Repository: sabafly/gobot
Length of output: 1098
🏁 Script executed:
# Check error handling patterns in the codebase for GORM operations
rg "\.Delete\(&.*\)\.Error" . -A 2 -B 2 | head -100Repository: sabafly/gobot
Length of output: 3584
🏁 Script executed:
# Check how errors.NewError is used in the codebase
rg "errors\.NewError" bot/commands/message/message.go -B 2 -A 2 | head -50Repository: sabafly/gobot
Length of output: 1801
Delete操作のエラーハンドリングを追加してください。
現在のコードはメッセージピン削除に失敗した場合、エラーが無視されています。同一ファイル内の他のDelete操作では正しくエラーハンドリングが実装されており、コードベース全体でも標準パターンとなっているため、ここでも同じ対応が必要です。
修正案
- c.GormDB().Delete(&m)
+ if err := c.GormDB().Delete(&m).Error; err != nil {
+ return errors.NewError(err)
+ }🤖 Prompt for AI Agents
In `@bot/commands/message/message.go` around lines 924 - 931, The Delete call for
models.MessagePin ignores errors; change the c.GormDB().Delete(&m) invocation to
check its returned .Error, and if non-nil log it (e.g. with slog.Error including
context "ピン留め削除失敗", channel id and message id) and return the error to the
caller (following the same pattern used by other Delete operations in this
file). Use the existing models.MessagePin variable (m) and the same c.GormDB()
call so the change is minimal and consistent with surrounding code.
Comment on lines
945
to
947
| if w.Expired != nil && time.Now().Compare(*w.Expired) == 1 { | ||
| c.DB().WordSuffix.DeleteOneID(w.ID).ExecX(e) | ||
| c.GormDB().Delete(w) | ||
| return nil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n bot/commands/message/message.go | sed -n '940,950p'Repository: sabafly/gobot
Length of output: 349
🏁 Script executed:
# Search for other Delete operations in the codebase to understand error handling patterns
rg "\.Delete\(" --type go -B 2 -A 2 | head -100Repository: sabafly/gobot
Length of output: 5046
🏁 Script executed:
# Check the broader context of the file to understand the codebase structure
head -50 bot/commands/message/message.goRepository: sabafly/gobot
Length of output: 1489
🌐 Web query:
GORM Delete メソッドのエラー処理のベストプラクティス
💡 Result:
GORM（主に v2）の Delete におけるエラー処理は、「Error と RowsAffected を分けて扱う」のが基本です。
1) Delete は「対象なし」でも ErrRecordNotFound を返さない
ErrRecordNotFound が返るのは原則 First/Last/Take のような「1件返る想定の取得」に限られ、Delete では返りません。[3]
そのため「存在しないIDを削除しようとした」を検知したい場合は RowsAffected == 0 を見ます（=削除件数0）。[5][6]
res := db.Where("id = ?", id).Delete(&User{})
if res.Error != nil {
return res.Error
}
if res.RowsAffected == 0 {
// 対象なし（404相当など）
}
return nil2) まずは .Error を必ずチェック（Traditional API）
Traditional API では、チェーン実行後に *gorm.DB の Error を確認するのが基本です。[1]
3) “全件削除事故” をエラーとして扱う（ErrMissingWhereClause）
条件なしの一括削除はデフォルトでブロックされ、ErrMissingWhereClause が返ります（＝意図しない全件削除を防ぐ）。このエラーはそのまま上位に返す（またはログ＋即失敗）扱いが推奨です。[5]
4) 複数操作は Transaction でまとめ、エラーを返してロールバック
削除前後に別更新がある等は db.Transaction(func(tx *gorm.DB) error { ... }) で囲い、途中でエラーが出たら return err してロールバックさせます。[2]
5) Soft Delete / 物理削除の意図を明確化
gorm.DeletedAt があるモデルは Delete が 物理削除ではなく Soft Delete（DeletedAt を埋める）になります。物理削除したい場合は Unscoped().Delete(...) を明示します。[5]
6) 業務ルール違反は Delete Hook でエラー化できる
BeforeDelete などのフックで「この条件なら削除不可」をエラーとして返せます。[5]
要点: Delete の結果は res.Error（DB/SQLエラー） と res.RowsAffected（対象なし判定） をセットで扱うのがベストプラクティスです。[1][5][6]
GORM Delete のエラーを処理してください。
GORM の Delete メソッドはデータベース操作のエラーを .Error フィールドに設定します。これを確認せずに無視すると、削除失敗時にエラーが伝播されず、データの整合性が損なわれる可能性があります。
修正案
- c.GormDB().Delete(w)
+ if err := c.GormDB().Delete(w).Error; err != nil {
+ return err
+ }📝 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 w.Expired != nil && time.Now().Compare(*w.Expired) == 1 { | |
| c.DB().WordSuffix.DeleteOneID(w.ID).ExecX(e) | |
| c.GormDB().Delete(w) | |
| return nil | |
| if w.Expired != nil && time.Now().Compare(*w.Expired) == 1 { | |
| if err := c.GormDB().Delete(w).Error; err != nil { | |
| return err | |
| } | |
| return nil |
🤖 Prompt for AI Agents
In `@bot/commands/message/message.go` around lines 945 - 947, w.Expired チェック後の
c.GormDB().Delete(w) 呼び出しで GORM の返却結果を無視しています；Delete の戻り値を受け取り result.Error
を確認し、エラーがあれば適切に返すかログ出力して処理を中断するよう修正してください（参考箇所: w.Expired と c.GormDB().Delete(w)
を使っているブロック）。必要なら result.RowsAffected も確認して期待した削除が行われたかを検証してください。
Comment on lines
+348
to
+350
| var panel models.RolePanel | ||
| c.GormDB().Where("id = ?", edit.ParentID).First(&panel) | ||
| initialize(&edit, &panel) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the role.go file
find . -name "role.go" -path "*/commands/role/*" -type fRepository: sabafly/gobot
Length of output: 84
🏁 Script executed:
# Check the specific lines mentioned (348-350 and 639-640)
head -n 350 ./bot/commands/role/role.go | tail -n 20Repository: sabafly/gobot
Length of output: 862
🏁 Script executed:
# Check lines 639-640 area
sed -n '630,650p' ./bot/commands/role/role.goRepository: sabafly/gobot
Length of output: 1059
🏁 Script executed:
# Look for error handling patterns in the codebase with GORM
rg "\.First\(&" bot/commands/role/role.go -A 2 -B 2Repository: sabafly/gobot
Length of output: 2844
🏁 Script executed:
# Check how errors are typically handled in this codebase
rg "c\.GormDB\(\)" bot/commands/role/role.go -A 3 | head -n 50Repository: sabafly/gobot
Length of output: 2171
panelのクエリ結果をエラーチェックなしで使用すると、レコードが見つからない場合に不整合が発生します。
edit.ParentIDとplace.RolePanelIDで指定されたpanelが存在しない場合、ゼロ値の構造体で処理が続行され、誤った動作やデータ更新につながる可能性があります。First()の結果を常にチェックして、エラー時は早期にリターンしてください。
修正案
- var panel models.RolePanel
- c.GormDB().Where("id = ?", edit.ParentID).First(&panel)
+ var panel models.RolePanel
+ if err := c.GormDB().Where("id = ?", edit.ParentID).First(&panel).Error; err != nil {
+ return errors.NewError(errors.ErrorMessage("errors.not_exist", event))
+ }- var panel models.RolePanel
- c.GormDB().Where("id = ?", place.RolePanelID).First(&panel)
+ var panel models.RolePanel
+ if err := c.GormDB().Where("id = ?", place.RolePanelID).First(&panel).Error; err != nil {
+ return errors.NewError(errors.ErrorMessage("errors.not_exist", event))
+ }📝 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
| var panel models.RolePanel | |
| c.GormDB().Where("id = ?", edit.ParentID).First(&panel) | |
| initialize(&edit, &panel) | |
| var panel models.RolePanel | |
| if err := c.GormDB().Where("id = ?", edit.ParentID).First(&panel).Error; err != nil { | |
| return errors.NewError(errors.ErrorMessage("errors.not_exist", event)) | |
| } | |
| initialize(&edit, &panel) |
🤖 Prompt for AI Agents
In `@bot/commands/role/role.go` around lines 348 - 350, The code calls
c.GormDB().Where("id = ?", edit.ParentID).First(&panel) and then
initialize(&edit, &panel) without checking the DB result; ensure you check the
returned error from First() (and handle gorm.ErrRecordNotFound) before using
panel: if the query fails or no record is found for edit.ParentID (or later for
place.RolePanelID usage), return early with an appropriate error/response
instead of proceeding with a zero-value panel; update the code paths around the
panel variable and initialize(&edit, &panel) to perform this error check and
short-circuit on failure.
Comment on lines
+391
to
+392
| g, _ := c.GuildCreateID(event, *event.GuildID()) | ||
| var edit models.RolePanelEdit |
There was a problem hiding this comment.
GuildCreateID のエラー無視で nil 参照の可能性があります。
取得失敗時に g が nil のまま使われるため panic し得ます。エラーを返して処理を中断してください。
🐛 修正案
- g, _ := c.GuildCreateID(event, *event.GuildID())
+ g, err := c.GuildCreateID(event, *event.GuildID())
+ if err != nil {
+ return errors.NewError(err)
+ }Also applies to: 633-635, 691-693, 833-834
🤖 Prompt for AI Agents
In `@bot/commands/role/role.go` around lines 391 - 392, GuildCreateID's returned
error is currently ignored causing g to be potentially nil and used later (e.g.,
where g, _ := c.GuildCreateID(event, *event.GuildID()) is called); change each
call to capture and check the error (e.g., g, err := c.GuildCreateID(...); if
err != nil { return err / abort processing } ) before using g so you don't
dereference nil; apply the same fix to the other occurrences noted (the calls
around the RolePanelEdit usage and the similar sites at the other occurrences).
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.
データベースモデルをGORMに移行し、エンティティの操作を改善した。WordSuffixにUUIDのデフォルト値を追加し、ギルド脱退時のデータベース削除エラーをログに記録する機能を追加した。
Summary by CodeRabbit
リリースノート
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.