I've been going over our error handling practices and some of the log messages and I think we need to have some alignment, especially with Terraform's best practices:
2) In case of error in a Set operations or any other action we need to use return fmt.Errorf() method with a short description of the error that occurred, affected resource and the error content, for example:
Currently, we use log.Printf() and return err which is not a very good way to handle this, since the log message is suppressed and the err returned loacks context.
3) Logging of debug and info messages we wish to add throughout the code, should use log.Printf() (if formatting is required) or log.Println() (if there's no formatting) and be tagged as [DEBUG] or [INFO] accordingly, but this should not be used in error contexts.
I will try to work on a PR to align with these practices.
dannyk81
commented
6 years ago
Hi @scshitole,
I've been going over our error handling practices and some of the log messages and I think we need to have some alignment, especially with Terraform's best practices:
1) Error checking on aggregate types only (schema.TypeList, schema.TypeSet, and schema.TypeMap) reference: https://www.terraform.io/docs/extend/best-practices/detecting-drift.html#error-checking-aggregate-types
2) In case of error in a Set operations or any other action we need to use
return fmt.Errorf()method with a short description of the error that occurred, affected resource and the error content, for example:Currently, we use
log.Printf()andreturn errwhich is not a very good way to handle this, since the log message is suppressed and theerrreturned loacks context.3) Logging of debug and info messages we wish to add throughout the code, should use
log.Printf()(if formatting is required) orlog.Println()(if there's no formatting) and be tagged as[DEBUG]or[INFO]accordingly, but this should not be used in error contexts.I will try to work on a PR to align with these practices.