Skip to content

Conversation

@marcalff
Copy link
Member

Contributes to #2481

This is a partial fix, to implement parser location, for better yaml error messages.

Changes

Please provide a brief description of the changes here.

  • Implement the "Location" of a yaml node in the parser

    • Make all document nodes point to the document root
    • Make the document root own the yaml parser that is used to create the document
    • Inspect a node location by following the links up to the document root, and then invoke the rapidyaml parser.
  • Implement a location property in yaml related exceptions

  • Change all the parsing code to pass the current yaml node as context, in case code needs to access a node location to raise an exception.

With all these changes, when the parser fails with an invalid schema exception, the exception contains:

  • the name of the yaml input file used
  • the line number, column number, and offset from the document start, where the error was detected.

This helps ease of use, so error messages about invalid config.yaml files point precisely at the error location.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@marcalff marcalff requested a review from a team as a code owner October 18, 2025 20:57
@marcalff marcalff added the pr:please-review This PR is ready for review label Oct 18, 2025
@codecov
Copy link

codecov bot commented Oct 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.00%. Comparing base (82f72f5) to head (b1a73f8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3705      +/-   ##
==========================================
+ Coverage   89.95%   90.00%   +0.05%     
==========================================
  Files         225      225              
  Lines        7273     7273              
==========================================
+ Hits         6542     6545       +3     
+ Misses        731      728       -3     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@dbarker dbarker left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks! one very minor / optional nitpick

@marcalff marcalff merged commit 755411e into open-telemetry:main Oct 20, 2025
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:please-review This PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants