CFn: improve error message for invalid ref#10149
Conversation
simonrw
added
the
semver: patch
dominikschubert
requested review from
dominikschubert and
pinzon
as code owners
dominikschubert
left a comment
dominikschubert
left a comment
There was a problem hiding this comment.
LGTM thanks for jumping on this. Minor change but will be super useful for anyone debugging their Stacks!
| if not resource: | ||
| raise Exception("Should be detected earlier.") | ||
| raise Exception( | ||
| f"Resource target for `!Ref {ref}` could not be found. Is there a resource with name {ref} in your stack?" |
There was a problem hiding this comment.
*with the logical resource id
There was a problem hiding this comment.
I was trying to not be too technical in this message. I think referring to "name" is more actionable, rather than "logical resource id" which is technically correct.
| raise DependencyNotYetSatisfied(resource_ids=resource_logical_id, message="") | ||
| raise DependencyNotYetSatisfied( | ||
| resource_ids=resource_logical_id, | ||
| message=f"Could not resolve attribute {attribute_name} on resource {resource_logical_id}", |
There was a problem hiding this comment.
nit: enclosing replaced values with quotation marks might make this a bit more readable
| if none_values: | ||
| raise Exception( | ||
| "Cannot resolve CF fn::Join %s due to null values: %s" % (value, join_values) | ||
| f"Cannot resolve CF Fn::Join {value} due to null values: {join_values}" |
There was a problem hiding this comment.
Here we're using the full canonical version while using !Ref above. Would suggest keeping this consistent. (Personal preference would be slightly towards canonical)
|
|
||
|
|
||
| @markers.aws.only_localstack | ||
| def test_useful_error_when_invalid_ref(deploy_cfn_template): |
There was a problem hiding this comment.
Good idea to add a test here tp avoid us messing it up when refactoring on the future. We might want to pull out tests like this from the file in the future though.
3 participants
Motivation
When we cannot resolve a
Ref- specifically if it is targeting a non-existent resource or parameter, we currently print "Should be detected earlier." which is not very useful. Instead, since this is a programming error from the creator of the template, we should be more helpful and print out the ref that cannot be resolved.Changes