chore(engines): improves error messages for easier debugging#84
chore(engines): improves error messages for easier debugging#84
Conversation
Enhances error messages to provide more context when resolving values during Terraform deployment, aiding in debugging and issue identification. The improved error messages now include the specific property, intent, or variable name that failed to resolve, along with the underlying error. This makes it easier to pinpoint the source of the problem.
WalkthroughThe change updates error handling in engines/terraform/resolve.go by wrapping returned errors with contextual messages in resolveValue, resolveTokensForModule, and resolveDependencies. Specific error strings were standardized to lowercase for messages about missing variables in a blueprint or platform. A minor formatting adjustment ensures a trailing newline at the end of the file. No exported or public declarations were altered. Pre-merge checks✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
engines/terraform/resolve.go (1)
36-45: Guard infra tokens to avoid index-out-of-range panic; quote resource in errorCurrently assumes at least two path segments for infra tokens. Add a length check and improve quoting.
- if specRef.Source == "infra" { - refName := specRef.Path[0] - propertyName := specRef.Path[1] - - infraResource, err := td.resolveInfraResource(refName) - if err != nil { - return nil, fmt.Errorf("failed to resolve infrastructure resource %s: %w", refName, err) - } - return infraResource.Get(jsii.String(propertyName)), nil + if specRef.Source == "infra" { + if len(specRef.Path) < 2 { + return nil, fmt.Errorf("invalid infra token %q: expected format infra.<resource>.<property>", v) + } + refName := specRef.Path[0] + propertyName := specRef.Path[1] + + infraResource, err := td.resolveInfraResource(refName) + if err != nil { + return nil, fmt.Errorf("failed to resolve infrastructure resource %q: %w", refName, err) + } + return infraResource.Get(jsii.String(propertyName)), nil
🧹 Nitpick comments (6)
engines/terraform/resolve.go (6)
48-48: Include clearer context and quote variable nameAdd intent context and use %q for safer/more readable messages.
- return nil, fmt.Errorf("variable %s does not exist for provided blueprint", specRef.Path[0]) + return nil, fmt.Errorf("variable %q does not exist for provided blueprint %q", specRef.Path[0], intentName)
54-54: Quote variable name for consistencyUse %q to clearly delimit names.
- return nil, fmt.Errorf("variable %s does not exist for this platform", specRef.Path[0]) + return nil, fmt.Errorf("variable %q does not exist for this platform", specRef.Path[0])
88-92: Quote property and intent in errorSmall readability boost; avoids ambiguity with spaces/special chars.
- return fmt.Errorf("failed to resolve property %s for %s: %w", property, intentName, err) + return fmt.Errorf("failed to resolve property %q for %q: %w", property, intentName, err)
111-113: Include the offending token in depends_on errorHelps pinpoint the exact bad reference in blueprints.
- if specRef.Source != "infra" { - return fmt.Errorf("depends_on can only reference infra resources") - } + if specRef.Source != "infra" { + return fmt.Errorf("depends_on can only reference infra resources: %q", dependsOn) + }
116-119: Quote dependency name in errorMinor readability improvement.
- return fmt.Errorf("failed to resolve infrastructure dependency %s: %w", specRef.Path[0], err) + return fmt.Errorf("failed to resolve infrastructure dependency %q: %w", specRef.Path[0], err)
11-21: Echo the invalid token in parse errorsIncluding the raw token makes diagnostics much faster.
- if !ok { - return nil, fmt.Errorf("invalid token format") - } + if !ok { + return nil, fmt.Errorf("invalid token format: %q", token) + } @@ - if len(parts) < 2 { - return nil, fmt.Errorf("invalid token format") - } + if len(parts) < 2 { + return nil, fmt.Errorf("invalid token format: %q (expected <source>.<path>)", token) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
engines/terraform/resolve.go(3 hunks)
🔇 Additional comments (2)
engines/terraform/resolve.go (2)
42-42: Nice contextual wrapError wrapping with context is good. Combined with quoting (see prior suggestion), this will be very readable.
126-126: No-op formatting changeTrailing newline is fine.
|
🎉 This PR is included in version 0.1.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
Enhances error messages to provide more context when resolving values during Terraform deployment, aiding in debugging and issue identification.
The improved error messages now include the specific property, intent, or variable name that failed to resolve, along with the underlying error. This makes it easier to pinpoint the source of the problem.