fix-issue1756: panic in dingTalkRotation function #1785
Merged
Conversation
This file contains 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
direct cause: a panic occurred when accessing a nil pointer while retrieving the workflow.assignee in the dingTalkRotation function. root cause: the status of the dingtalk instance was not updated when the workflow was canceled. as a result, the dingTalkRotation function, which handled the canceled workflow, filtered by the dingtalk instance whose status was not updated correctly. solution: update the status of the dingtalk instance to “canceled” in the logic for closing the workflow. this will prevent similar issues from occurring in the future.
taolx0
approved these changes
Sep 6, 2023
| @@ -189,14 +190,52 @@ func CancelApprove(workflowID uint) { | |||
| s := model.GetStorage() | |||
There was a problem hiding this comment.
CancelApprove 函数不仅仅只有钉钉审批,还有其他的,比如飞书...如果是飞书审批,这里逻辑就错了
There was a problem hiding this comment.
确实，CancelApprove 应该像im.go中其他函数一样区分不同的im类型，比如飞书和钉钉，根据不同的im类型去取消对应的审批，但是这个issue是解决钉钉轮询panic的问题，我觉得应该另外起一个issue来解决CancelApprove函数中没有根据不同的im类型取消对应的审批的逻辑缺陷。
taolx0
reviewed
Sep 6, 2023
| @@ -189,14 +190,52 @@ func CancelApprove(workflowID uint) { | |||
| s := model.GetStorage() | |||
sqle/model/configuration.go
Outdated
| } | ||
|
|
||
| // batch update ding_talk_instances'status into canceled by workflow_ids | ||
| func (s *Storage) BatchCancelDingTalkInstance(workflowIds []uint) error { |
There was a problem hiding this comment.
入参可以改成(workflowIds []uint,status string),批量更新钉钉审批状态,更通用点
|
好的😉，谢谢，你改完之后我再拜读一下
获取Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: taolongxiang ***@***.***>
Sent: Wednesday, September 6, 2023 7:08:41 PM
To: actiontech/sqle ***@***.***>
Cc: Winfred ***@***.***>; Author ***@***.***>
Subject: Re: [actiontech/sqle] fix-issue1756: panic in dingTalkRotation function (PR #1785)
@taolx0 commented on this pull request.
________________________________
In sqle/model/configuration.go<#1785 (comment)>:
@@ -497,6 +498,24 @@ func (s *Storage) GetDingTalkInstanceByWorkflowID(workflowId uint) (*DingTalkIns
return dti, true, errors.New(errors.ConnectStorageError, err)
}
+func (s *Storage) GetDingTalkInstanceListByWorkflowIDs(workflowIds []uint) ([]DingTalkInstance, error) {
+ var dingTalkInstances []DingTalkInstance
+ err := s.db.Model(&DingTalkInstance{}).Where("workflow_id IN (?)", workflowIds).Find(&dingTalkInstances).Error
+ if err != nil {
+ return nil, err
+ }
+ return dingTalkInstances, nil
+}
+
+// batch update ding_talk_instances'status into canceled by workflow_ids
+func (s *Storage) BatchCancelDingTalkInstance(workflowIds []uint) error {
入参可以改成(workflowIds []uint,status string),批量更新钉钉审批状态,更通用点
________________________________
In sqle/pkg/im/im.go<#1785 (comment)>:
@@ -189,14 +190,52 @@ func CancelApprove(workflowID uint) {
s := model.GetStorage()
额...刚好我在处理飞书审批,那就我来改吧
—
Reply to this email directly, view it on GitHub<#1785 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHNCBUZD4JNFYJ6DRBIEZU3XZBKTTANCNFSM6AAAAAA4M3ATSA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
before: func (s *Storage) BatchCancelDingTalkInstance(workflowIds []uint) error after: func (s *Storage) BatchUptateStatusOfDingTalkInstance(workflowIds []uint, status string) error the latter function is able to update ding_talk_instances'status into any status which belongs to ding_talk_instances'status. @LinXiaoTao
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#1756
direct cause: a panic occurred when accessing a nil pointer while retrieving the workflow.assignee in the dingTalkRotation function.
root cause: the status of the dingtalk instance was not updated when the workflow was canceled. as a result, the dingTalkRotation function, which handled the canceled workflow, filtered by the dingtalk instance whose status was not updated correctly.
solution: update the status of the dingtalk instance to “canceled” in the logic for closing the workflow. this will prevent similar issues from occurring in the future.