-
Notifications
You must be signed in to change notification settings - Fork 25
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
Hubのpayloadの型キャストで起こっていたPanicを修正 #2095
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
他のところはあまり網羅的には見てないけどokしておきます
typesafeなpubsubは、TypeScriptみたいに型自体をindexed accessできないから、https://github.com/traPtitech/NeoShowcase/blob/b7902613441584ef24024dd8ec9f047b4621ab8b/pkg/domain/pubsub.go のようなものをイベントごとにstructのフィールドで持つ、くらいしか考えられないな |
わかる、それが欲しい |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
良さそう、 #1894 でバグに気づけなくて申し訳ないです
念の為確認なんですが、↓はWebSocketで配信するイベントなので関係ない、という認識であってますか?あってたらmergeお願いします
traQ/service/notification/handlers.go
Lines 507 to 514 in a3870e8
func userGroupCreatedHandler(ns *Service, ev hub.Message) { | |
broadcast(ns, | |
"USER_GROUP_CREATED", | |
map[string]interface{}{ | |
"id": ev.Fields["group_id"].(uuid.UUID), | |
}, | |
) | |
} |
traQ/service/notification/handlers.go
Lines 525 to 532 in a3870e8
func userGroupDeletedHandler(ns *Service, ev hub.Message) { | |
broadcast(ns, | |
"USER_GROUP_DELETED", | |
map[string]interface{}{ | |
"id": ev.Fields["group_id"].(uuid.UUID), | |
}, | |
) | |
} |
そうですね、前回バグが入った範囲ではないので、変更しないで大丈夫という認識です |
グループ追加 / グループ削除で、Hubのpayloadの型キャストが適切に行われておらず、Bot ServiceインスタンスがPanicを起こしてtraQが落ちるというバグが確認された
そのため、以下の二つのPanicを直した
Created
参考:
traQ/repository/gorm/user_group.go
Lines 39 to 45 in 867e671
Deleted
参考:
traQ/repository/gorm/user_group.go
Lines 126 to 131 in 867e671