Skip to content

_yaml: fix confusing error message #2089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
abderrahim wants to merge 1 commit into
base: master
Choose a base branch
from
+3 −3
Draft

_yaml: fix confusing error message #2089

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/buildstream/_yaml.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,10 @@ cpdef MappingNode load(str filename, str shortname, bint copy_tree=False, object
cpdef MappingNode load_data(str data, int file_index=node._SYNTHETIC_FILE_INDEX, str file_name=None, bint copy_tree=False):
cdef Representer rep

try:

@gtristan gtristan Nov 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ideally, we would have the precise exception below instead of the blind except Exception statement.

Given that determining what exception specifically is raised there may be quite difficult, I think I'd still be more comfortable with an additional except ImportError block, so that anything else happening inside the yaml.CParser(data) constructor is still in the try block.

E.g. just adding the following before the blind except may be preferrable:

try:
   ...
except ImportError as e:
    raise e
except Exception:
   ... keeping the same blind except...
``

@juergbi juergbi Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The error reported in #1833 ("'NoneType' object is not callable") is not an ImportError, so this wouldn't help.

What about something like

try:
    rep = Representer(file_index)
    parser = yaml.CParser(data)
except Exception as e:
    raise LoadError("YAML parser initialization failed:\n\n{}\n\n".format(e),
                       LoadErrorReason.INVALID_YAML) from e

try:
    try:
        while parser.check_event():
...

I.e., just switching to a more suitable error message. (We could also define a new LoadErrorReason if we prefer).

rep = Representer(file_index)
parser = yaml.CParser(data)
rep = Representer(file_index)
parser = yaml.CParser(data)

try:
try:
while parser.check_event():
rep.handle_event(parser.get_event())
Expand Down
Loading