🐛 azure: fail gracefully if unable to get security contacts #5183
Conversation
|
Failing because of #5184 |
afiune
changed the title
afiune
force-pushed
the
azure/bug/alertNotifications
branch
from
b3bce03 to
fb4477f
Compare
Closes #4997 Signed-off-by: Salim Afiune Maya <[email protected]>
afiune
force-pushed
the
azure/bug/alertNotifications
branch
from
fb4477f to
04f80ca
Compare
afiune
changed the title
Test |
| list, err := getSecurityContacts(ctx, armConn) | ||
| if err != nil { | ||
| return nil, err | ||
| // https: //github.com/mondoohq/cnquery/issues/4997 | ||
| log.Warn().Err(err).Msg("fail gracefully") |
There was a problem hiding this comment.
I was debating between this an a simple debug log message, I fee that is a better option, would you agree?
| } | ||
| res := []interface{}{} | ||
| for _, contact := range list { | ||
| alertNotifications, err := convert.JsonToDict(contact.Properties.AlertNotifications) |
There was a problem hiding this comment.
do we need to check if contact or contact.Properties are set?
There was a problem hiding this comment.
yeah, we probably should have done that, I'll add the check.
| for _, contact := range list { | ||
| alertNotifications, err := convert.JsonToDict(contact.Properties.AlertNotifications) | ||
| if err != nil { | ||
| return nil, err | ||
| log.Debug().Err(err).Msg("unable to convert armsecurity.Contact.Properties.AlertNotifications to dict") | ||
| } | ||
| notificationsByRole, err := convert.JsonToDict(contact.Properties.NotificationsByRole) |
There was a problem hiding this comment.
do we need to check if contact or contact.Properties are set?
There was a problem hiding this comment.
yeah, we probably should have done that, I'll add the check.
| for _, contact := range list { | ||
| alertNotifications, err := convert.JsonToDict(contact.Properties.AlertNotifications) | ||
| if err != nil { | ||
| return nil, err | ||
| log.Debug().Err(err).Msg("unable to convert armsecurity.Contact.Properties.AlertNotifications to dict") |
There was a problem hiding this comment.
in which cases can this happen? We should continue to error out and handle the case properly
There was a problem hiding this comment.
We haven't seen this but, the issue says that, any error trying to get the security contacts should fail gracefully. So I checked other places that we could end up in an undesirable error.
Signed-off-by: Salim Afiune Maya <[email protected]>
|
@chris-rock updated with your comments, thank you! Also, I much rather go this way #5185 - Thoughts? |
|
Closing in favor of #5185 |
Closes #4997