Conversation
📝 Walkthroughウォークスルーent固有のエラーハンドリング（ent.IsNotFound、ent.IsConstraintError）を削除し、エラーハンドリングを簡素化しました。GetFile、GetFileMeta、DeleteFileおよび認証処理でエラーを元々の形式で直接返すようにし、未使用のentインポートを削除してログにファイルIDを追加しました。 変更内容
推定コードレビュー作業量🎯 2 (Simple) | ⏱️ ~12分 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/router/file.go`:
- Around line 172-178: GetFile のエラーパスは GetFileMeta と同様に DB エラーなど全ての失敗を単に
logger.Info で「file not found in repository」と記録しているため、ログレベルと文言を修正してください —
h.Repository.GetFile 呼び出し後のエラーハンドリングで logger.Info を logger.Error
に変更し、メッセージを「failed to get file from repository」や「failed to get file
metadata」など失敗を示す文言にしつつ zap.String("ID", fileID.String()) と zap.Error(err)
を含める（GetFileMeta の該当箇所と同様の形式に合わせる）ことで、実際の DB エラーを適切なレベルで記録してください.
- Around line 118-124: GetFile error handling currently logs every error as info
"file not found in repository" and returns the raw error; change it to
distinguish ent.IsNotFound(err) from other errors: when ent.IsNotFound(err) keep
the Info-level "file not found" log (including fileID) and return the
appropriate not-found error, but for all other errors log with logger.Error
(include fileID and the error) and return a wrapped unexpected error (e.g.
NewUnexpectedError) so DB/infra failures are surfaced at error level; update
handling around h.Repository.GetFile, logger.Info/logger.Error, ent.IsNotFound
and the return path instead of returning the raw err.
Comment on lines
118
to
124
| file, err := h.Repository.GetFile(ctx, fileID) | ||
| if err != nil { | ||
| if ent.IsNotFound(err) { | ||
| logger.Info("could not find file in repository", zap.String("ID", fileID.String())) | ||
| return echo.ErrNotFound.Wrap(err) | ||
| } | ||
| logger.Error("failed to get file from repository", zap.Error(err)) | ||
| return service.NewUnexpectedError(err) | ||
| logger.Info( | ||
| "file not found in repository", | ||
| zap.String("ID", fileID.String()), zap.Error(err)) | ||
| return err | ||
| } |
There was a problem hiding this comment.
ログメッセージとレベルがすべてのエラーに誤適用されている
GetFile が返すエラーは「ファイルが存在しない」ケースに限りません。DB 接続失敗やクエリエラーが発生した場合でも logger.Info("file not found in repository", ...) が記録されてしまい、実際のエラー内容を誤解させます。以前は ent.IsNotFound の場合のみこのメッセージが使われ、それ以外は別経路（logger.Error + NewUnexpectedError）で処理されていました。
DB 障害を Info レベルで "file not found" として記録すると、インフラ問題の検知が遅れます。
🔧 修正案
file, err := h.Repository.GetFile(ctx, fileID)
if err != nil {
- logger.Info(
- "file not found in repository",
- zap.String("ID", fileID.String()), zap.Error(err))
- return err
+ logger.Error(
+ "failed to get file from repository",
+ zap.String("ID", fileID.String()), zap.Error(err))
+ return err
}また return err で生の ent エラーが返ることによる HTTP ステータスのマッピングについては、middleware.go 側のコメントで生成した確認スクリプトで合わせて検証してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/router/file.go` around lines 118 - 124, GetFile error handling
currently logs every error as info "file not found in repository" and returns
the raw error; change it to distinguish ent.IsNotFound(err) from other errors:
when ent.IsNotFound(err) keep the Info-level "file not found" log (including
fileID) and return the appropriate not-found error, but for all other errors log
with logger.Error (include fileID and the error) and return a wrapped unexpected
error (e.g. NewUnexpectedError) so DB/infra failures are surfaced at error
level; update handling around h.Repository.GetFile, logger.Info/logger.Error,
ent.IsNotFound and the return path instead of returning the raw err.
Comment on lines
172
to
178
| file, err := h.Repository.GetFile(ctx, fileID) | ||
| if err != nil { | ||
| if ent.IsNotFound(err) { | ||
| logger.Info("could not find file in repository", zap.String("ID", fileID.String())) | ||
| return echo.ErrNotFound.Wrap(err) | ||
| } | ||
| logger.Error("failed to get file from repository", zap.Error(err)) | ||
| return service.NewUnexpectedError(err) | ||
| logger.Info( | ||
| "file not found in repository", | ||
| zap.String("ID", fileID.String()), zap.Error(err)) | ||
| return err | ||
| } |
There was a problem hiding this comment.
GetFile エラーパスと同じ：ログメッセージ・レベルが誤り
GetFileMeta でも GetFile のエラーパスと同様に、DB エラーを含むすべての失敗に対して logger.Info("file not found in repository", ...) が記録されます。
🔧 修正案
file, err := h.Repository.GetFile(ctx, fileID)
if err != nil {
- logger.Info(
- "file not found in repository",
- zap.String("ID", fileID.String()), zap.Error(err))
- return err
+ logger.Error(
+ "failed to get file from repository",
+ zap.String("ID", fileID.String()), zap.Error(err))
+ 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
| file, err := h.Repository.GetFile(ctx, fileID) | |
| if err != nil { | |
| if ent.IsNotFound(err) { | |
| logger.Info("could not find file in repository", zap.String("ID", fileID.String())) | |
| return echo.ErrNotFound.Wrap(err) | |
| } | |
| logger.Error("failed to get file from repository", zap.Error(err)) | |
| return service.NewUnexpectedError(err) | |
| logger.Info( | |
| "file not found in repository", | |
| zap.String("ID", fileID.String()), zap.Error(err)) | |
| return err | |
| } | |
| file, err := h.Repository.GetFile(ctx, fileID) | |
| if err != nil { | |
| logger.Error( | |
| "failed to get file from repository", | |
| zap.String("ID", fileID.String()), zap.Error(err)) | |
| return err | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/router/file.go` around lines 172 - 178, GetFile のエラーパスは GetFileMeta
と同様に DB エラーなど全ての失敗を単に logger.Info で「file not found in
repository」と記録しているため、ログレベルと文言を修正してください — h.Repository.GetFile 呼び出し後のエラーハンドリングで
logger.Info を logger.Error に変更し、メッセージを「failed to get file from
repository」や「failed to get file metadata」など失敗を示す文言にしつつ zap.String("ID",
fileID.String()) と zap.Error(err) を含める（GetFileMeta の該当箇所と同様の形式に合わせる）ことで、実際の DB
エラーを適切なレベルで記録してください.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #1010 +/- ##
========================================
+ Coverage 7.24% 7.29% +0.05%
========================================
Files 118 118
Lines 16931 16924 -7
========================================
+ Hits 1226 1235 +9
+ Misses 15620 15607 -13
+ Partials 85 82 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
|
うっせーーーーーー |
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.
#998 の漏れ
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor