Skip to content

Improve logging for unresolved placeholders#7335

Merged
galvana merged 3 commits intomainfrom
placeholder-logging
Feb 6, 2026
Merged

Improve logging for unresolved placeholders#7335
galvana merged 3 commits intomainfrom
placeholder-logging

Conversation

@galvana
Copy link
Copy Markdown
Contributor

@galvana galvana commented Feb 6, 2026

Description Of Changes

Improving the error log messages when a placeholder in a SaaS config could not be evaluated.

Old message:

ValueError: At least one param_value references an invalid field for the 'delete' request of examples.

New message:

ValueError: The following param_values reference invalid fields for the 'delete' request of examples: ['user_id', 'account_key']

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@galvana galvana requested a review from a team as a code owner February 6, 2026 20:37
@galvana galvana requested review from johnewart and removed request for a team February 6, 2026 20:37
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Feb 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Feb 6, 2026 8:43pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Feb 6, 2026 8:43pm

Request Review

@galvana galvana requested review from Linker44 and removed request for johnewart February 6, 2026 20:38
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 6, 2026

Greptile Overview

Greptile Summary

Added diagnostic helper function find_unresolved_placeholders to improve error messages when placeholder resolution fails in SaaS requests. Previously, error messages were generic ("at least one param_value references an invalid field"); now they explicitly list which placeholders couldn't be resolved.

Changes:

  • Added find_unresolved_placeholders() function that identifies which required placeholders cannot be resolved from param_values
  • Updated two error messages in map_param_values() to show specific unresolved placeholders for both path and body validation failures
  • Added comprehensive test coverage with 11 test cases covering edge cases including optional placeholders, dot-delimited keys, and partial resolution scenarios

Impact:
This significantly improves debuggability for SaaS connector configuration issues by showing developers exactly which placeholders are missing rather than forcing them to debug generic error messages.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes add a pure diagnostic function with comprehensive test coverage, improve error messages without changing any business logic, and follow existing code patterns. The new function correctly handles optional placeholders by skipping them (consistent with assign_placeholders), and all edge cases are covered by tests.
  • No files require special attention

Important Files Changed

Filename Overview
src/fides/api/util/saas_util.py Added find_unresolved_placeholders helper function and improved error messages to show specific unresolved placeholders instead of generic messages
tests/ops/util/test_saas_util.py Added comprehensive test suite for find_unresolved_placeholders covering edge cases and various placeholder scenarios

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Contributor

@Linker44 Linker44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really needed this! ship itt

@galvana galvana added this pull request to the merge queue Feb 6, 2026
Merged via the queue into main with commit 280af3d Feb 6, 2026
55 checks passed
@galvana galvana deleted the placeholder-logging branch February 6, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants