Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/issue504 #556

Merged
merged 6 commits into from
Aug 21, 2024
Merged

Fix/issue504 #556

merged 6 commits into from
Aug 21, 2024

Conversation

iChemy
Copy link
Contributor

@iChemy iChemy commented Jul 15, 2024

close #504

@iChemy iChemy requested review from pasmophobia and Nzt3-gh July 15, 2024 12:01
@iChemy
Copy link
Contributor Author

iChemy commented Jul 15, 2024

本番環境でも確認

{"time":"2024-07-15T11:51:37.312581325Z","level":"-","prefix":"echo","file":"recover.go","line":"119","message":"[PANIC RECOVER] runtime error: invalid memory address or nil pointer dereference goroutine 143756 [running]:\ngithub.com/traPtitech/knoQ/router.(*Handlers).SetupRoute.Recover.RecoverWithConfig.func4.1.1()\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/middleware/recover.go:99 +0x165\npanic({0xa24d40?, 0xf8d780?})\n\t/usr/local/go/src/runtime/panic.go:770 +0x132\ngithub.com/traPtitech/knoQ/infra/db.(*dialector).SavePoint(0xf91d80?, 0xa9fae0?, {0xc0014e91c0?, 0xa?})\n\t<autogenerated>:1 +0x24\ngorm.io/gorm.(*DB).SavePoint(0xc006ef6120, {0xc0014e91c0, 0xa})\n\t/go/pkg/mod/gorm.io/[email protected]/finisher_api.go:724 +0xda\ngorm.io/gorm.(*DB).Transaction(0xc006ef6120, 0xc0013ed2f0, {0x0?, 0x0?, 0x0?})\n\t/go/pkg/mod/gorm.io/[email protected]/finisher_api.go:626 +0xea\ngithub.com/traPtitech/knoQ/infra/db.saveUser(0xc006ef6120, 0xc0013ab080)\n\t/app/infra/db/user.go:75 +0xa5\ngithub.com/traPtitech/knoQ/infra/db.(*GormRepository).SyncUsers.func1(0xc006ef6120)\n\t/app/infra/db/user.go:49 +0xa6\ngorm.io/gorm.(*DB).Transaction(0xc000250ab0, 0xc00104c020, {0x0?, 0x0?, 0x0?})\n\t/go/pkg/mod/gorm.io/[email protected]/finisher_api.go:651 +0x25e\ngithub.com/traPtitech/knoQ/infra/db.(*GormRepository).SyncUsers(0xc0002bb9e0, {0xc000ca3008, 0x71f, 0x8ff})\n\t/app/infra/db/user.go:35 +0x8b\ngithub.com/traPtitech/knoQ/repository.(*Repository).SyncUsers(0xc0002bb9e0, 0xc00024f860?)\n\t/app/repository/user.go:44 +0x292\ngithub.com/traPtitech/knoQ/router.(*Handlers).HandleSyncUser(0xc000248ea0, {0xbc6c40, 0xc00024f860})\n\t/app/router/users.go:70 +0x43\ngithub.com/traPtitech/knoQ/router.(*Handlers).PrivilegeUserMiddleware-fm.(*Handlers).PrivilegeUserMiddleware.func1({0xbc6c40, 0xc00024f860})\n\t/app/router/middleware.go:117 +0x66\ngithub.com/traPtitech/knoQ/router.(*Handlers).TraQUserMiddleware-fm.(*Handlers).TraQUserMiddleware.func1({0xbc6c40, 0xc00024f860})\n\t/app/router/middleware.go:101 +0x142\ngithub.com/labstack/echo/v4.(*Echo).add.func1({0xbc6c40, 0xc00024f860})\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/echo.go:587 +0x4b\ngithub.com/labstack/echo/v4/middleware.StaticWithConfig.func1.1({0xbc6c40, 0xc00024f860})\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/middleware/static.go:166 +0x95b\ngithub.com/labstack/echo/v4/middleware.CORSWithConfig.func1.1({0xbc6c40, 0xc00024f860})\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/middleware/cors.go:253 +0x562\ngithub.com/traPtitech/knoQ/router.(*Handlers).SetupRoute.ServerVersionMiddleware.func3.1({0xbc6c40, 0xc00024f860})\n\t/app/router/middleware.go:78 +0x79\ngithub.com/traPtitech/knoQ/router.(*Handlers).SetupRoute.Middleware.MiddlewareWithConfig.func6.1({0xbc6c40, 0xc00024f860})\n\t/go/pkg/mod/github.com/labstack/[email protected]/session/session.go:73 +0x104\ngithub.com/traPtitech/knoQ/router.(*Handlers).SetupRoute.AccessLoggingMiddleware.func2.1({0xbc6c40, 0xc00024f860})\n\t/app/router/middleware.go:26 +0x7c\ngithub.com/traPtitech/knoQ/router.(*Handlers).SetupRoute.Secure.SecureWithConfig.func5.1({0xbc6c40, 0xc00024f860})\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/middleware/secure.go:141 +0x364\ngithub.com/traPtitech/knoQ/router.(*Handlers).SetupRoute.Recover.RecoverWithConfig.func4.1({0xbc6c40, 0xc00024f860})\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/middleware/recover.go:130 +0x114\ngithub.com/labstack/echo/v4.(*Echo).ServeHTTP(0xc0001806c8, {0xbbdde0, 0xc000118000}, 0xc0003979e0)\n\t/go/pkg/mod/github.com/labstack/echo/[email protected]/echo.go:674 +0x327\nnet/http.serverHandler.ServeHTTP({0xc006ef65a0?}, {0xbbdde0?, 0xc000118000?}, 0x6?)\n\t/usr/local/go/src/net/http/server.go:3142 +0x8e\nnet/http.(*conn).serve(0xc0000f5200, {0xbbecf8, 0xc000074480})\n\t/usr/local/go/src/net/http/server.go:2044 +0x5e8\ncreated by net/http.(*Server).Serve in goroutine 28\n\t/usr/local/go/src/net/http/server.go:3290 +0x4b4\n\ngoroutine 1 [chan receive, 7878 minutes]:\nmain.main()\n\t/app/main.go:125 +0x8cb\n\ngoroutine 21 [select, 9 minutes]:\ngithub.com/patrickmn/go-cache.(*janitor).Run(0xc0001d5c50, 0xc000215180)\n\t/go/pkg/mod/github.com/patrickmn/[email protected]+incompatible/cache.go:1079 +0x7d\ncreated by github.com/patrickmn/go-cache.runJanitor in goroutine 1\n\t/go/pkg/mod/github.com/patrickmn/[email protected]+incompatible/cache.go:1099 +0x\n"}

@iChemy
Copy link
Contributor Author

iChemy commented Jul 15, 2024

mysql.New() で返される値は gorm.Dialector, gorm.Translator, gorm.SavePointerDialectorInterface を全て実装している

現在の実装だと dialectorgorm.SavePointerDialectorInterface を実装しているはずということになっている (そのためキャストはできる) が
dgorm.SavePointerDialectorInterface として働く実体を持っていないため null ポインタへのアクセスが発生する.

dialector に実態を持たせるように実装する方法もある (gorm.Translator のようにオーバーライドさせることである)
ただし,それならそもそも mysql.New() で返されるものがこれら全てを実装しているのだからそれを使えば良いはず.

また,dialector 型を定義するメリットはそれが gorm.Dialector, gorm.SavePointerDialectorInterface を実装していることを確定させることであるが gorm.Dialector, gorm.SavePointerDialectorInterface の実装が確定していることで成り立っている部分はないためそもそもdialector 型を利用する意味はない

@iChemy
Copy link
Contributor Author

iChemy commented Jul 15, 2024

mysql.New が返すのは mysql.Dialector 構造体.これを明示的に gorm.Dialector インターフェースとして扱ってしまうと,多分gorm.Dialector インターフェースが持っているメソッド以外は使えないものとして扱われる(はず,)

@Nzt3-gh
Copy link
Contributor

Nzt3-gh commented Aug 11, 2024

(私がprivilegeではなかったのでIsPrivilegeをいじってのローカルでのテストですが) /api/users/sync で500エラーが出ていたのが201になったことがこちらでも確認できました。

@Nzt3-gh
Copy link
Contributor

Nzt3-gh commented Aug 11, 2024

mysql.New()が返すのはgorm.Dialectorではないですか? GORM MySQL Driverのやつ

@iChemy
Copy link
Contributor Author

iChemy commented Aug 12, 2024

関数の宣言としてはgorm.Dialector(インターフェース) を返していますね.
関数の実装を見ると (gorm.Dialectorインターフェースを実装している)mysql.Dialector 構造体を返していますね.

で,この mysql.Dialector 構造体は gorm.Translatorインターフェースとgorm.SavePointerDialectorInterfaceインターフェース実装しているという感じです

Copy link
Contributor

@Nzt3-gh Nzt3-gh left a comment

Choose a reason for hiding this comment

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

動作確認できました。
gorm.SavePointerDialectorInterfaceインターフェースの話についても問題はないと思います。

@iChemy iChemy merged commit e7da191 into main Aug 21, 2024
1 of 2 checks passed
@iChemy iChemy deleted the fix/issue504 branch August 21, 2024 05:51
@iChemy iChemy restored the fix/issue504 branch August 21, 2024 05:54
@iChemy iChemy mentioned this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SyncUsersで500
2 participants