fix: handle ValueError from formatter.parse() on malformed format strings (#733)#1030
fix: handle ValueError from formatter.parse() on malformed format strings (#733)#1030devimano2011 wants to merge 4 commits intotaverntesting:masterfrom
Conversation
…ings (taverntesting#733) Add error handling for invalid format strings in dict_util.py
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tavern/_core/dict_util.py`:
- Around line 48-56: The _attempt_find_include() path must be protected the same
way as _check_and_format_values(): wrap the call to formatter.parse(to_format)
inside a try/except that catches ValueError, logs a warning (same message style
used in the existing block) and returns the original to_format (or otherwise
skip include processing) instead of allowing the exception to propagate;
refactor or extract the guarded-parse logic into a small helper used by both
_check_and_format_values() and _attempt_find_include() so both call the safe
parse routine rather than calling formatter.parse(...) directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25effe64-e8cb-4cc4-ad35-113b3da0d6b6
📒 Files selected for processing (1)
tavern/_core/dict_util.py
| try: | ||
| would_format = list(formatter.parse(to_format)) | ||
| except ValueError: | ||
| logger.warning( | ||
| "Could not parse format string '%s' - string contains invalid format syntax " | ||
| "(e.g. unmatched '{' or '}'). Returning string unformatted.", | ||
| to_format, | ||
| ) | ||
| return to_format |
There was a problem hiding this comment.
Also guard the !include parse path.
This try/except fixes _check_and_format_values(), but _attempt_find_include() still does a bare formatter.parse(to_format) on Line 103. A malformed string can still raise ValueError there and crash the run, so the fix is only partial. Consider reusing the same guarded parse logic in both code paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tavern/_core/dict_util.py` around lines 48 - 56, The _attempt_find_include()
path must be protected the same way as _check_and_format_values(): wrap the call
to formatter.parse(to_format) inside a try/except that catches ValueError, logs
a warning (same message style used in the existing block) and returns the
original to_format (or otherwise skip include processing) instead of allowing
the exception to propagate; refactor or extract the guarded-parse logic into a
small helper used by both _check_and_format_values() and _attempt_find_include()
so both call the safe parse routine rather than calling formatter.parse(...)
directly.
…ValueError Add error handling for string formatting in dict_util.py
…lers Added a helper function to safely parse format strings, improving error handling for invalid syntax.
michaelboulton
left a comment
There was a problem hiding this comment.
This shouldn't silently fail and send None, this would be very undesirable in the case where, for example, someone is trying to PATCH a field on an object and it sends null and wipes out a field instead of setting it.
If anything it should catch the ValueError and re-raise a specific tavern exception mentioning the unmatched curl brace.
…urning None Removed the _safe_parse_format_string function and integrated its logic directly into _check_and_format_values and another function, improving error handling for format strings.
Fixes #733
Problem
When a string value in a test contains an unmatched
{character (e.g. from an MQTT payload field like{Explanation 1), Python'sstring.Formatter().parse()raises aValueError:This caused Tavern to crash with an unhandled exception instead of continuing test execution.
Fix
As suggested by @michaelboulton in the issue, wrap
formatter.parse(to_format)in atry/except ValueError. When the string cannot be parsed (due to unmatched braces), log a warning and return the original string unformatted so test execution can continue.Changes
tavern/_core/dict_util.py: wrapformatter.parse()in try/except ValueError in_check_and_format_values()Summary by CodeRabbit
Release Notes